From: Greg Kroah-Hartman Date: Sun, 5 May 2019 13:41:17 +0000 (+0200) Subject: 4.4-stable patches X-Git-Tag: v4.9.174~37 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7a0a7a4a8ab356df68f342d6113e1041d7391c3c;p=thirdparty%2Fkernel%2Fstable-queue.git 4.4-stable patches added patches: usb-core-fix-bug-caused-by-duplicate-interface-pm-usage-counter.patch usb-core-fix-unterminated-string-returned-by-usb_string.patch usb-w1-ds2490-fix-bug-caused-by-improper-use-of-altsetting-array.patch usb-yurex-fix-protection-fault-after-device-removal.patch --- diff --git a/queue-4.4/series b/queue-4.4/series index 0d51d544043..8e5517d7cf3 100644 --- a/queue-4.4/series +++ b/queue-4.4/series @@ -113,3 +113,7 @@ ipv6-flowlabel-wait-rcu-grace-period-before-put_pid.patch ipv6-invert-flowlabel-sharing-check-in-process-and-user-mode.patch bnxt_en-improve-multicast-address-setup-logic.patch packet-validate-msg_namelen-in-send-directly.patch +usb-yurex-fix-protection-fault-after-device-removal.patch +usb-w1-ds2490-fix-bug-caused-by-improper-use-of-altsetting-array.patch +usb-core-fix-unterminated-string-returned-by-usb_string.patch +usb-core-fix-bug-caused-by-duplicate-interface-pm-usage-counter.patch diff --git a/queue-4.4/usb-core-fix-bug-caused-by-duplicate-interface-pm-usage-counter.patch b/queue-4.4/usb-core-fix-bug-caused-by-duplicate-interface-pm-usage-counter.patch new file mode 100644 index 00000000000..bc0c8ada2e6 --- /dev/null +++ b/queue-4.4/usb-core-fix-bug-caused-by-duplicate-interface-pm-usage-counter.patch @@ -0,0 +1,212 @@ +From c2b71462d294cf517a0bc6e4fd6424d7cee5596f Mon Sep 17 00:00:00 2001 +From: Alan Stern +Date: Fri, 19 Apr 2019 13:52:38 -0400 +Subject: USB: core: Fix bug caused by duplicate interface PM usage counter + +From: Alan Stern + +commit c2b71462d294cf517a0bc6e4fd6424d7cee5596f upstream. + +The syzkaller fuzzer reported a bug in the USB hub driver which turned +out to be caused by a negative runtime-PM usage counter. This allowed +a hub to be runtime suspended at a time when the driver did not expect +it. The symptom is a WARNING issued because the hub's status URB is +submitted while it is already active: + + URB 0000000031fb463e submitted while active + WARNING: CPU: 0 PID: 2917 at drivers/usb/core/urb.c:363 + +The negative runtime-PM usage count was caused by an unfortunate +design decision made when runtime PM was first implemented for USB. +At that time, USB class drivers were allowed to unbind from their +interfaces without balancing the usage counter (i.e., leaving it with +a positive count). The core code would take care of setting the +counter back to 0 before allowing another driver to bind to the +interface. + +Later on when runtime PM was implemented for the entire kernel, the +opposite decision was made: Drivers were required to balance their +runtime-PM get and put calls. In order to maintain backward +compatibility, however, the USB subsystem adapted to the new +implementation by keeping an independent usage counter for each +interface and using it to automatically adjust the normal usage +counter back to 0 whenever a driver was unbound. + +This approach involves duplicating information, but what is worse, it +doesn't work properly in cases where a USB class driver delays +decrementing the usage counter until after the driver's disconnect() +routine has returned and the counter has been adjusted back to 0. +Doing so would cause the usage counter to become negative. There's +even a warning about this in the USB power management documentation! + +As it happens, this is exactly what the hub driver does. The +kick_hub_wq() routine increments the runtime-PM usage counter, and the +corresponding decrement is carried out by hub_event() in the context +of the hub_wq work-queue thread. This work routine may sometimes run +after the driver has been unbound from its interface, and when it does +it causes the usage counter to go negative. + +It is not possible for hub_disconnect() to wait for a pending +hub_event() call to finish, because hub_disconnect() is called with +the device lock held and hub_event() acquires that lock. The only +feasible fix is to reverse the original design decision: remove the +duplicate interface-specific usage counter and require USB drivers to +balance their runtime PM gets and puts. As far as I know, all +existing drivers currently do this. + +Signed-off-by: Alan Stern +Reported-and-tested-by: syzbot+7634edaea4d0b341c625@syzkaller.appspotmail.com +CC: +Signed-off-by: Greg Kroah-Hartman +Signed-off-by: Greg Kroah-Hartman + +--- + Documentation/usb/power-management.txt | 14 +++++++++----- + drivers/usb/core/driver.c | 13 ------------- + drivers/usb/storage/realtek_cr.c | 13 +++++-------- + include/linux/usb.h | 2 -- + 4 files changed, 14 insertions(+), 28 deletions(-) + +--- a/Documentation/usb/power-management.txt ++++ b/Documentation/usb/power-management.txt +@@ -365,11 +365,15 @@ autosuspend the interface's device. Whe + then the interface is considered to be idle, and the kernel may + autosuspend the device. + +-Drivers need not be concerned about balancing changes to the usage +-counter; the USB core will undo any remaining "get"s when a driver +-is unbound from its interface. As a corollary, drivers must not call +-any of the usb_autopm_* functions after their disconnect() routine has +-returned. ++Drivers must be careful to balance their overall changes to the usage ++counter. Unbalanced "get"s will remain in effect when a driver is ++unbound from its interface, preventing the device from going into ++runtime suspend should the interface be bound to a driver again. On ++the other hand, drivers are allowed to achieve this balance by calling ++the ``usb_autopm_*`` functions even after their ``disconnect`` routine ++has returned -- say from within a work-queue routine -- provided they ++retain an active reference to the interface (via ``usb_get_intf`` and ++``usb_put_intf``). + + Drivers using the async routines are responsible for their own + synchronization and mutual exclusion. +--- a/drivers/usb/core/driver.c ++++ b/drivers/usb/core/driver.c +@@ -470,11 +470,6 @@ static int usb_unbind_interface(struct d + pm_runtime_disable(dev); + pm_runtime_set_suspended(dev); + +- /* Undo any residual pm_autopm_get_interface_* calls */ +- for (r = atomic_read(&intf->pm_usage_cnt); r > 0; --r) +- usb_autopm_put_interface_no_suspend(intf); +- atomic_set(&intf->pm_usage_cnt, 0); +- + if (!error) + usb_autosuspend_device(udev); + +@@ -1625,7 +1620,6 @@ void usb_autopm_put_interface(struct usb + int status; + + usb_mark_last_busy(udev); +- atomic_dec(&intf->pm_usage_cnt); + status = pm_runtime_put_sync(&intf->dev); + dev_vdbg(&intf->dev, "%s: cnt %d -> %d\n", + __func__, atomic_read(&intf->dev.power.usage_count), +@@ -1654,7 +1648,6 @@ void usb_autopm_put_interface_async(stru + int status; + + usb_mark_last_busy(udev); +- atomic_dec(&intf->pm_usage_cnt); + status = pm_runtime_put(&intf->dev); + dev_vdbg(&intf->dev, "%s: cnt %d -> %d\n", + __func__, atomic_read(&intf->dev.power.usage_count), +@@ -1676,7 +1669,6 @@ void usb_autopm_put_interface_no_suspend + struct usb_device *udev = interface_to_usbdev(intf); + + usb_mark_last_busy(udev); +- atomic_dec(&intf->pm_usage_cnt); + pm_runtime_put_noidle(&intf->dev); + } + EXPORT_SYMBOL_GPL(usb_autopm_put_interface_no_suspend); +@@ -1707,8 +1699,6 @@ int usb_autopm_get_interface(struct usb_ + status = pm_runtime_get_sync(&intf->dev); + if (status < 0) + pm_runtime_put_sync(&intf->dev); +- else +- atomic_inc(&intf->pm_usage_cnt); + dev_vdbg(&intf->dev, "%s: cnt %d -> %d\n", + __func__, atomic_read(&intf->dev.power.usage_count), + status); +@@ -1742,8 +1732,6 @@ int usb_autopm_get_interface_async(struc + status = pm_runtime_get(&intf->dev); + if (status < 0 && status != -EINPROGRESS) + pm_runtime_put_noidle(&intf->dev); +- else +- atomic_inc(&intf->pm_usage_cnt); + dev_vdbg(&intf->dev, "%s: cnt %d -> %d\n", + __func__, atomic_read(&intf->dev.power.usage_count), + status); +@@ -1767,7 +1755,6 @@ void usb_autopm_get_interface_no_resume( + struct usb_device *udev = interface_to_usbdev(intf); + + usb_mark_last_busy(udev); +- atomic_inc(&intf->pm_usage_cnt); + pm_runtime_get_noresume(&intf->dev); + } + EXPORT_SYMBOL_GPL(usb_autopm_get_interface_no_resume); +--- a/drivers/usb/storage/realtek_cr.c ++++ b/drivers/usb/storage/realtek_cr.c +@@ -772,18 +772,16 @@ static void rts51x_suspend_timer_fn(unsi + break; + case RTS51X_STAT_IDLE: + case RTS51X_STAT_SS: +- usb_stor_dbg(us, "RTS51X_STAT_SS, intf->pm_usage_cnt:%d, power.usage:%d\n", +- atomic_read(&us->pusb_intf->pm_usage_cnt), ++ usb_stor_dbg(us, "RTS51X_STAT_SS, power.usage:%d\n", + atomic_read(&us->pusb_intf->dev.power.usage_count)); + +- if (atomic_read(&us->pusb_intf->pm_usage_cnt) > 0) { ++ if (atomic_read(&us->pusb_intf->dev.power.usage_count) > 0) { + usb_stor_dbg(us, "Ready to enter SS state\n"); + rts51x_set_stat(chip, RTS51X_STAT_SS); + /* ignore mass storage interface's children */ + pm_suspend_ignore_children(&us->pusb_intf->dev, true); + usb_autopm_put_interface_async(us->pusb_intf); +- usb_stor_dbg(us, "RTS51X_STAT_SS 01, intf->pm_usage_cnt:%d, power.usage:%d\n", +- atomic_read(&us->pusb_intf->pm_usage_cnt), ++ usb_stor_dbg(us, "RTS51X_STAT_SS 01, power.usage:%d\n", + atomic_read(&us->pusb_intf->dev.power.usage_count)); + } + break; +@@ -816,11 +814,10 @@ static void rts51x_invoke_transport(stru + int ret; + + if (working_scsi(srb)) { +- usb_stor_dbg(us, "working scsi, intf->pm_usage_cnt:%d, power.usage:%d\n", +- atomic_read(&us->pusb_intf->pm_usage_cnt), ++ usb_stor_dbg(us, "working scsi, power.usage:%d\n", + atomic_read(&us->pusb_intf->dev.power.usage_count)); + +- if (atomic_read(&us->pusb_intf->pm_usage_cnt) <= 0) { ++ if (atomic_read(&us->pusb_intf->dev.power.usage_count) <= 0) { + ret = usb_autopm_get_interface(us->pusb_intf); + usb_stor_dbg(us, "working scsi, ret=%d\n", ret); + } +--- a/include/linux/usb.h ++++ b/include/linux/usb.h +@@ -127,7 +127,6 @@ enum usb_interface_condition { + * @dev: driver model's view of this device + * @usb_dev: if an interface is bound to the USB major, this will point + * to the sysfs representation for that device. +- * @pm_usage_cnt: PM usage counter for this interface + * @reset_ws: Used for scheduling resets from atomic context. + * @resetting_device: USB core reset the device, so use alt setting 0 as + * current; needs bandwidth alloc after reset. +@@ -184,7 +183,6 @@ struct usb_interface { + + struct device dev; /* interface specific device info */ + struct device *usb_dev; +- atomic_t pm_usage_cnt; /* usage counter for autosuspend */ + struct work_struct reset_ws; /* for resets in atomic context */ + }; + #define to_usb_interface(d) container_of(d, struct usb_interface, dev) diff --git a/queue-4.4/usb-core-fix-unterminated-string-returned-by-usb_string.patch b/queue-4.4/usb-core-fix-unterminated-string-returned-by-usb_string.patch new file mode 100644 index 00000000000..4f5a5865070 --- /dev/null +++ b/queue-4.4/usb-core-fix-unterminated-string-returned-by-usb_string.patch @@ -0,0 +1,49 @@ +From c01c348ecdc66085e44912c97368809612231520 Mon Sep 17 00:00:00 2001 +From: Alan Stern +Date: Mon, 15 Apr 2019 11:51:38 -0400 +Subject: USB: core: Fix unterminated string returned by usb_string() + +From: Alan Stern + +commit c01c348ecdc66085e44912c97368809612231520 upstream. + +Some drivers (such as the vub300 MMC driver) expect usb_string() to +return a properly NUL-terminated string, even when an error occurs. +(In fact, vub300's probe routine doesn't bother to check the return +code from usb_string().) When the driver goes on to use an +unterminated string, it leads to kernel errors such as +stack-out-of-bounds, as found by the syzkaller USB fuzzer. + +An out-of-range string index argument is not at all unlikely, given +that some devices don't provide string descriptors and therefore list +0 as the value for their string indexes. This patch makes +usb_string() return a properly terminated empty string along with the +-EINVAL error code when an out-of-range index is encountered. + +And since a USB string index is a single-byte value, indexes >= 256 +are just as invalid as values of 0 or below. + +Signed-off-by: Alan Stern +Reported-by: syzbot+b75b85111c10b8d680f1@syzkaller.appspotmail.com +CC: +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/usb/core/message.c | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +--- a/drivers/usb/core/message.c ++++ b/drivers/usb/core/message.c +@@ -820,9 +820,11 @@ int usb_string(struct usb_device *dev, i + + if (dev->state == USB_STATE_SUSPENDED) + return -EHOSTUNREACH; +- if (size <= 0 || !buf || !index) ++ if (size <= 0 || !buf) + return -EINVAL; + buf[0] = 0; ++ if (index <= 0 || index >= 256) ++ return -EINVAL; + tbuf = kmalloc(256, GFP_NOIO); + if (!tbuf) + return -ENOMEM; diff --git a/queue-4.4/usb-w1-ds2490-fix-bug-caused-by-improper-use-of-altsetting-array.patch b/queue-4.4/usb-w1-ds2490-fix-bug-caused-by-improper-use-of-altsetting-array.patch new file mode 100644 index 00000000000..55097f07d77 --- /dev/null +++ b/queue-4.4/usb-w1-ds2490-fix-bug-caused-by-improper-use-of-altsetting-array.patch @@ -0,0 +1,50 @@ +From c114944d7d67f24e71562fcfc18d550ab787e4d4 Mon Sep 17 00:00:00 2001 +From: Alan Stern +Date: Mon, 22 Apr 2019 11:16:04 -0400 +Subject: USB: w1 ds2490: Fix bug caused by improper use of altsetting array + +From: Alan Stern + +commit c114944d7d67f24e71562fcfc18d550ab787e4d4 upstream. + +The syzkaller USB fuzzer spotted a slab-out-of-bounds bug in the +ds2490 driver. This bug is caused by improper use of the altsetting +array in the usb_interface structure (the array's entries are not +always stored in numerical order), combined with a naive assumption +that all interfaces probed by the driver will have the expected number +of altsettings. + +The bug can be fixed by replacing references to the possibly +non-existent intf->altsetting[alt] entry with the guaranteed-to-exist +intf->cur_altsetting entry. + +Signed-off-by: Alan Stern +Reported-and-tested-by: syzbot+d65f673b847a1a96cdba@syzkaller.appspotmail.com +CC: +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/w1/masters/ds2490.c | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +--- a/drivers/w1/masters/ds2490.c ++++ b/drivers/w1/masters/ds2490.c +@@ -1039,15 +1039,15 @@ static int ds_probe(struct usb_interface + /* alternative 3, 1ms interrupt (greatly speeds search), 64 byte bulk */ + alt = 3; + err = usb_set_interface(dev->udev, +- intf->altsetting[alt].desc.bInterfaceNumber, alt); ++ intf->cur_altsetting->desc.bInterfaceNumber, alt); + if (err) { + dev_err(&dev->udev->dev, "Failed to set alternative setting %d " + "for %d interface: err=%d.\n", alt, +- intf->altsetting[alt].desc.bInterfaceNumber, err); ++ intf->cur_altsetting->desc.bInterfaceNumber, err); + goto err_out_clear; + } + +- iface_desc = &intf->altsetting[alt]; ++ iface_desc = intf->cur_altsetting; + if (iface_desc->desc.bNumEndpoints != NUM_EP-1) { + pr_info("Num endpoints=%d. It is not DS9490R.\n", + iface_desc->desc.bNumEndpoints); diff --git a/queue-4.4/usb-yurex-fix-protection-fault-after-device-removal.patch b/queue-4.4/usb-yurex-fix-protection-fault-after-device-removal.patch new file mode 100644 index 00000000000..45d49700950 --- /dev/null +++ b/queue-4.4/usb-yurex-fix-protection-fault-after-device-removal.patch @@ -0,0 +1,40 @@ +From ef61eb43ada6c1d6b94668f0f514e4c268093ff3 Mon Sep 17 00:00:00 2001 +From: Alan Stern +Date: Tue, 23 Apr 2019 14:48:29 -0400 +Subject: USB: yurex: Fix protection fault after device removal + +From: Alan Stern + +commit ef61eb43ada6c1d6b94668f0f514e4c268093ff3 upstream. + +The syzkaller USB fuzzer found a general-protection-fault bug in the +yurex driver. The fault occurs when a device has been unplugged; the +driver's interrupt-URB handler logs an error message referring to the +device by name, after the device has been unregistered and its name +deallocated. + +This problem is caused by the fact that the interrupt URB isn't +cancelled until the driver's private data structure is released, which +can happen long after the device is gone. The cure is to make sure +that the interrupt URB is killed before yurex_disconnect() returns; +this is exactly the sort of thing that usb_poison_urb() was meant for. + +Signed-off-by: Alan Stern +Reported-and-tested-by: syzbot+2eb9121678bdb36e6d57@syzkaller.appspotmail.com +CC: +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/usb/misc/yurex.c | 1 + + 1 file changed, 1 insertion(+) + +--- a/drivers/usb/misc/yurex.c ++++ b/drivers/usb/misc/yurex.c +@@ -332,6 +332,7 @@ static void yurex_disconnect(struct usb_ + usb_deregister_dev(interface, &yurex_class); + + /* prevent more I/O from starting */ ++ usb_poison_urb(dev->urb); + mutex_lock(&dev->io_mutex); + dev->interface = NULL; + mutex_unlock(&dev->io_mutex);