> Specificly, if you have a thread doing bulk reads on a USB device in a
> loop, and the device stops talking to you (for instance, waiting for you
> to reply), it becomes impossible to take any action on the device,
> including aborting the read or issuing a write to the device to tell it
> to keep talking until such a time as the read times out, the device
> speaks to us, or the device is physically disconnected.
> 
> Using timeouts can mediate this, but at the cost of being far less
> responsive unless you use a very short timeout.
> 
> I am unsure if the kernel provides the locking infrastructure to allow
> us to keep us from reading/writing while doing the various events, while
> also allowing us to read and write at the same time.

Hi Zephaniah, I've written a patch to do this, but I haven't finished testing
it yet.  I've put the current version below.  It doesn't (yet) fix the problem
of not being able to interrupt a read/write by sending a signal.  However
it brings things back to where they were before the extra locking went in.
It's against Greg's tree from a little while ago, so may need some fiddling
with before it applies.

Ciao,

Duncan.

===== drivers/usb/core/devio.c 1.77 vs edited =====
--- 1.77/drivers/usb/core/devio.c       2004-05-13 17:39:48 +02:00
+++ edited/drivers/usb/core/devio.c     2004-06-27 21:40:36 +02:00
@@ -45,6 +45,7 @@
 #include <linux/usbdevice_fs.h>
 #include <asm/uaccess.h>
 #include <asm/byteorder.h>
+#include <linux/moduleparam.h>
 
 #include "hcd.h"       /* for usbcore internals */
 #include "usb.h"
@@ -70,6 +71,7 @@
                        dev_info( dev , format , ## arg);       \
        } while (0)
 
+#define MAX_BUFFER_LENGTH      16384
 
 static inline int connected (struct usb_device *dev)
 {
@@ -180,29 +182,12 @@
  * async list handling
  */
 
-static struct async *alloc_async(unsigned int numisoframes)
-{
-        unsigned int assize = sizeof(struct async) + numisoframes * sizeof(struct 
usb_iso_packet_descriptor);
-        struct async *as = kmalloc(assize, GFP_KERNEL);
-        if (!as)
-                return NULL;
-        memset(as, 0, assize);
-       as->urb = usb_alloc_urb(numisoframes, GFP_KERNEL);
-       if (!as->urb) {
-               kfree(as);
-               return NULL;
-       }
-        return as;
-}
-
 static void free_async(struct async *as)
 {
-        if (as->urb->transfer_buffer)
-                kfree(as->urb->transfer_buffer);
-        if (as->urb->setup_packet)
-                kfree(as->urb->setup_packet);
+       kfree(as->urb->transfer_buffer);
+       kfree(as->urb->setup_packet);
        usb_free_urb(as->urb);
-        kfree(as);
+       kfree(as);
 }
 
 static inline void async_newpending(struct async *as)
@@ -526,225 +511,378 @@
         return 0;
 }
 
+static void wakeup_completion(struct urb *urb, struct pt_regs *regs)
+{
+               complete((struct completion *)urb->context);
+}
+
+static void timeout_kill(unsigned long data)
+{
+       struct urb *urb = (struct urb *) data;
+        usb_unlink_urb(urb);
+}
+
+static int start_wait_urb(struct usb_device *dev, struct urb *urb, int timeout, int* 
actual_length)
+{
+       struct completion done;
+       struct timer_list timer;
+       int status;
+
+       init_completion(&done);
+       urb->context = &done;
+       urb->complete = wakeup_completion;
+       urb->transfer_flags |= URB_ASYNC_UNLINK;
+       urb->actual_length = 0;
+       status = usb_submit_urb(urb, GFP_NOIO);
+
+       if (status == 0) {
+               if (timeout > 0) {
+                       init_timer(&timer);
+                       timer.expires = jiffies + timeout;
+                       timer.data = (unsigned long)urb;
+                       timer.function = timeout_kill;
+                       /* grr.  timeout _should_ include submit delays. */
+                       add_timer(&timer);
+               }
+               up(&dev->serialize);
+               wait_for_completion(&done);
+               down(&dev->serialize);
+               status = urb->status;
+               /* note:  HCDs return ETIMEDOUT for other reasons too */
+               if (status == -ECONNRESET)
+                       status = -ETIMEDOUT;
+               if (timeout > 0)
+                       del_timer_sync(&timer);
+       }
+
+       if (actual_length)
+               *actual_length = urb->actual_length;
+       return status;
+}
+
 static int proc_control(struct dev_state *ps, void __user *arg)
 {
-       struct usb_device *dev = ps->dev;
        struct usbdevfs_ctrltransfer ctrl;
-       unsigned int tmo;
-       unsigned char *tbuf;
-       int i, j, ret;
+       struct usb_device *dev = ps->dev;
+       struct usb_ctrlrequest *dr = NULL;
+       unsigned char *tbuf = NULL;
+       struct urb *urb = NULL;
+       int dir_in, j, length, ret;
+       unsigned int pipe, timeout;
 
        if (copy_from_user(&ctrl, arg, sizeof(ctrl)))
                return -EFAULT;
-       if ((ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.wIndex)))
-               return ret;
-       if (ctrl.wLength > PAGE_SIZE)
+       length = ctrl.wLength;
+       if (length < 0 || length > PAGE_SIZE)
                return -EINVAL;
        if (!(tbuf = (unsigned char *)__get_free_page(GFP_KERNEL)))
                return -ENOMEM;
