> Here is what I suggest.  I send you a patch that fixes up proc_bulk and
> proc_control as my current patch does, but doesn't push taking dev->serialize
> down into the individual ioctl methods.

How about this?  It's not as minimal as it could be, because it contains some
cleanups/rearrangements from my previous patch.  I could slim it down if you
like, by the end of the week.  There's one part of the patch you may not want:
notice the constant MAX_BUFFER_LENGTH?  proc_submit_urb checks that
bulk urbs have a buffer no bigger than 16384 (same for interrupt urbs), but proc_bulk
didn't check the buffer length at all.  So I added a check to proc_bulk: it now also
rejects urbs with a buffer length bigger than 16384.  Two patches for the price of one,
what a deal!  Recalling a request on the mailing list for an increase in the maximum
buffer size, I made 16384 into the constant MAX_BUFFER_LENGTH, in case you
want to tweak it.

Signed-off-by: Duncan Sands <[EMAIL PROTECTED]>

--- local_tree.orig/drivers/usb/core/devio.c    2004-08-02 23:45:10.000000000 +0200
+++ local_tree/drivers/usb/core/devio.c 2004-08-04 00:36:45.813166381 +0200
@@ -71,6 +71,7 @@
                        dev_info( dev , format , ## arg);       \
        } while (0)
 
+#define MAX_BUFFER_LENGTH      16384
 
 static inline int connected (struct usb_device *dev)
 {
@@ -530,126 +531,202 @@
         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)(tbuf)[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;
                }
        } 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;
                        }
                }
-               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 ", tbuf[j]);
+               printk("\n");
+       }
+       if (!(urb = usb_alloc_urb(0, GFP_KERNEL))) {
+               ret = -ENOMEM;
+               goto out;
+       }
+       if (!(dr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_KERNEL))) {
+               ret = -ENOMEM;
+               goto out;
+       }
+       pipe = (dir_in ? usb_rcvctrlpipe(dev, 0) : 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);
+       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;
+       }
+       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)(tbuf)[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;
+               }
        }
+       ret = length;
+out:
+       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;
 }
 
 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)
+       if ((ret = findintfep(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;
                }
        } 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;
                        }
                }
-               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;
+       }
+       pipe = (dir_in ? usb_rcvbulkpipe(dev, bulk.ep & 0x7f) : usb_sndbulkpipe(dev, 
bulk.ep & 0x7f));
+       if (!usb_maxpacket(dev, pipe, !dir_in)) {
+               ret = -EINVAL;
+               goto out;
+       }
+       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);
+       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;
        }
-       return len2;
+       if (dir_in && length) {
+               if (copy_to_user(bulk.data, tbuf, length)) {
+                       ret = -EFAULT;
+                       goto out;
+               }
+       }
+       ret = length;
+out:
+       usb_free_urb(urb);
+       kfree(tbuf);
+       return ret;
 }
 
 static int proc_resetep(struct dev_state *ps, void __user *arg)
@@ -849,7 +926,7 @@
 
        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;
@@ -891,7 +968,7 @@
                        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;


-------------------------------------------------------
This SF.Net email is sponsored by OSTG. Have you noticed the changes on
Linux.com, ITManagersJournal and NewsForge in the past few weeks? Now,
one more big change to announce. We are now OSTG- Open Source Technology
Group. Come see the changes on the new OSTG site. www.ostg.com
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to