Skip to content

Commit e1af344

Browse files
Meng Xutiwai
authored andcommitted
ALSA: asihpi: fix a potential double-fetch bug when copying puhm
The hm->h.size is intended to hold the actual size of the hm struct that is copied from userspace and should always be <= sizeof(*hm). However, after copy_from_user(hm, puhm, hm->h.size), since userspace process has full control over the memory region pointed by puhm, it is possible that the value of hm->h.size is different from what is fetched-in previously (get_user(hm->h.size, (u16 __user *)puhm)). In other words, hm->h.size is overriden and the relation between hm->h.size and the hm struct is broken. This patch proposes to use a seperate variable, msg_size, to hold the value of the first fetch and override hm->h.size to msg_size after the second fetch to maintain the relation. Signed-off-by: Meng Xu <[email protected]> Signed-off-by: Takashi Iwai <[email protected]>
1 parent a931b9c commit e1af344

File tree

1 file changed

+8
-4
lines changed

1 file changed

+8
-4
lines changed

sound/pci/asihpi/hpioctl.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ long asihpi_hpi_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
103103
void __user *puhr;
104104
union hpi_message_buffer_v1 *hm;
105105
union hpi_response_buffer_v1 *hr;
106+
u16 msg_size;
106107
u16 res_max_size;
107108
u32 uncopied_bytes;
108109
int err = 0;
@@ -127,22 +128,25 @@ long asihpi_hpi_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
127128
}
128129

129130
/* Now read the message size and data from user space. */
130-
if (get_user(hm->h.size, (u16 __user *)puhm)) {
131+
if (get_user(msg_size, (u16 __user *)puhm)) {
131132
err = -EFAULT;
132133
goto out;
133134
}
134-
if (hm->h.size > sizeof(*hm))
135-
hm->h.size = sizeof(*hm);
135+
if (msg_size > sizeof(*hm))
136+
msg_size = sizeof(*hm);
136137

137138
/* printk(KERN_INFO "message size %d\n", hm->h.wSize); */
138139

139-
uncopied_bytes = copy_from_user(hm, puhm, hm->h.size);
140+
uncopied_bytes = copy_from_user(hm, puhm, msg_size);
140141
if (uncopied_bytes) {
141142
HPI_DEBUG_LOG(ERROR, "uncopied bytes %d\n", uncopied_bytes);
142143
err = -EFAULT;
143144
goto out;
144145
}
145146

147+
/* Override h.size in case it is changed between two userspace fetches */
148+
hm->h.size = msg_size;
149+
146150
if (get_user(res_max_size, (u16 __user *)puhr)) {
147151
err = -EFAULT;
148152
goto out;

0 commit comments

Comments
 (0)