| From 9180135bc80ab11199d482b6111e23f74d65af4a Mon Sep 17 00:00:00 2001 |
| From: Alan Stern <stern@rowland.harvard.edu> |
| Date: Mon, 29 Jun 2009 11:04:54 -0400 |
| Subject: USB: handle zero-length usbfs submissions correctly |
| |
| From: Alan Stern <stern@rowland.harvard.edu> |
| |
| commit 9180135bc80ab11199d482b6111e23f74d65af4a upstream. |
| |
| 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: Alan Stern <stern@rowland.harvard.edu> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> |
| |
| --- |
| drivers/usb/core/devio.c | 41 ++++++++++++++++++++--------------------- |
| 1 file changed, 20 insertions(+), 21 deletions(-) |
| |
| --- a/drivers/usb/core/devio.c |
| +++ b/drivers/usb/core/devio.c |
| @@ -982,7 +982,7 @@ static int proc_do_submiturb(struct dev_ |
| USBDEVFS_URB_ZERO_PACKET | |
| USBDEVFS_URB_NO_INTERRUPT)) |
| return -EINVAL; |
| - if (!uurb->buffer) |
| + if (uurb->buffer_length > 0 && !uurb->buffer) |
| return -EINVAL; |
| if (!(uurb->type == USBDEVFS_URB_TYPE_CONTROL && |
| (uurb->endpoint & ~USB_ENDPOINT_DIR_MASK) == 0)) { |
| @@ -1038,11 +1038,6 @@ static int proc_do_submiturb(struct dev_ |
| is_in = 0; |
| 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 " |
| "bRrequestType=%02x wValue=%04x " |
| "wIndex=%04x wLength=%04x\n", |
| @@ -1062,9 +1057,6 @@ static int proc_do_submiturb(struct dev_ |
| uurb->number_of_packets = 0; |
| if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE) |
| 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"); |
| break; |
| |
| @@ -1106,28 +1098,35 @@ static int proc_do_submiturb(struct dev_ |
| return -EINVAL; |
| if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE) |
| 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"); |
| break; |
| |
| default: |
| return -EINVAL; |
| } |
| - as = alloc_async(uurb->number_of_packets); |
| - if (!as) { |
| + if (uurb->buffer_length > 0 && |
| + !access_ok(is_in ? VERIFY_WRITE : VERIFY_READ, |
| + uurb->buffer, uurb->buffer_length)) { |
| kfree(isopkt); |
| kfree(dr); |
| - return -ENOMEM; |
| + return -EFAULT; |
| } |
| - as->urb->transfer_buffer = kmalloc(uurb->buffer_length, GFP_KERNEL); |
| - if (!as->urb->transfer_buffer) { |
| + as = alloc_async(uurb->number_of_packets); |
| + if (!as) { |
| kfree(isopkt); |
| kfree(dr); |
| - free_async(as); |
| 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->pipe = (uurb->type << 30) | |
| __create_pipe(ps->dev, uurb->endpoint & 0xf) | |
| @@ -1169,7 +1168,7 @@ static int proc_do_submiturb(struct dev_ |
| kfree(isopkt); |
| as->ps = ps; |
| as->userurb = arg; |
| - if (uurb->endpoint & USB_DIR_IN) |
| + if (is_in && uurb->buffer_length > 0) |
| as->userbuffer = uurb->buffer; |
| else |
| as->userbuffer = NULL; |
| @@ -1179,9 +1178,9 @@ static int proc_do_submiturb(struct dev_ |
| as->uid = cred->uid; |
| as->euid = cred->euid; |
| 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, |
| - as->urb->transfer_buffer_length)) { |
| + uurb->buffer_length)) { |
| free_async(as); |
| return -EFAULT; |
| } |