--- /dev/null
+From c2b71462d294cf517a0bc6e4fd6424d7cee5596f Mon Sep 17 00:00:00 2001
+From: Alan Stern <stern@rowland.harvard.edu>
+Date: Fri, 19 Apr 2019 13:52:38 -0400
+Subject: USB: core: Fix bug caused by duplicate interface PM usage counter
+
+From: Alan Stern <stern@rowland.harvard.edu>
+
+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 <stern@rowland.harvard.edu>
+Reported-and-tested-by: syzbot+7634edaea4d0b341c625@syzkaller.appspotmail.com
+CC: <stable@vger.kernel.org>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+
+---
+ 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
+@@ -776,18 +776,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;
+@@ -820,11 +818,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
+@@ -129,7 +129,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.
+@@ -186,7 +185,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)
--- /dev/null
+From c409ca3be3c6ff3a1eeb303b191184e80d412862 Mon Sep 17 00:00:00 2001
+From: Malte Leip <malte@leip.net>
+Date: Sun, 14 Apr 2019 12:00:12 +0200
+Subject: usb: usbip: fix isoc packet num validation in get_pipe
+
+From: Malte Leip <malte@leip.net>
+
+commit c409ca3be3c6ff3a1eeb303b191184e80d412862 upstream.
+
+Change the validation of number_of_packets in get_pipe to compare the
+number of packets to a fixed maximum number of packets allowed, set to
+be 1024. This number was chosen due to it being used by other drivers as
+well, for example drivers/usb/host/uhci-q.c
+
+Background/reason:
+The get_pipe function in stub_rx.c validates the number of packets in
+isochronous mode and aborts with an error if that number is too large,
+in order to prevent malicious input from possibly triggering large
+memory allocations. This was previously done by checking whether
+pdu->u.cmd_submit.number_of_packets is bigger than the number of packets
+that would be needed for pdu->u.cmd_submit.transfer_buffer_length bytes
+if all except possibly the last packet had maximum length, given by
+usb_endpoint_maxp(epd) * usb_endpoint_maxp_mult(epd). This leads to an
+error if URBs with packets shorter than the maximum possible length are
+submitted, which is allowed according to
+Documentation/driver-api/usb/URB.rst and occurs for example with the
+snd-usb-audio driver.
+
+Fixes: c6688ef9f297 ("usbip: fix stub_rx: harden CMD_SUBMIT path to handle malicious input")
+Signed-off-by: Malte Leip <malte@leip.net>
+Cc: stable <stable@vger.kernel.org>
+Acked-by: Shuah Khan <skhan@linuxfoundation.org>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+
+---
+ drivers/usb/usbip/stub_rx.c | 12 +++---------
+ drivers/usb/usbip/usbip_common.h | 7 +++++++
+ 2 files changed, 10 insertions(+), 9 deletions(-)
+
+--- a/drivers/usb/usbip/stub_rx.c
++++ b/drivers/usb/usbip/stub_rx.c
+@@ -383,16 +383,10 @@ static int get_pipe(struct stub_device *
+ }
+
+ if (usb_endpoint_xfer_isoc(epd)) {
+- /* validate packet size and number of packets */
+- unsigned int maxp, packets, bytes;
+-
+- maxp = usb_endpoint_maxp(epd);
+- maxp *= usb_endpoint_maxp_mult(epd);
+- bytes = pdu->u.cmd_submit.transfer_buffer_length;
+- packets = DIV_ROUND_UP(bytes, maxp);
+-
++ /* validate number of packets */
+ if (pdu->u.cmd_submit.number_of_packets < 0 ||
+- pdu->u.cmd_submit.number_of_packets > packets) {
++ pdu->u.cmd_submit.number_of_packets >
++ USBIP_MAX_ISO_PACKETS) {
+ dev_err(&sdev->udev->dev,
+ "CMD_SUBMIT: isoc invalid num packets %d\n",
+ pdu->u.cmd_submit.number_of_packets);
+--- a/drivers/usb/usbip/usbip_common.h
++++ b/drivers/usb/usbip/usbip_common.h
+@@ -136,6 +136,13 @@ extern struct device_attribute dev_attr_
+ #define USBIP_DIR_OUT 0x00
+ #define USBIP_DIR_IN 0x01
+
++/*
++ * Arbitrary limit for the maximum number of isochronous packets in an URB,
++ * compare for example the uhci_submit_isochronous function in
++ * drivers/usb/host/uhci-q.c
++ */
++#define USBIP_MAX_ISO_PACKETS 1024
++
+ /**
+ * struct usbip_header_basic - data pertinent to every request
+ * @command: the usbip request type
--- /dev/null
+From c114944d7d67f24e71562fcfc18d550ab787e4d4 Mon Sep 17 00:00:00 2001
+From: Alan Stern <stern@rowland.harvard.edu>
+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 <stern@rowland.harvard.edu>
+
+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 <stern@rowland.harvard.edu>
+Reported-and-tested-by: syzbot+d65f673b847a1a96cdba@syzkaller.appspotmail.com
+CC: <stable@vger.kernel.org>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+
+---
+ 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);
projects / thirdparty / kernel / stable-queue.git / commitdiff
