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: Alan Stern <stern@rowland.harvard.edu>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

1 files changed, 20 insertions, 21 deletions

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 46ca2af5ef1c..38b8bce782d6 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -995,7 +995,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
                                 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)) {
@@ -1051,11 +1051,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
                         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",
@@ -1075,9 +1070,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
                 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;
 
@@ -1119,28 +1111,35 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
                         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) |
@@ -1182,7 +1181,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
         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;
@@ -1192,9 +1191,9 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
         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;
                 }