> 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