From f080a51bef2caa9b0f647dc430bc608d5723ac29 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Thu, 20 Feb 2014 10:49:30 -0500 Subject: USB: complain if userspace resets an active endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is an error for a driver to call usb_clear_halt() or usb_reset_endpoint() while there are URBs queued for the endpoint, because the end result is not well defined. At the time the endpoint gets reset, it may or may not be actively running. As far as I know, no kernel drivers do this. But some userspace drivers do, and it seems like a good idea to bring this error to their attention. This patch adds a warning to the kernel log whenever a program invokes the USBDEVFS_CLEAR_HALT or USBDEVFS_RESETEP ioctls at an inappropriate time, and includes the name of the program. This will make it clear that any subsequent errors are not due to the misbehavior of a kernel driver. Signed-off-by: Alan Stern Suggested-by: Bjørn Mork CC: Stanislaw Gruszka Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/devio.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 90e18f6fa2bb..f3ba2e076ee3 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1043,6 +1043,20 @@ static int proc_bulk(struct dev_state *ps, void __user *arg) return ret; } +static void check_reset_of_active_ep(struct usb_device *udev, + unsigned int epnum, char *ioctl_name) +{ + struct usb_host_endpoint **eps; + struct usb_host_endpoint *ep; + + eps = (epnum & USB_DIR_IN) ? udev->ep_in : udev->ep_out; + ep = eps[epnum & 0x0f]; + if (ep && !list_empty(&ep->urb_list)) + dev_warn(&udev->dev, "Process %d (%s) called USBDEVFS_%s for active endpoint 0x%02x\n", + task_pid_nr(current), current->comm, + ioctl_name, epnum); +} + static int proc_resetep(struct dev_state *ps, void __user *arg) { unsigned int ep; @@ -1056,6 +1070,7 @@ static int proc_resetep(struct dev_state *ps, void __user *arg) ret = checkintf(ps, ret); if (ret) return ret; + check_reset_of_active_ep(ps->dev, ep, "RESETEP"); usb_reset_endpoint(ps->dev, ep); return 0; } @@ -1074,6 +1089,7 @@ static int proc_clearhalt(struct dev_state *ps, void __user *arg) ret = checkintf(ps, ret); if (ret) return ret; + check_reset_of_active_ep(ps->dev, ep, "CLEAR_HALT"); if (ep & USB_DIR_IN) pipe = usb_rcvbulkpipe(ps->dev, ep & 0x7f); else -- cgit v1.2.3 From 5ec9c1771ce83a1e2b7ec96ed9f29a9f1b25e71e Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:27 +0200 Subject: usbfs: Kill urbs on interface before doing a set_interface The usb_set_interface documentation says: * Also, drivers must not change altsettings while urbs are scheduled for * endpoints in that interface; all such urbs must first be completed * (perhaps forced by unlinking). For in kernel drivers we trust the drivers to get this right, but we cannot trust userspace to get this right, so enforce it by killing any urbs still pending on the interface. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/devio.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index f3ba2e076ee3..2a95e4e574bb 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1143,6 +1143,9 @@ static int proc_setintf(struct dev_state *ps, void __user *arg) return -EFAULT; if ((ret = checkintf(ps, setintf.interface))) return ret; + + destroy_async_on_interface(ps, setintf.interface); + return usb_set_interface(ps->dev, setintf.interface, setintf.altsetting); } -- cgit v1.2.3 From b2d03eb56e66620a9b27f1a0c2795722087effc9 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:28 +0200 Subject: usbfs: proc_do_submiturb use a local variable for number_of_packets This is a preparation patch for adding support for bulk streams. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/devio.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 2a95e4e574bb..c88d8bfaca8d 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1208,6 +1208,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, struct usb_ctrlrequest *dr = NULL; unsigned int u, totlen, isofrmlen; int i, ret, is_in, num_sgs = 0, ifnum = -1; + int number_of_packets = 0; void *buf; if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP | @@ -1261,7 +1262,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, le16_to_cpup(&dr->wIndex)); if (ret) goto error; - uurb->number_of_packets = 0; uurb->buffer_length = le16_to_cpup(&dr->wLength); uurb->buffer += 8; if ((dr->bRequestType & USB_DIR_IN) && uurb->buffer_length) { @@ -1291,7 +1291,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, uurb->type = USBDEVFS_URB_TYPE_INTERRUPT; goto interrupt_urb; } - uurb->number_of_packets = 0; num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE); if (num_sgs == 1 || num_sgs > ps->dev->bus->sg_tablesize) num_sgs = 0; @@ -1301,7 +1300,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, if (!usb_endpoint_xfer_int(&ep->desc)) return -EINVAL; interrupt_urb: - uurb->number_of_packets = 0; break; case USBDEVFS_URB_TYPE_ISO: @@ -1311,15 +1309,16 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, return -EINVAL; if (!usb_endpoint_xfer_isoc(&ep->desc)) return -EINVAL; + number_of_packets = uurb->number_of_packets; isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) * - uurb->number_of_packets; + number_of_packets; if (!(isopkt = kmalloc(isofrmlen, GFP_KERNEL))) return -ENOMEM; if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) { ret = -EFAULT; goto error; } - for (totlen = u = 0; u < uurb->number_of_packets; u++) { + for (totlen = u = 0; u < number_of_packets; u++) { /* * arbitrary limit need for USB 3.0 * bMaxBurst (0~15 allowed, 1~16 packets) @@ -1350,7 +1349,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, ret = -EFAULT; goto error; } - as = alloc_async(uurb->number_of_packets); + as = alloc_async(number_of_packets); if (!as) { ret = -ENOMEM; goto error; @@ -1444,7 +1443,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, as->urb->setup_packet = (unsigned char *)dr; dr = NULL; as->urb->start_frame = uurb->start_frame; - as->urb->number_of_packets = uurb->number_of_packets; + as->urb->number_of_packets = number_of_packets; if (uurb->type == USBDEVFS_URB_TYPE_ISO || ps->dev->speed == USB_SPEED_HIGH) as->urb->interval = 1 << min(15, ep->desc.bInterval - 1); @@ -1452,7 +1451,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, as->urb->interval = ep->desc.bInterval; as->urb->context = as; as->urb->complete = async_completed; - for (totlen = u = 0; u < uurb->number_of_packets; u++) { + for (totlen = u = 0; u < number_of_packets; u++) { as->urb->iso_frame_desc[u].offset = totlen; as->urb->iso_frame_desc[u].length = isopkt[u].length; totlen += isopkt[u].length; -- cgit v1.2.3 From 948cd8c18c466fdcbe707bb2a42a148796bfccdd Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:29 +0200 Subject: usbfs: Add support for bulk stream ids This patch makes it possible to specify a bulk stream id when submitting an urb using the async usbfs API. It overloads the number_of_packets usbdevfs_urb field for this. This is not pretty, but given other constraints it is the best we can do. The reasoning leading to this goes as follows: 1) We want to support bulk streams in the usbfs API 2) We do not want to extend the usbdevfs_urb struct with a new member, as that would mean defining new ioctl numbers for all async API ioctls + adding compat versions for the old ones (times 2 for 32 bit support) 3) 1 + 2 means we need to re-use an existing field 4) number_of_packets is only used for isoc urbs, and streams are bulk only so it is the best (and only) candidate for re-using Note that: 1) This patch only uses number_of_packets as stream_id if the app has actually allocated streams on the ep, so that old apps which may have garbage in there (as it was unused until now in the bulk case), will not break 2) This patch does not add support for allocating / freeing bulk-streams, that is done in a follow up patch Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/devio.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index c88d8bfaca8d..d7571a63181d 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1209,6 +1209,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, unsigned int u, totlen, isofrmlen; int i, ret, is_in, num_sgs = 0, ifnum = -1; int number_of_packets = 0; + unsigned int stream_id = 0; void *buf; if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP | @@ -1294,6 +1295,8 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE); if (num_sgs == 1 || num_sgs > ps->dev->bus->sg_tablesize) num_sgs = 0; + if (ep->streams) + stream_id = uurb->stream_id; break; case USBDEVFS_URB_TYPE_INTERRUPT: @@ -1444,6 +1447,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, dr = NULL; as->urb->start_frame = uurb->start_frame; as->urb->number_of_packets = number_of_packets; + as->urb->stream_id = stream_id; if (uurb->type == USBDEVFS_URB_TYPE_ISO || ps->dev->speed == USB_SPEED_HIGH) as->urb->interval = 1 << min(15, ep->desc.bInterval - 1); -- cgit v1.2.3 From 2fec32b06e374642802f7fb4f5350317cd14732b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:30 +0200 Subject: usbfs: Add ep_to_host_endpoint helper function Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/devio.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index d7571a63181d..502974b4deb5 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -769,6 +769,15 @@ static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, return ret; } +static struct usb_host_endpoint *ep_to_host_endpoint(struct usb_device *dev, + unsigned char ep) +{ + if (ep & USB_ENDPOINT_DIR_MASK) + return dev->ep_in[ep & USB_ENDPOINT_NUMBER_MASK]; + else + return dev->ep_out[ep & USB_ENDPOINT_NUMBER_MASK]; +} + static int match_devt(struct device *dev, void *data) { return dev->devt == (dev_t) (unsigned long) data; @@ -1230,15 +1239,10 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, if (ret) return ret; } - if ((uurb->endpoint & USB_ENDPOINT_DIR_MASK) != 0) { - is_in = 1; - ep = ps->dev->ep_in[uurb->endpoint & USB_ENDPOINT_NUMBER_MASK]; - } else { - is_in = 0; - ep = ps->dev->ep_out[uurb->endpoint & USB_ENDPOINT_NUMBER_MASK]; - } + ep = ep_to_host_endpoint(ps->dev, uurb->endpoint); if (!ep) return -ENOENT; + is_in = (uurb->endpoint & USB_ENDPOINT_DIR_MASK) != 0; u = 0; switch(uurb->type) { -- cgit v1.2.3 From bcf7f6e39335af4f03da8c26a98185fd49754fcc Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:31 +0200 Subject: usbfs: Add support for allocating / freeing streams This allows userspace to use bulk-streams, just like in kernel drivers, see Documentation/usb/bulk-streams.txt for details on the in kernel API. This is exported pretty much one on one to userspace. To use streams an app must first make a USBDEVFS_ALLOC_STREAMS ioctl, on success this will return the number of streams available (which may be less then requested). If there are n streams the app can then submit usbdevfs_urb-s with their stream_id member set to 1-n to use a specific stream. IE if USBDEVFS_ALLOC_STREAMS returns 4 then stream_id 1-4 can be used. When the app is done using streams it should call USBDEVFS_FREE_STREAMS Note applications are advised to use libusb rather then using the usbdevfs api directly. The latest version of libusb has support for streams. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/devio.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 502974b4deb5..12401ee4ba0e 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -778,6 +778,79 @@ static struct usb_host_endpoint *ep_to_host_endpoint(struct usb_device *dev, return dev->ep_out[ep & USB_ENDPOINT_NUMBER_MASK]; } +static int parse_usbdevfs_streams(struct dev_state *ps, + struct usbdevfs_streams __user *streams, + unsigned int *num_streams_ret, + unsigned int *num_eps_ret, + struct usb_host_endpoint ***eps_ret, + struct usb_interface **intf_ret) +{ + unsigned int i, num_streams, num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf = NULL; + unsigned char ep; + int ifnum, ret; + + if (get_user(num_streams, &streams->num_streams) || + get_user(num_eps, &streams->num_eps)) + return -EFAULT; + + if (num_eps < 1 || num_eps > USB_MAXENDPOINTS) + return -EINVAL; + + /* The XHCI controller allows max 2 ^ 16 streams */ + if (num_streams_ret && (num_streams < 2 || num_streams > 65536)) + return -EINVAL; + + eps = kmalloc(num_eps * sizeof(*eps), GFP_KERNEL); + if (!eps) + return -ENOMEM; + + for (i = 0; i < num_eps; i++) { + if (get_user(ep, &streams->eps[i])) { + ret = -EFAULT; + goto error; + } + eps[i] = ep_to_host_endpoint(ps->dev, ep); + if (!eps[i]) { + ret = -EINVAL; + goto error; + } + + /* usb_alloc/free_streams operate on an usb_interface */ + ifnum = findintfep(ps->dev, ep); + if (ifnum < 0) { + ret = ifnum; + goto error; + } + + if (i == 0) { + ret = checkintf(ps, ifnum); + if (ret < 0) + goto error; + intf = usb_ifnum_to_if(ps->dev, ifnum); + } else { + /* Verify all eps belong to the same interface */ + if (ifnum != intf->altsetting->desc.bInterfaceNumber) { + ret = -EINVAL; + goto error; + } + } + } + + if (num_streams_ret) + *num_streams_ret = num_streams; + *num_eps_ret = num_eps; + *eps_ret = eps; + *intf_ret = intf; + + return 0; + +error: + kfree(eps); + return ret; +} + static int match_devt(struct device *dev, void *data) { return dev->devt == (dev_t) (unsigned long) data; @@ -2009,6 +2082,45 @@ static int proc_disconnect_claim(struct dev_state *ps, void __user *arg) return claimintf(ps, dc.interface); } +static int proc_alloc_streams(struct dev_state *ps, void __user *arg) +{ + unsigned num_streams, num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf; + int r; + + r = parse_usbdevfs_streams(ps, arg, &num_streams, &num_eps, + &eps, &intf); + if (r) + return r; + + destroy_async_on_interface(ps, + intf->altsetting[0].desc.bInterfaceNumber); + + r = usb_alloc_streams(intf, eps, num_eps, num_streams, GFP_KERNEL); + kfree(eps); + return r; +} + +static int proc_free_streams(struct dev_state *ps, void __user *arg) +{ + unsigned num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf; + int r; + + r = parse_usbdevfs_streams(ps, arg, NULL, &num_eps, &eps, &intf); + if (r) + return r; + + destroy_async_on_interface(ps, + intf->altsetting[0].desc.bInterfaceNumber); + + r = usb_free_streams(intf, eps, num_eps, GFP_KERNEL); + kfree(eps); + return r; +} + /* * NOTE: All requests here that have interface numbers as parameters * are assuming that somehow the configuration has been prevented from @@ -2185,6 +2297,12 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd, case USBDEVFS_DISCONNECT_CLAIM: ret = proc_disconnect_claim(ps, p); break; + case USBDEVFS_ALLOC_STREAMS: + ret = proc_alloc_streams(ps, p); + break; + case USBDEVFS_FREE_STREAMS: + ret = proc_free_streams(ps, p); + break; } usb_unlock_device(dev); if (ret >= 0) -- cgit v1.2.3