-       tmo = (ctrl.timeout * HZ + 999) / 1000;
-       if (ctrl.bRequestType & 0x80) {
-               if (ctrl.wLength && !access_ok(VERIFY_WRITE, ctrl.data, ctrl.wLength)) 
{
-                       free_page((unsigned long)tbuf);
-                       return -EINVAL;
-               }
-               snoop(&dev->dev, "control read: bRequest=%02x bRrequestType=%02x 
wValue=%04x wIndex=%04x\n", 
-                       ctrl.bRequest, ctrl.bRequestType, ctrl.wValue, ctrl.wIndex);
-
-               i = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), ctrl.bRequest, 
ctrl.bRequestType,
-                                      ctrl.wValue, ctrl.wIndex, tbuf, ctrl.wLength, 
tmo);
-               if ((i > 0) && ctrl.wLength) {
-                       if (usbfs_snoop) {
-                               dev_info(&dev->dev, "control read: data ");
-                               for (j = 0; j < ctrl.wLength; ++j)
-                                       printk ("%02x ", (unsigned char)((char 
*)ctrl.data)[j]);
-                               printk("\n");
-                       }
-                       if (copy_to_user(ctrl.data, tbuf, ctrl.wLength)) {
-                               free_page((unsigned long)tbuf);
-                               return -EFAULT;
-                       }
+       dir_in = ctrl.bRequestType & USB_DIR_IN;
+       if (dir_in) {
+               if (length && !access_ok(VERIFY_WRITE, ctrl.data, length)) {
+                       ret = -EINVAL;
+                       goto out_free;
                }
        } else {
-               if (ctrl.wLength) {
-                       if (copy_from_user(tbuf, ctrl.data, ctrl.wLength)) {
-                               free_page((unsigned long)tbuf);
-                               return -EFAULT;
+               if (length) {
+                       if (copy_from_user(tbuf, ctrl.data, length)) {
+                               ret = -EFAULT;
+                               goto out_free;
                        }
                }
-               snoop(&dev->dev, "control write: bRequest=%02x bRrequestType=%02x 
wValue=%04x wIndex=%04x\n", 
-                       ctrl.bRequest, ctrl.bRequestType, ctrl.wValue, ctrl.wIndex);
+       }
+       snoop(&dev->dev, "control %s: bRequest=%02x bRrequestType=%02x wValue=%04x 
wIndex=%04x wLength=%04x\n", 
+               dir_in ? "read" : "write", ctrl.bRequest, ctrl.bRequestType, 
ctrl.wValue, ctrl.wIndex, ctrl.wLength);
+       if (usbfs_snoop && !dir_in) {
+               dev_info(&dev->dev, "control write: data: ");
+               for (j = 0; j < length; ++j)
+                       printk ("%02x ", (unsigned char)((char *)ctrl.data)[j]);
+               printk("\n");
+       }
+       if (!(urb = usb_alloc_urb(0, GFP_KERNEL))) {
+               ret = -ENOMEM;
+               goto out_free;
+       }
+       if (!(dr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_KERNEL))) {
+               ret = -ENOMEM;
+               goto out_free;
+       }
+       down(&dev->serialize);
+       if (!connected(dev)) {
+               ret = -ENODEV;
+               goto out_release;
+       }
+       if ((ret = check_ctrlrecip(ps, ctrl.bRequestType, ctrl.wIndex)))
+               goto out_release;
+       if (dir_in)
+               pipe = usb_rcvctrlpipe(dev, 0);
+       else
+               pipe = usb_sndctrlpipe(dev, 0);
+       timeout = (ctrl.timeout * HZ + 999) / 1000;
+       dr->bRequestType= ctrl.bRequestType;
+       dr->bRequest = ctrl.bRequest;
+       dr->wValue = cpu_to_le16p(&ctrl.wValue);
+       dr->wIndex = cpu_to_le16p(&ctrl.wIndex);
+       dr->wLength = cpu_to_le16p(&ctrl.wLength);
+       usb_fill_control_urb(urb, dev, pipe, (unsigned char*)dr, tbuf, length, NULL, 
0);
+       ret = start_wait_urb(dev, urb, timeout, &length);
+       up(&dev->serialize);
+       if (ret < 0) {
+               dev_warn(&dev->dev, "usbfs: USBDEVFS_CONTROL failed "
+                          "cmd %s rqt %u rq %u len %u ret %d\n",
+                          current->comm, ctrl.bRequestType, ctrl.bRequest,
+                          ctrl.wLength, ret);
+               goto out_free;
+       }
+       if (dir_in && length) {
                if (usbfs_snoop) {
-                       dev_info(&dev->dev, "control write: data: ");
-                       for (j = 0; j < ctrl.wLength; ++j)
-                               printk ("%02x ", (unsigned char)((char 
*)ctrl.data)[j]);
+                       dev_info(&dev->dev, "control read: data ");
+                       for (j = 0; j < length; ++j)
+                               printk ("%02x ", tbuf[j]);
                        printk("\n");
                }
-               i = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), ctrl.bRequest, 
ctrl.bRequestType,
-                                      ctrl.wValue, ctrl.wIndex, tbuf, ctrl.wLength, 
tmo);
+               if (copy_to_user(ctrl.data, tbuf, length)) {
+                       ret = -EFAULT;
+                       goto out_free;
+               }
        }
+       kfree(dr);
+       usb_free_urb(urb);
        free_page((unsigned long)tbuf);
