Commit 9180135b authored by Alan Stern's avatar Alan Stern Committed by Greg Kroah-Hartman

USB: handle zero-length usbfs submissions correctly

This patch (as1262) fixes a bug in usbfs: It refuses to accept
zero-length transfers, and it insists that the buffer pointer be valid
even if there is no data being transferred.

The patch also consolidates a bunch of repetitive access_ok() checks
into a single check, which incidentally fixes the lack of such a check
for Isochronous URBs.
Signed-off-by: default avatarAlan Stern <stern@rowland.harvard.edu>
Cc: stable <stable@kernel.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
parent ec6d67e3
...@@ -995,7 +995,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, ...@@ -995,7 +995,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
USBDEVFS_URB_ZERO_PACKET | USBDEVFS_URB_ZERO_PACKET |
USBDEVFS_URB_NO_INTERRUPT)) USBDEVFS_URB_NO_INTERRUPT))
return -EINVAL; return -EINVAL;
if (!uurb->buffer) if (uurb->buffer_length > 0 && !uurb->buffer)
return -EINVAL; return -EINVAL;
if (!(uurb->type == USBDEVFS_URB_TYPE_CONTROL && if (!(uurb->type == USBDEVFS_URB_TYPE_CONTROL &&
(uurb->endpoint & ~USB_ENDPOINT_DIR_MASK) == 0)) { (uurb->endpoint & ~USB_ENDPOINT_DIR_MASK) == 0)) {
...@@ -1051,11 +1051,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, ...@@ -1051,11 +1051,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
is_in = 0; is_in = 0;
uurb->endpoint &= ~USB_DIR_IN; uurb->endpoint &= ~USB_DIR_IN;
} }
if (!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
uurb->buffer, uurb->buffer_length)) {
kfree(dr);
return -EFAULT;
}
snoop(&ps->dev->dev, "control urb: bRequest=%02x " snoop(&ps->dev->dev, "control urb: bRequest=%02x "
"bRrequestType=%02x wValue=%04x " "bRrequestType=%02x wValue=%04x "
"wIndex=%04x wLength=%04x\n", "wIndex=%04x wLength=%04x\n",
...@@ -1075,9 +1070,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, ...@@ -1075,9 +1070,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
uurb->number_of_packets = 0; uurb->number_of_packets = 0;
if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE) if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE)
return -EINVAL; return -EINVAL;
if (!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
uurb->buffer, uurb->buffer_length))
return -EFAULT;
snoop(&ps->dev->dev, "bulk urb\n"); snoop(&ps->dev->dev, "bulk urb\n");
break; break;
...@@ -1119,28 +1111,35 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, ...@@ -1119,28 +1111,35 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
return -EINVAL; return -EINVAL;
if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE) if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE)
return -EINVAL; return -EINVAL;
if (!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
uurb->buffer, uurb->buffer_length))
return -EFAULT;
snoop(&ps->dev->dev, "interrupt urb\n"); snoop(&ps->dev->dev, "interrupt urb\n");
break; break;
default: default:
return -EINVAL; return -EINVAL;
} }
as = alloc_async(uurb->number_of_packets); if (uurb->buffer_length > 0 &&
if (!as) { !access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
uurb->buffer, uurb->buffer_length)) {
kfree(isopkt); kfree(isopkt);
kfree(dr); kfree(dr);
return -ENOMEM; return -EFAULT;
} }
as->urb->transfer_buffer = kmalloc(uurb->buffer_length, GFP_KERNEL); as = alloc_async(uurb->number_of_packets);
if (!as->urb->transfer_buffer) { if (!as) {
kfree(isopkt); kfree(isopkt);
kfree(dr); kfree(dr);
free_async(as);
return -ENOMEM; return -ENOMEM;
} }
if (uurb->buffer_length > 0) {
as->urb->transfer_buffer = kmalloc(uurb->buffer_length,
GFP_KERNEL);
if (!as->urb->transfer_buffer) {
kfree(isopkt);
kfree(dr);
free_async(as);
return -ENOMEM;
}
}
as->urb->dev = ps->dev; as->urb->dev = ps->dev;
as->urb->pipe = (uurb->type << 30) | as->urb->pipe = (uurb->type << 30) |
__create_pipe(ps->dev, uurb->endpoint & 0xf) | __create_pipe(ps->dev, uurb->endpoint & 0xf) |
...@@ -1182,7 +1181,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, ...@@ -1182,7 +1181,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
kfree(isopkt); kfree(isopkt);
as->ps = ps; as->ps = ps;
as->userurb = arg; as->userurb = arg;
if (uurb->endpoint & USB_DIR_IN) if (is_in && uurb->buffer_length > 0)
as->userbuffer = uurb->buffer; as->userbuffer = uurb->buffer;
else else
as->userbuffer = NULL; as->userbuffer = NULL;
...@@ -1192,9 +1191,9 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, ...@@ -1192,9 +1191,9 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
as->uid = cred->uid; as->uid = cred->uid;
as->euid = cred->euid; as->euid = cred->euid;
security_task_getsecid(current, &as->secid); security_task_getsecid(current, &as->secid);
if (!is_in) { if (!is_in && uurb->buffer_length > 0) {
if (copy_from_user(as->urb->transfer_buffer, uurb->buffer, if (copy_from_user(as->urb->transfer_buffer, uurb->buffer,
as->urb->transfer_buffer_length)) { uurb->buffer_length)) {
free_async(as); free_async(as);
return -EFAULT; return -EFAULT;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment