Pull USB fixes from Greg KH: "Here are some small USB fixes for a bunch of warnings/errors that the syzbot has been finding with it's new-found ability to stress-test the USB layer. All of these are tiny, but fix real issues, and are marked for stable as well. All of these have had lots of testing in linux-next as well" * tag 'usb-5.1-rc8' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb: USB: w1 ds2490: Fix bug caused by improper use of altsetting array USB: yurex: Fix protection fault after device removal usb: usbip: fix isoc packet num validation in get_pipe USB: core: Fix bug caused by duplicate interface PM usage counter USB: dummy-hcd: Fix failure to give back unlinked URBs USB: core: Fix unterminated string returned by usb_string()
Unified · Split
| @@ -370,11 +370,15 @@ autosuspend the interface's device. When the usage counter is = 0 · 369 unmodified lines | ||
| 370 | 370 | then the interface is considered to be idle, and the kernel may |
| 371 | 371 | autosuspend the device. |
| 372 | 372 | |
| 373 | −Drivers need not be concerned about balancing changes to the usage | |
| 374 | −counter; the USB core will undo any remaining "get"s when a driver | |
| 375 | −is unbound from its interface. As a corollary, drivers must not call | |
| 376 | −any of the ``usb_autopm_*`` functions after their ``disconnect`` | |
| 377 | −routine has returned. | |
| 373 | +Drivers must be careful to balance their overall changes to the usage | |
| 374 | +counter. Unbalanced "get"s will remain in effect when a driver is | |
| 375 | +unbound from its interface, preventing the device from going into | |
| 376 | +runtime suspend should the interface be bound to a driver again. On | |
| 377 | +the other hand, drivers are allowed to achieve this balance by calling | |
| 378 | +the ``usb_autopm_*`` functions even after their ``disconnect`` routine | |
| 379 | +has returned -- say from within a work-queue routine -- provided they | |
| 380 | +retain an active reference to the interface (via ``usb_get_intf`` and | |
| 381 | +``usb_put_intf``). | |
| 378 | 382 | |
| 379 | 383 | Drivers using the async routines are responsible for their own |
| 380 | 384 | synchronization and mutual exclusion. |
| @@ -473,11 +473,6 @@ static int usb_unbind_interface(struct device *dev) · 472 unmodified lines | ||
| 473 | 473 | pm_runtime_disable(dev); |
| 474 | 474 | pm_runtime_set_suspended(dev); |
| 475 | 475 | |
| 476 | − /* Undo any residual pm_autopm_get_interface_* calls */ | |
| 477 | − for (r = atomic_read(&intf->pm_usage_cnt); r > 0; --r) | |
| 478 | − usb_autopm_put_interface_no_suspend(intf); | |
| 479 | − atomic_set(&intf->pm_usage_cnt, 0); | |
| 480 | − | |
| 481 | 476 | if (!error) |
| 482 | 477 | usb_autosuspend_device(udev); |
| 483 | 478 | |
| @@ -1633,7 +1628,6 @@ void usb_autopm_put_interface(struct usb_interface *intf) · 1149 unmodified lines | ||
| 1633 | 1628 | int status; |
| 1634 | 1629 | |
| 1635 | 1630 | usb_mark_last_busy(udev); |
| 1636 | − atomic_dec(&intf->pm_usage_cnt); | |
| 1637 | 1631 | status = pm_runtime_put_sync(&intf->dev); |
| 1638 | 1632 | dev_vdbg(&intf->dev, "%s: cnt %d -> %d\n", |
| 1639 | 1633 | __func__, atomic_read(&intf->dev.power.usage_count), |
| @@ -1662,7 +1656,6 @@ void usb_autopm_put_interface_async(struct usb_interface *intf) · 22 unmodified lines | ||
| 1662 | 1656 | int status; |
| 1663 | 1657 | |
| 1664 | 1658 | usb_mark_last_busy(udev); |
| 1665 | − atomic_dec(&intf->pm_usage_cnt); | |
| 1666 | 1659 | status = pm_runtime_put(&intf->dev); |
| 1667 | 1660 | dev_vdbg(&intf->dev, "%s: cnt %d -> %d\n", |
| 1668 | 1661 | __func__, atomic_read(&intf->dev.power.usage_count), |
| @@ -1684,7 +1677,6 @@ void usb_autopm_put_interface_no_suspend(struct usb_interface *intf) · 15 unmodified lines | ||
| 1684 | 1677 | struct usb_device *udev = interface_to_usbdev(intf); |
| 1685 | 1678 | |
| 1686 | 1679 | usb_mark_last_busy(udev); |
| 1687 | − atomic_dec(&intf->pm_usage_cnt); | |
| 1688 | 1680 | pm_runtime_put_noidle(&intf->dev); |
| 1689 | 1681 | } |
| 1690 | 1682 | EXPORT_SYMBOL_GPL(usb_autopm_put_interface_no_suspend); |
| @@ -1715,8 +1707,6 @@ int usb_autopm_get_interface(struct usb_interface *intf) · 24 unmodified lines | ||
| 1715 | 1707 | status = pm_runtime_get_sync(&intf->dev); |
| 1716 | 1708 | if (status < 0) |
| 1717 | 1709 | pm_runtime_put_sync(&intf->dev); |
| 1718 | − else | |
| 1719 | − atomic_inc(&intf->pm_usage_cnt); | |
| 1720 | 1710 | dev_vdbg(&intf->dev, "%s: cnt %d -> %d\n", |
| 1721 | 1711 | __func__, atomic_read(&intf->dev.power.usage_count), |
| 1722 | 1712 | status); |
| @@ -1750,8 +1740,6 @@ int usb_autopm_get_interface_async(struct usb_interface *intf) · 27 unmodified lines | ||
| 1750 | 1740 | status = pm_runtime_get(&intf->dev); |
| 1751 | 1741 | if (status < 0 && status != -EINPROGRESS) |
| 1752 | 1742 | pm_runtime_put_noidle(&intf->dev); |
| 1753 | − else | |
| 1754 | − atomic_inc(&intf->pm_usage_cnt); | |
| 1755 | 1743 | dev_vdbg(&intf->dev, "%s: cnt %d -> %d\n", |
| 1756 | 1744 | __func__, atomic_read(&intf->dev.power.usage_count), |
| 1757 | 1745 | status); |
| @@ -1775,7 +1763,6 @@ void usb_autopm_get_interface_no_resume(struct usb_interface *intf) · 17 unmodified lines | ||
| 1775 | 1763 | struct usb_device *udev = interface_to_usbdev(intf); |
| 1776 | 1764 | |
| 1777 | 1765 | usb_mark_last_busy(udev); |
| 1778 | − atomic_inc(&intf->pm_usage_cnt); | |
| 1779 | 1766 | pm_runtime_get_noresume(&intf->dev); |
| 1780 | 1767 | } |
| 1781 | 1768 | EXPORT_SYMBOL_GPL(usb_autopm_get_interface_no_resume); |
| @@ -820,9 +820,11 @@ int usb_string(struct usb_device *dev, int index, char *buf, size_t size) · 819 unmodified lines | ||
| 820 | 820 | |
| 821 | 821 | if (dev->state == USB_STATE_SUSPENDED) |
| 822 | 822 | return -EHOSTUNREACH; |
| 823 | − if (size <= 0 || !buf || !index) | |
| 823 | + if (size <= 0 || !buf) | |
| 824 | 824 | return -EINVAL; |
| 825 | 825 | buf[0] = 0; |
| 826 | + if (index <= 0 || index >= 256) | |
| 827 | + return -EINVAL; | |
| 826 | 828 | tbuf = kmalloc(256, GFP_NOIO); |
| 827 | 829 | if (!tbuf) |
| 828 | 830 | return -ENOMEM; |
| @@ -979,8 +979,18 @@ static int dummy_udc_start(struct usb_gadget *g, · 978 unmodified lines | ||
| 979 | 979 | struct dummy_hcd *dum_hcd = gadget_to_dummy_hcd(g); |
| 980 | 980 | struct dummy *dum = dum_hcd->dum; |
| 981 | 981 | |
| 982 | − if (driver->max_speed == USB_SPEED_UNKNOWN) | |
| 982 | + switch (g->speed) { | |
| 983 | + /* All the speeds we support */ | |
| 984 | + case USB_SPEED_LOW: | |
| 985 | + case USB_SPEED_FULL: | |
| 986 | + case USB_SPEED_HIGH: | |
| 987 | + case USB_SPEED_SUPER: | |
| 988 | + break; | |
| 989 | + default: | |
| 990 | + dev_err(dummy_dev(dum_hcd), "Unsupported driver max speed %d\n", | |
| 991 | + driver->max_speed); | |
| 983 | 992 | return -EINVAL; |
| 993 | + } | |
| 984 | 994 | |
| 985 | 995 | /* |
| 986 | 996 | * SLAVE side init ... the layer above hardware, which |
| @@ -1784,9 +1794,10 @@ static void dummy_timer(struct timer_list *t) · 797 unmodified lines | ||
| 1784 | 1794 | /* Bus speed is 500000 bytes/ms, so use a little less */ |
| 1785 | 1795 | total = 490000; |
| 1786 | 1796 | break; |
| 1787 | − default: | |
| 1797 | + default: /* Can't happen */ | |
| 1788 | 1798 | dev_err(dummy_dev(dum_hcd), "bogus device speed\n"); |
| 1789 | − return; | |
| 1799 | + total = 0; | |
| 1800 | + break; | |
| 1790 | 1801 | } |
| 1791 | 1802 | |
| 1792 | 1803 | /* FIXME if HZ != 1000 this will probably misbehave ... */ |
| @@ -1828,7 +1839,7 @@ restart: · 35 unmodified lines | ||
| 1828 | 1839 | |
| 1829 | 1840 | /* Used up this frame's bandwidth? */ |
| 1830 | 1841 | if (total <= 0) |
| 1831 | − break; | |
| 1842 | + continue; | |
| 1832 | 1843 | |
| 1833 | 1844 | /* find the gadget's ep for this request (if configured) */ |
| 1834 | 1845 | address = usb_pipeendpoint (urb->pipe); |
| @@ -314,6 +314,7 @@ static void yurex_disconnect(struct usb_interface *interface) · 313 unmodified lines | ||
| 314 | 314 | usb_deregister_dev(interface, &yurex_class); |
| 315 | 315 | |
| 316 | 316 | /* prevent more I/O from starting */ |
| 317 | + usb_poison_urb(dev->urb); | |
| 317 | 318 | mutex_lock(&dev->io_mutex); |
| 318 | 319 | dev->interface = NULL; |
| 319 | 320 | mutex_unlock(&dev->io_mutex); |
| @@ -763,18 +763,16 @@ static void rts51x_suspend_timer_fn(struct timer_list *t) · 762 unmodified lines | ||
| 763 | 763 | break; |
| 764 | 764 | case RTS51X_STAT_IDLE: |
| 765 | 765 | case RTS51X_STAT_SS: |
| 766 | − usb_stor_dbg(us, "RTS51X_STAT_SS, intf->pm_usage_cnt:%d, power.usage:%d\n", | |
| 767 | − atomic_read(&us->pusb_intf->pm_usage_cnt), | |
| 766 | + usb_stor_dbg(us, "RTS51X_STAT_SS, power.usage:%d\n", | |
| 768 | 767 | atomic_read(&us->pusb_intf->dev.power.usage_count)); |
| 769 | 768 | |
| 770 | − if (atomic_read(&us->pusb_intf->pm_usage_cnt) > 0) { | |
| 769 | + if (atomic_read(&us->pusb_intf->dev.power.usage_count) > 0) { | |
| 771 | 770 | usb_stor_dbg(us, "Ready to enter SS state\n"); |
| 772 | 771 | rts51x_set_stat(chip, RTS51X_STAT_SS); |
| 773 | 772 | /* ignore mass storage interface's children */ |
| 774 | 773 | pm_suspend_ignore_children(&us->pusb_intf->dev, true); |
| 775 | 774 | usb_autopm_put_interface_async(us->pusb_intf); |
| 776 | − usb_stor_dbg(us, "RTS51X_STAT_SS 01, intf->pm_usage_cnt:%d, power.usage:%d\n", | |
| 777 | − atomic_read(&us->pusb_intf->pm_usage_cnt), | |
| 775 | + usb_stor_dbg(us, "RTS51X_STAT_SS 01, power.usage:%d\n", | |
| 778 | 776 | atomic_read(&us->pusb_intf->dev.power.usage_count)); |
| 779 | 777 | } |
| 780 | 778 | break; |
| @@ -807,11 +805,10 @@ static void rts51x_invoke_transport(struct scsi_cmnd *srb, struct us_data *us) · 26 unmodified lines | ||
| 807 | 805 | int ret; |
| 808 | 806 | |
| 809 | 807 | if (working_scsi(srb)) { |
| 810 | − usb_stor_dbg(us, "working scsi, intf->pm_usage_cnt:%d, power.usage:%d\n", | |
| 811 | − atomic_read(&us->pusb_intf->pm_usage_cnt), | |
| 808 | + usb_stor_dbg(us, "working scsi, power.usage:%d\n", | |
| 812 | 809 | atomic_read(&us->pusb_intf->dev.power.usage_count)); |
| 813 | 810 | |
| 814 | − if (atomic_read(&us->pusb_intf->pm_usage_cnt) <= 0) { | |
| 811 | + if (atomic_read(&us->pusb_intf->dev.power.usage_count) <= 0) { | |
| 815 | 812 | ret = usb_autopm_get_interface(us->pusb_intf); |
| 816 | 813 | usb_stor_dbg(us, "working scsi, ret=%d\n", ret); |
| 817 | 814 | } |
| @@ -361,16 +361,10 @@ static int get_pipe(struct stub_device *sdev, struct usbip_header *pdu) · 360 unmodified lines | ||
| 361 | 361 | } |
| 362 | 362 | |
| 363 | 363 | if (usb_endpoint_xfer_isoc(epd)) { |
| 364 | − /* validate packet size and number of packets */ | |
| 365 | − unsigned int maxp, packets, bytes; | |
| 366 | − | |
| 367 | − maxp = usb_endpoint_maxp(epd); | |
| 368 | − maxp *= usb_endpoint_maxp_mult(epd); | |
| 369 | − bytes = pdu->u.cmd_submit.transfer_buffer_length; | |
| 370 | − packets = DIV_ROUND_UP(bytes, maxp); | |
| 371 | − | |
| 364 | + /* validate number of packets */ | |
| 372 | 365 | if (pdu->u.cmd_submit.number_of_packets < 0 || |
| 373 | − pdu->u.cmd_submit.number_of_packets > packets) { | |
| 366 | + pdu->u.cmd_submit.number_of_packets > | |
| 367 | + USBIP_MAX_ISO_PACKETS) { | |
| 374 | 368 | dev_err(&sdev->udev->dev, |
| 375 | 369 | "CMD_SUBMIT: isoc invalid num packets %d\n", |
| 376 | 370 | pdu->u.cmd_submit.number_of_packets); |
| @@ -121,6 +121,13 @@ extern struct device_attribute dev_attr_usbip_debug; · 120 unmodified lines | ||
| 121 | 121 | #define USBIP_DIR_OUT 0x00 |
| 122 | 122 | #define USBIP_DIR_IN 0x01 |
| 123 | 123 | |
| 124 | +/* | |
| 125 | + * Arbitrary limit for the maximum number of isochronous packets in an URB, | |
| 126 | + * compare for example the uhci_submit_isochronous function in | |
| 127 | + * drivers/usb/host/uhci-q.c | |
| 128 | + */ | |
| 129 | +#define USBIP_MAX_ISO_PACKETS 1024 | |
| 130 | + | |
| 124 | 131 | /** |
| 125 | 132 | * struct usbip_header_basic - data pertinent to every request |
| 126 | 133 | * @command: the usbip request type |
| @@ -1016,15 +1016,15 @@ static int ds_probe(struct usb_interface *intf, · 1015 unmodified lines | ||
| 1016 | 1016 | /* alternative 3, 1ms interrupt (greatly speeds search), 64 byte bulk */ |
| 1017 | 1017 | alt = 3; |
| 1018 | 1018 | err = usb_set_interface(dev->udev, |
| 1019 | − intf->altsetting[alt].desc.bInterfaceNumber, alt); | |
| 1019 | + intf->cur_altsetting->desc.bInterfaceNumber, alt); | |
| 1020 | 1020 | if (err) { |
| 1021 | 1021 | dev_err(&dev->udev->dev, "Failed to set alternative setting %d " |
| 1022 | 1022 | "for %d interface: err=%d.\n", alt, |
| 1023 | − intf->altsetting[alt].desc.bInterfaceNumber, err); | |
| 1023 | + intf->cur_altsetting->desc.bInterfaceNumber, err); | |
| 1024 | 1024 | goto err_out_clear; |
| 1025 | 1025 | } |
| 1026 | 1026 | |
| 1027 | − iface_desc = &intf->altsetting[alt]; | |
| 1027 | + iface_desc = intf->cur_altsetting; | |
| 1028 | 1028 | if (iface_desc->desc.bNumEndpoints != NUM_EP-1) { |
| 1029 | 1029 | pr_info("Num endpoints=%d. It is not DS9490R.\n", |
| 1030 | 1030 | iface_desc->desc.bNumEndpoints); |
| @@ -200,7 +200,6 @@ usb_find_last_int_out_endpoint(struct usb_host_interface *alt, · 199 unmodified lines | ||
| 200 | 200 | * @dev: driver model's view of this device |
| 201 | 201 | * @usb_dev: if an interface is bound to the USB major, this will point |
| 202 | 202 | * to the sysfs representation for that device. |
| 203 | − * @pm_usage_cnt: PM usage counter for this interface | |
| 204 | 203 | * @reset_ws: Used for scheduling resets from atomic context. |
| 205 | 204 | * @resetting_device: USB core reset the device, so use alt setting 0 as |
| 206 | 205 | * current; needs bandwidth alloc after reset. |
| @@ -257,7 +256,6 @@ struct usb_interface { · 50 unmodified lines | ||
| 257 | 256 | |
| 258 | 257 | struct device dev; /* interface specific device info */ |
| 259 | 258 | struct device *usb_dev; |
| 260 | − atomic_t pm_usage_cnt; /* usage counter for autosuspend */ | |
| 261 | 259 | struct work_struct reset_ws; /* for resets in atomic context */ |
| 262 | 260 | }; |
| 263 | 261 | #define to_usb_interface(d) container_of(d, struct usb_interface, dev) |