-       if (i<0) {
-               dev_printk(KERN_DEBUG, &dev->dev, "usbfs: USBDEVFS_CONTROL "
-                          "failed cmd %s rqt %u rq %u len %u ret %d\n",
-                          current->comm, ctrl.bRequestType, ctrl.bRequest,
-                          ctrl.wLength, i);
-       }
-       return i;
+       return ret;
+out_release:
+       up(&dev->serialize);
+out_free:
+       kfree(dr);
+       usb_free_urb(urb);
+       free_page((unsigned long)tbuf);
+       return ret;
 }
 
 static int proc_bulk(struct dev_state *ps, void __user *arg)
 {
-       struct usb_device *dev = ps->dev;
        struct usbdevfs_bulktransfer bulk;
-       unsigned int tmo, len1, pipe;
-       int len2;
-       unsigned char *tbuf;
-       int i, ret;
+       struct usb_device *dev = ps->dev;
+       unsigned char *tbuf = NULL;
+       struct urb *urb = NULL;
+       int dir_in, length, ret;
+       unsigned int pipe, timeout;
 
        if (copy_from_user(&bulk, arg, sizeof(bulk)))
                return -EFAULT;
-       if ((ret = findintfep(ps->dev, bulk.ep)) < 0)
-               return ret;
-       if ((ret = checkintf(ps, ret)))
-               return ret;
-       if (bulk.ep & USB_DIR_IN)
-               pipe = usb_rcvbulkpipe(dev, bulk.ep & 0x7f);
-       else
-               pipe = usb_sndbulkpipe(dev, bulk.ep & 0x7f);
-       if (!usb_maxpacket(dev, pipe, !(bulk.ep & USB_DIR_IN)))
+       length = bulk.len;
+       if (length < 0 || length > MAX_BUFFER_LENGTH)
                return -EINVAL;
-       len1 = bulk.len;
-       if (!(tbuf = kmalloc(len1, GFP_KERNEL)))
+       if (!(tbuf = kmalloc(length, GFP_KERNEL)))
                return -ENOMEM;
-       tmo = (bulk.timeout * HZ + 999) / 1000;
-       if (bulk.ep & 0x80) {
-               if (len1 && !access_ok(VERIFY_WRITE, bulk.data, len1)) {
-                       kfree(tbuf);
-                       return -EINVAL;
-               }
-               i = usb_bulk_msg(dev, pipe, tbuf, len1, &len2, tmo);
-               if (!i && len2) {
-                       if (copy_to_user(bulk.data, tbuf, len2)) {
-                               kfree(tbuf);
-                               return -EFAULT;
-                       }
+       dir_in = bulk.ep & USB_DIR_IN;
+       if (dir_in) {
+               if (length && !access_ok(VERIFY_WRITE, bulk.data, length)) {
+                       ret = -EINVAL;
+                       goto out_free;
                }
        } else {
-               if (len1) {
-                       if (copy_from_user(tbuf, bulk.data, len1)) {
-                               kfree(tbuf);
-                               return -EFAULT;
+               if (length) {
+                       if (copy_from_user(tbuf, bulk.data, length)) {
+                               ret = -EFAULT;
+                               goto out_free;
                        }
                }
-               i = usb_bulk_msg(dev, pipe, tbuf, len1, &len2, tmo);
        }
-       kfree(tbuf);
-       if (i < 0) {
+       if (!(urb = usb_alloc_urb(0, GFP_KERNEL))) {
+               ret = -ENOMEM;
+               goto out_free;
+       }
+       down(&dev->serialize);
+       if (!connected(dev)) {
+               ret = -ENODEV;
+               goto out_release;
+       }
+       if ((ret = findintfep(ps->dev, bulk.ep)) < 0)
+               goto out_release;
+       if ((ret = checkintf(ps, ret)))
+               goto out_release;
+       if (dir_in)
+               pipe = usb_rcvbulkpipe(dev, bulk.ep & 0x7f);
+       else
+               pipe = usb_sndbulkpipe(dev, bulk.ep & 0x7f);
+       if (!usb_maxpacket(dev, pipe, !dir_in)) {
+               ret = -EINVAL;
+               goto out_release;
+       }
+       timeout = (bulk.timeout * HZ + 999) / 1000;
+       usb_fill_bulk_urb(urb, dev, pipe, tbuf, length, NULL, NULL);
+       ret = start_wait_urb(dev, urb, timeout, &length);
+       up(&dev->serialize);
+       if (ret < 0) {
                dev_warn(&dev->dev, "usbfs: USBDEVFS_BULK failed "
-                        "ep 0x%x len %u ret %d\n", bulk.ep, bulk.len, i);
-               return i;
+                        "ep 0x%x len %u ret %d\n", bulk.ep, bulk.len, ret);
+               goto out_free;
+       }
+       if (dir_in && length) {
+               if (copy_to_user(bulk.data, tbuf, length)) {
+                       ret = -EFAULT;
+                       goto out_free;
+               }
        }
-       return len2;
+       usb_free_urb(urb);
+       kfree(tbuf);
+       return length;
+out_release:
+       up(&dev->serialize);
+out_free:
+       usb_free_urb(urb);
+       kfree(tbuf);
+       return ret;
 }
 
 static int proc_resetep(struct dev_state *ps, void __user *arg)
 {
+       struct usb_device *dev = ps->dev;
        unsigned int ep;
        int ret;
 
        if (get_user(ep, (unsigned int __user *)arg))
                return -EFAULT;
+       down(&dev->serialize);
+       if (!connected(dev)) {
+               ret = -ENODEV;
+               goto out;
+       }
        if ((ret = findintfep(ps->dev, ep)) < 0)
-               return ret;
+               goto out;
        if ((ret = checkintf(ps, ret)))
-               return ret;
+               goto out;
        usb_settoggle(ps->dev, ep & 0xf, !(ep & USB_DIR_IN), 0);
-       return 0;
+out:
+       up(&dev->serialize);
+       return ret;
 }
 
 static int proc_clearhalt(struct dev_state *ps, void __user *arg)
 {
+       struct usb_device *dev = ps->dev;
        unsigned int ep;
        int pipe;
        int ret;
 
        if (get_user(ep, (unsigned int __user *)arg))
                return -EFAULT;
-       if ((ret = findintfep(ps->dev, ep)) < 0)
-               return ret;
+       down(&dev->serialize);
+       if (!connected(dev)) {
+               ret = -ENODEV;
+               goto out;
+       }
+       if ((ret = findintfep(dev, ep)) < 0)
+               goto out;
        if ((ret = checkintf(ps, ret)))
-               return ret;
+               goto out;
        if (ep & USB_DIR_IN)
-                pipe = usb_rcvbulkpipe(ps->dev, ep & 0x7f);
+                pipe = usb_rcvbulkpipe(dev, ep & 0x7f);
         else
-                pipe = usb_sndbulkpipe(ps->dev, ep & 0x7f);
+                pipe = usb_sndbulkpipe(dev, ep & 0x7f);
 
-       return usb_clear_halt(ps->dev, pipe);
+       ret = usb_clear_halt(dev, pipe);
+out:
+       up(&dev->serialize);
+       return ret;
 }
                
 
 static int proc_getdriver(struct dev_state *ps, void __user *arg)
 {
        struct usbdevfs_getdriver gd;
+       struct usb_device *dev = ps->dev;
        struct usb_interface *intf;
-       int ret;
+       int ret = 0;
 
        if (copy_from_user(&gd, arg, sizeof(gd)))
                return -EFAULT;
+       down(&dev->serialize);
+       if (!connected(dev)) {
+               up(&dev->serialize);
+               return -ENODEV;
+       }
        down_read(&usb_bus_type.subsys.rwsem);
-       intf = usb_ifnum_to_if(ps->dev, gd.interface);
+       intf = usb_ifnum_to_if(dev, gd.interface);
        if (!intf || !intf->dev.driver)
                ret = -ENODATA;
-       else {
-               strncpy(gd.driver, intf->dev.driver->name,
-                               sizeof(gd.driver));
-               ret = (copy_to_user(arg, &gd, sizeof(gd)) ? -EFAULT : 0);
-       }
+       else
+               strncpy(gd.driver, intf->dev.driver->name, sizeof(gd.driver));
        up_read(&usb_bus_type.subsys.rwsem);
+       up(&dev->serialize);
+       if (!ret)
+               ret = (copy_to_user(arg, &gd, sizeof(gd)) ? -EFAULT : 0);
        return ret;
 }
 
 static int proc_connectinfo(struct dev_state *ps, void __user *arg)
 {
        struct usbdevfs_connectinfo ci;
+       struct usb_device *dev = ps->dev;
 
-       ci.devnum = ps->dev->devnum;
-       ci.slow = ps->dev->speed == USB_SPEED_LOW;
-       if (copy_to_user(arg, &ci, sizeof(ci)))
-               return -EFAULT;
-       return 0;
+       down(&dev->serialize);
+       if (!connected(dev)) {
+               up(&dev->serialize);
+               return -ENODEV;
+       }
+       ci.devnum = dev->devnum;
+       ci.slow = dev->speed == USB_SPEED_LOW;
+       up(&dev->serialize);
+       return copy_to_user(arg, &ci, sizeof(ci)) ? -EFAULT : 0;
 }
 
 static int proc_resetdevice(struct dev_state *ps)
 {
-       return usb_reset_device(ps->dev);
+       struct usb_device *dev = ps->dev;
+       int ret;
 
+       down(&dev->serialize);
+       if (!connected(dev))
+               ret = -ENODEV;
+       else
+               ret = usb_reset_device(dev);
+       up(&dev->serialize);
+       return ret;
 }
 
 static int proc_setintf(struct dev_state *ps, void __user *arg)
 {
        struct usbdevfs_setinterface setintf;
+       struct usb_device *dev = ps->dev;
        int ret;
 
        if (copy_from_user(&setintf, arg, sizeof(setintf)))
                return -EFAULT;
+       down(&dev->serialize);
+       if (!connected(dev)) {
+               ret = -ENODEV;
+               goto out;
+       }
        if ((ret = checkintf(ps, setintf.interface)))
-               return ret;
-       return usb_set_interface(ps->dev, setintf.interface,
-                       setintf.altsetting);
+               goto out;
+       ret = usb_set_interface(dev, setintf.interface, setintf.altsetting);
+out:
+       up(&dev->serialize);
+       return ret;
 }
 
 static int proc_setconfig(struct dev_state *ps, void __user *arg)
 {
-       unsigned int u;
-       int status = 0;
+       struct usb_device *dev = ps->dev;
        struct usb_host_config *actconfig;
+       int ret;
+       unsigned int u;
 
        if (get_user(u, (unsigned int __user *)arg))
                return -EFAULT;
 
-       actconfig = ps->dev->actconfig;
+       down(&dev->serialize);
+       if (!connected(dev)) {
+               ret = -ENODEV;
+               goto out;
+       }
+       actconfig = dev->actconfig;
  
        /* Don't touch the device if any interfaces are claimed.
         * It could interfere with other drivers' operations, and if
@@ -755,7 +893,7 @@
  
                for (i = 0; i < actconfig->desc.bNumInterfaces; ++i) {
                        if (usb_interface_claimed(actconfig->interface[i])) {
-                               dev_warn (&ps->dev->dev,
+                               dev_warn (&dev->dev,
                                        "usbfs: interface %d claimed "
                                        "while '%s' sets config #%d\n",
                                        actconfig->interface[i]
@@ -763,8 +901,8 @@
                                                ->desc.bInterfaceNumber,
                                        current->comm, u);
 #if 0  /* FIXME:  enable in 2.6.10 or so */
-                               status = -EBUSY;
-                               break;
+                               ret = -EBUSY;
+                               goto out;
 #endif
                        }
                }
@@ -773,23 +911,24 @@
        /* SET_CONFIGURATION is often abused as a "cheap" driver reset,
         * so avoid usb_set_configuration()'s kick to sysfs
         */
-       if (status == 0) {
-               if (actconfig && actconfig->desc.bConfigurationValue == u)
-                       status = usb_reset_configuration(ps->dev);
-               else
-                       status = usb_set_configuration(ps->dev, u);
-       }
+       if (actconfig && actconfig->desc.bConfigurationValue == u)
+               ret = usb_reset_configuration(dev);
+       else
+               ret = usb_set_configuration(dev, u);
 
-       return status;
+out:
+       up(&dev->serialize);
+       return ret;
 }
 
 static int proc_submiturb(struct dev_state *ps, void __user *arg)
 {
        struct usbdevfs_urb uurb;
-       struct usbdevfs_iso_packet_desc *isopkt = NULL;
-       struct usb_endpoint_descriptor *ep_desc;
-       struct async *as;
+       struct async *as = NULL;
+       struct usb_device *dev = ps->dev;
        struct usb_ctrlrequest *dr = NULL;
+       struct usb_endpoint_descriptor *ep_desc;
+       struct usbdevfs_iso_packet_desc *isopkt = NULL;
        unsigned int u, totlen, isofrmlen;
        int ret, interval = 0, ifnum = -1;
 
@@ -802,50 +941,35 @@
                return -EINVAL;
        if (uurb.signr != 0 && (uurb.signr < SIGRTMIN || uurb.signr > SIGRTMAX))
                return -EINVAL;
-       if (!(uurb.type == USBDEVFS_URB_TYPE_CONTROL && (uurb.endpoint & 
~USB_ENDPOINT_DIR_MASK) == 0)) {
-               if ((ifnum = findintfep(ps->dev, uurb.endpoint)) < 0)
-                       return ifnum;
-               if ((ret = checkintf(ps, ifnum)))
-                       return ret;
-       }
+
        switch(uurb.type) {
        case USBDEVFS_URB_TYPE_CONTROL:
-               if ((uurb.endpoint & ~USB_ENDPOINT_DIR_MASK) != 0) {
-                       if (!(ep_desc = usb_epnum_to_ep_desc(ps->dev, uurb.endpoint)))
-                               return -ENOENT;
-                       if ((ep_desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) != 
USB_ENDPOINT_XFER_CONTROL)
-                               return -EINVAL;
-               }
                /* min 8 byte setup packet, max arbitrary */
                if (uurb.buffer_length < 8 || uurb.buffer_length > PAGE_SIZE)
                        return -EINVAL;
                if (!(dr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_KERNEL)))
                        return -ENOMEM;
                if (copy_from_user(dr, uurb.buffer, 8)) {
-                       kfree(dr);
-                       return -EFAULT;
+                       ret = -EFAULT;
+                       goto out_free;
                }
                if (uurb.buffer_length < (le16_to_cpup(&dr->wLength) + 8)) {
-                       kfree(dr);
-                       return -EINVAL;
-               }
-               if ((ret = check_ctrlrecip(ps, dr->bRequestType, 
le16_to_cpup(&dr->wIndex)))) {
-                       kfree(dr);
-                       return ret;
+                       ret = -EINVAL;
+                       goto out_free;
                }
                uurb.endpoint = (uurb.endpoint & ~USB_ENDPOINT_DIR_MASK) | 
(dr->bRequestType & USB_ENDPOINT_DIR_MASK);
                uurb.number_of_packets = 0;
                uurb.buffer_length = le16_to_cpup(&dr->wLength);
                uurb.buffer += 8;
                if (!access_ok((uurb.endpoint & USB_DIR_IN) ?  VERIFY_WRITE : 
VERIFY_READ, uurb.buffer, uurb.buffer_length)) {
-                       kfree(dr);
-                       return -EFAULT;
+                       ret = -EFAULT;
+                       goto out_free;
                }
                break;
 
        case USBDEVFS_URB_TYPE_BULK:
                uurb.number_of_packets = 0;
-               if (uurb.buffer_length > 16384)
+               if (uurb.buffer_length > MAX_BUFFER_LENGTH)
                        return -EINVAL;
                if (!access_ok((uurb.endpoint & USB_DIR_IN) ? VERIFY_WRITE : 
VERIFY_READ, uurb.buffer, uurb.buffer_length))
                        return -EFAULT;
@@ -855,39 +979,30 @@
                /* arbitrary limit */
                if (uurb.number_of_packets < 1 || uurb.number_of_packets > 128)
                        return -EINVAL;
-               if (!(ep_desc = usb_epnum_to_ep_desc(ps->dev, uurb.endpoint)))
-                       return -ENOENT;
-               interval = 1 << min (15, ep_desc->bInterval - 1);
                isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) * 
uurb.number_of_packets;
                if (!(isopkt = kmalloc(isofrmlen, GFP_KERNEL)))
                        return -ENOMEM;
                if (copy_from_user(isopkt, &((struct usbdevfs_urb 
*)arg)->iso_frame_desc, isofrmlen)) {
-                       kfree(isopkt);
-                       return -EFAULT;
+                       ret = -EFAULT;
+                       goto out_free;
                }
                for (totlen = u = 0; u < uurb.number_of_packets; u++) {
                        if (isopkt[u].length > 1023) {
-                               kfree(isopkt);
-                               return -EINVAL;
+                               ret = -EINVAL;
+                               goto out_free;
                        }
                        totlen += isopkt[u].length;
                }
                if (totlen > 32768) {
-                       kfree(isopkt);
-                       return -EINVAL;
+                       ret = -EINVAL;
+                       goto out_free;
                }
                uurb.buffer_length = totlen;
                break;
 
        case USBDEVFS_URB_TYPE_INTERRUPT:
                uurb.number_of_packets = 0;
-               if (!(ep_desc = usb_epnum_to_ep_desc(ps->dev, uurb.endpoint)))
-                       return -ENOENT;
-               if (ps->dev->speed == USB_SPEED_HIGH)
-                       interval = 1 << min (15, ep_desc->bInterval - 1);
-               else
-                       interval = ep_desc->bInterval;
-               if (uurb.buffer_length > 16384)
+               if (uurb.buffer_length > MAX_BUFFER_LENGTH)
                        return -EINVAL;
                if (!access_ok((uurb.endpoint & USB_DIR_IN) ? VERIFY_WRITE : 
VERIFY_READ, uurb.buffer, uurb.buffer_length))
                        return -EFAULT;
@@ -896,21 +1011,88 @@
        default:
                return -EINVAL;
        }
-       if (!(as = alloc_async(uurb.number_of_packets))) {
-               if (isopkt)
-                       kfree(isopkt);
-               if (dr)
-                       kfree(dr);
-               return -ENOMEM;
+
+       {
+               unsigned int assize = sizeof(struct async) + uurb.number_of_packets * 
sizeof(struct usb_iso_packet_descriptor);
+
+               if (!(as = kmalloc(assize, GFP_KERNEL))) {
+                       ret = -ENOMEM;
+                       goto out_free;
+               }
+               memset(as, 0, assize);
+       }
+
+       if (!(as->urb = usb_alloc_urb(uurb.number_of_packets, GFP_KERNEL))) {
+               ret = -ENOMEM;
+               goto out_free;
        }
+
        if (!(as->urb->transfer_buffer = kmalloc(uurb.buffer_length, GFP_KERNEL))) {
-               if (isopkt)
-                       kfree(isopkt);
-               if (dr)
-                       kfree(dr);
-               free_async(as);
-               return -ENOMEM;
+               ret = -ENOMEM;
+               goto out_free;
        }
+
+       if (!(uurb.endpoint & USB_DIR_IN)) {
+               if (copy_from_user(as->urb->transfer_buffer, uurb.buffer, 
uurb.buffer_length)) {
+                       ret = -EFAULT;
+                       goto out_free;
+               }
+       }
+
+       down(&dev->serialize);
+       if (!connected(dev)) {
+               ret = -ENODEV;
+               goto out_release;
+       }
+
+       if (!(uurb.type == USBDEVFS_URB_TYPE_CONTROL && (uurb.endpoint & 
~USB_ENDPOINT_DIR_MASK) == 0)) {
+               if ((ifnum = findintfep(ps->dev, uurb.endpoint)) < 0) {
+                       ret = ifnum;
+                       goto out_release;
+               }
+               if ((ret = checkintf(ps, ifnum)))
+                       goto out_release;
+       }
+
+       switch(uurb.type) {
+       case USBDEVFS_URB_TYPE_CONTROL:
+               if ((uurb.endpoint & ~USB_ENDPOINT_DIR_MASK) != 0) {
+                       if (!(ep_desc = usb_epnum_to_ep_desc(ps->dev, uurb.endpoint))) 
{
+                               ret = -ENOENT;
+                               goto out_release;
+                       }
+                       if ((ep_desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) != 
USB_ENDPOINT_XFER_CONTROL) {
+                               ret = -EINVAL;
+                               goto out_release;
+                       }
+               }
+               if ((ret = check_ctrlrecip(ps, dr->bRequestType, 
le16_to_cpup(&dr->wIndex))))
+                       goto out_release;
+               break;
+
+       case USBDEVFS_URB_TYPE_ISO:
+               if (!(ep_desc = usb_epnum_to_ep_desc(ps->dev, uurb.endpoint))) {
+                       ret = -ENOENT;
+                       goto out_release;
+               }
+               interval = 1 << min (15, ep_desc->bInterval - 1);
+               break;
+
+       case USBDEVFS_URB_TYPE_INTERRUPT:
+               if (!(ep_desc = usb_epnum_to_ep_desc(ps->dev, uurb.endpoint))) {
+                       ret = -ENOENT;
+                       goto out_release;
+               }
+               if (ps->dev->speed == USB_SPEED_HIGH)
+                       interval = 1 << min (15, ep_desc->bInterval - 1);
+               else
+                       interval = ep_desc->bInterval;
+               break;
+
+       default:
+               break;
+       }
+
         as->urb->dev = ps->dev;
         as->urb->pipe = (uurb.type << 30) | __create_pipe(ps->dev, uurb.endpoint & 
0xf) | (uurb.endpoint & USB_DIR_IN);
         as->urb->transfer_flags = uurb.flags;
@@ -926,8 +1108,8 @@
                as->urb->iso_frame_desc[u].length = isopkt[u].length;
                totlen += isopkt[u].length;
        }
-       if (isopkt)
-               kfree(isopkt);
+       kfree(isopkt);
+       isopkt = NULL;
        as->ps = ps;
         as->userurb = arg;
        if (uurb.endpoint & USB_DIR_IN)
@@ -937,31 +1119,47 @@
        as->signr = uurb.signr;
        as->ifnum = ifnum;
        as->task = current;
-       if (!(uurb.endpoint & USB_DIR_IN)) {
-               if (copy_from_user(as->urb->transfer_buffer, uurb.buffer, 
as->urb->transfer_buffer_length)) {
-                       free_async(as);
-                       return -EFAULT;
-               }
-       }
         async_newpending(as);
         if ((ret = usb_submit_urb(as->urb, GFP_KERNEL))) {
                dev_printk(KERN_DEBUG, &ps->dev->dev, "usbfs: usb_submit_urb returned 
%d\n", ret);
                 async_removepending(as);
-                free_async(as);
-                return ret;
+               goto out_release;
         }
+       up(&dev->serialize);
         return 0;
+
+out_release:
+       up(&dev->serialize);
+out_free:
+       if (as) {
+               if (as->urb) {
+                       kfree(as->urb->transfer_buffer);
+                       usb_free_urb(as->urb);
+               }
+               kfree(as);
+       }
+       kfree(isopkt);
+       kfree(dr);
+       return ret;
 }
 
 static int proc_unlinkurb(struct dev_state *ps, void __user *arg)
 {
        struct async *as;
+       int ret = 0;
 
-       as = async_getpending(ps, arg);
-       if (!as)
-               return -EINVAL;
-       usb_unlink_urb(as->urb);
-       return 0;
+       down(&ps->dev->serialize);
+       if (!connected(ps->dev)) {
+               ret = -ENODEV;
+               goto out;
+       }
+       if ((as = async_getpending(ps, arg)))
+               usb_unlink_urb(as->urb);
+       else
+               ret = -EINVAL;
+out:
+       up(&ps->dev->serialize);
+       return ret;
 }
 
 static int processcompl(struct async *as)
@@ -1003,6 +1201,11 @@
        struct usb_device *dev = ps->dev;
        int ret;
 
+       down(&dev->serialize);
+       if (!connected(dev)) {
+               ret = -ENODEV;
+               goto fail;
+       }
        add_wait_queue(&ps->wait, &wait);
        while (connected(dev)) {
                __set_current_state(TASK_INTERRUPTIBLE);
@@ -1016,19 +1219,20 @@
        }
        remove_wait_queue(&ps->wait, &wait);
        set_current_state(TASK_RUNNING);
-       if (as) {
-               ret = processcompl(as);
-               addr = as->userurb;
-               free_async(as);
-               if (ret)
-                       return ret;
-               if (put_user(addr, (void **)arg))
-                       return -EFAULT;
-               return 0;
+       if (!as) {
+               ret = signal_pending(current) ? -EINTR : -EIO;
+               goto fail;
        }
-       if (signal_pending(current))
-               return -EINTR;
-       return -EIO;
+       ret = processcompl(as);
+       addr = as->userurb;
+       free_async(as);
+       if (ret)
+               goto fail;
+       up(&dev->serialize);
+       return put_user(addr, (void **)arg) ? -EFAULT : 0;
+fail:
+       up(&dev->serialize);
+       return ret;
 }
 
 static int proc_reapurbnonblock(struct dev_state *ps, void __user *arg)
@@ -1037,16 +1241,25 @@
        void __user *addr;
        int ret;
 
-       if (!(as = async_getcompleted(ps)))
-               return -EAGAIN;
+       down(&ps->dev->serialize);
+       if (!connected(ps->dev)) {
+               ret = -ENODEV;
+               goto fail;
+       }
+       if (!(as = async_getcompleted(ps))) {
+               ret = -EAGAIN;
+               goto fail;
+       }
        ret = processcompl(as);
        addr = as->userurb;
        free_async(as);
        if (ret)
-               return ret;
-       if (put_user(addr, (void **)arg))
-               return -EFAULT;
-       return 0;
+               goto fail;
+       up(&ps->dev->serialize);
+       return put_user(addr, (void **)arg) ? -EFAULT : 0;
+fail:
+       up(&ps->dev->serialize);
+       return ret;
 }
 
 static int proc_disconnectsignal(struct dev_state *ps, void __user *arg)
@@ -1057,18 +1270,33 @@
                return -EFAULT;
        if (ds.signr != 0 && (ds.signr < SIGRTMIN || ds.signr > SIGRTMAX))
                return -EINVAL;
+       down(&ps->dev->serialize);
+       if (!connected(ps->dev)) {
+               up(&ps->dev->serialize);
+               return -ENODEV;
+       }
        ps->discsignr = ds.signr;
        ps->disccontext = ds.context;
+       up(&ps->dev->serialize);
        return 0;
 }
 
 static int proc_claiminterface(struct dev_state *ps, void __user *arg)
 {
        unsigned int ifnum;
+       int ret;
 
        if (get_user(ifnum, (unsigned int __user *)arg))
                return -EFAULT;
-       return claimintf(ps, ifnum);
+       down(&ps->dev->serialize);
+       if (!connected(ps->dev)) {
+               ret = -ENODEV;
+               goto out;
+       }
+       ret = claimintf(ps, ifnum);
+out:
+       up(&ps->dev->serialize);
+       return ret;
 }
 
 static int proc_releaseinterface(struct dev_state *ps, void __user *arg)
@@ -1078,20 +1306,28 @@
 
        if (get_user(ifnum, (unsigned int __user *)arg))
                return -EFAULT;
-       if ((ret = releaseintf(ps, ifnum)) < 0)
-               return ret;
+       down(&ps->dev->serialize);
+       if (!connected(ps->dev)) {
+               ret = -ENODEV;
+               goto out;
+       }
+       if ((ret = releaseintf(ps, ifnum)))
+               goto out;
        destroy_async_on_interface (ps, ifnum);
-       return 0;
+out:
+       up(&ps->dev->serialize);
+       return ret;
 }
 
 static int proc_ioctl (struct dev_state *ps, void __user *arg)
 {
        struct usbdevfs_ioctl   ctrl;
+       void                    *buf = NULL;
+       struct usb_device       *dev = ps->dev;
+       struct usb_driver       *driver = NULL;
+       struct usb_interface    *intf = NULL;
+       int                     ret = 0;
        int                     size;
-       void                    *buf = 0;
-       int                     retval = 0;
-       struct usb_interface    *intf = 0;
-       struct usb_driver       *driver = 0;
 
        /* get input parameters and alloc buffer */
        if (copy_from_user(&ctrl, arg, sizeof (ctrl)))
@@ -1109,17 +1345,23 @@
                }
        }
 
-       if (!connected(ps->dev)) {
-               if (buf)
-                       kfree(buf);
-               return -ENODEV;
+       down(&dev->serialize);
+       if (!connected(dev)) {
+               ret = -ENODEV;
+               goto out;
+       }
+
+       if (dev->state != USB_STATE_CONFIGURED) {
+               ret = -ENODEV;
+               goto out;
+       }
+
+       if (!(intf = usb_ifnum_to_if (dev, ctrl.ifno))) {
+               ret = -EINVAL;
+              goto out;
        }
 
-       if (ps->dev->state != USB_STATE_CONFIGURED)
-               retval = -ENODEV;
-       else if (!(intf = usb_ifnum_to_if (ps->dev, ctrl.ifno)))
-               retval = -EINVAL;
-       else switch (ctrl.ioctl_code) {
+       switch (ctrl.ioctl_code) {
 
        /* disconnect kernel driver from interface */
        case USBDEVFS_DISCONNECT:
@@ -1129,7 +1371,7 @@
                        dev_dbg (&intf->dev, "disconnect by usbfs\n");
                        usb_driver_release_interface(driver, intf);
                } else
-                       retval = -ENODATA;
+                       ret = -ENODATA;
                up_write(&usb_bus_type.subsys.rwsem);
                break;
 
@@ -1144,24 +1386,25 @@
                if (intf->dev.driver)
                        driver = to_usb_driver(intf->dev.driver);
                if (driver == 0 || driver->ioctl == 0) {
-                       retval = -ENOTTY;
+                       ret = -ENOTTY;
                } else {
-                       retval = driver->ioctl (intf, ctrl.ioctl_code, buf);
-                       if (retval == -ENOIOCTLCMD)
-                               retval = -ENOTTY;
+                       ret = driver->ioctl (intf, ctrl.ioctl_code, buf);
+                       if (ret == -ENOIOCTLCMD)
+                               ret = -ENOTTY;
                }
                up_read(&usb_bus_type.subsys.rwsem);
        }
 
+       if (ret < 0)
+               goto out;
+
        /* cleanup and return */
-       if (retval >= 0
-                       && (_IOC_DIR (ctrl.ioctl_code) & _IOC_READ) != 0
-                       && size > 0
-                       && copy_to_user (ctrl.data, buf, size) != 0)
-               retval = -EFAULT;
-       if (buf != 0)
-               kfree (buf);
-       return retval;
+       if ((_IOC_DIR (ctrl.ioctl_code) & _IOC_READ) != 0 && size > 0)
+           ret = copy_to_user (ctrl.data, buf, size) ? -EFAULT : 0;
+out:
+       kfree (buf);
+       up(&dev->serialize);
+       return ret;
 }
 
 /*
@@ -1177,11 +1420,6 @@
 
        if (!(file->f_mode & FMODE_WRITE))
                return -EPERM;
-       down(&dev->serialize);
-       if (!connected(dev)) {
-               up(&dev->serialize);
-               return -ENODEV;
-       }
 
        switch (cmd) {
        case USBDEVFS_CONTROL:
@@ -1279,7 +1517,6 @@
                ret = proc_ioctl(ps, (void __user *) arg);
                break;
        }
-       up(&dev->serialize);
        if (ret >= 0)
                inode->i_atime = CURRENT_TIME;
        return ret;


-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training.
Attend Black Hat Briefings & Training, Las Vegas July 24-29 - 
digital self defense, top technical experts, no vendor pitches, 
unmatched networking opportunities. Visit www.blackhat.com
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to