|
| 1 | +usb: core: Don't hold the device lock while sleeping in do_proc_control() |
| 2 | + |
| 3 | +jira LE-3201 |
| 4 | +cve CVE-2021-47582 |
| 5 | +Rebuild_History Non-Buildable kernel-rt-4.18.0-553.22.1.rt7.363.el8_10 |
| 6 | +commit-author Tasos Sahanidis <tasos@tasossah.com> |
| 7 | +commit 0543e4e8852ef5ff1809ae62f1ea963e2ab23b66 |
| 8 | +Empty-Commit: Cherry-Pick Conflicts during history rebuild. |
| 9 | +Will be included in final tarball splat. Ref for failed cherry-pick at: |
| 10 | +ciq/ciq_backports/kernel-rt-4.18.0-553.22.1.rt7.363.el8_10/0543e4e8.failed |
| 11 | + |
| 12 | +Since commit ae8709b296d8 ("USB: core: Make do_proc_control() and |
| 13 | +do_proc_bulk() killable") if a device has the USB_QUIRK_DELAY_CTRL_MSG |
| 14 | +quirk set, it will temporarily block all other URBs (e.g. interrupts) |
| 15 | +while sleeping due to a control. |
| 16 | + |
| 17 | +This results in noticeable delays when, for example, a userspace usbfs |
| 18 | +application is sending URB interrupts at a high rate to a keyboard and |
| 19 | +simultaneously updates the lock indicators using controls. Interrupts |
| 20 | +with direction set to IN are also affected by this, meaning that |
| 21 | +delivery of HID reports (containing scancodes) to the usbfs application |
| 22 | +is delayed as well. |
| 23 | + |
| 24 | +This patch fixes the regression by calling msleep() while the device |
| 25 | +mutex is unlocked, as was the case originally with usb_control_msg(). |
| 26 | + |
| 27 | +Fixes: ae8709b296d8 ("USB: core: Make do_proc_control() and do_proc_bulk() killable") |
| 28 | + Cc: stable <stable@kernel.org> |
| 29 | + Acked-by: Alan Stern <stern@rowland.harvard.edu> |
| 30 | + Signed-off-by: Tasos Sahanidis <tasos@tasossah.com> |
| 31 | +Link: https://lore.kernel.org/r/3e299e2a-13b9-ddff-7fee-6845e868bc06@tasossah.com |
| 32 | + Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| 33 | +(cherry picked from commit 0543e4e8852ef5ff1809ae62f1ea963e2ab23b66) |
| 34 | + Signed-off-by: Jonathan Maple <jmaple@ciq.com> |
| 35 | + |
| 36 | +# Conflicts: |
| 37 | +# drivers/usb/core/devio.c |
| 38 | +diff --cc drivers/usb/core/devio.c |
| 39 | +index 986ad8499855,b5b85bf80329..000000000000 |
| 40 | +--- a/drivers/usb/core/devio.c |
| 41 | ++++ b/drivers/usb/core/devio.c |
| 42 | +@@@ -1141,19 -1200,23 +1141,27 @@@ static int do_proc_control(struct usb_d |
| 43 | + "wIndex=%04x wLength=%04x\n", |
| 44 | + ctrl->bRequestType, ctrl->bRequest, ctrl->wValue, |
| 45 | + ctrl->wIndex, ctrl->wLength); |
| 46 | + - |
| 47 | + - if ((ctrl->bRequestType & USB_DIR_IN) && wLength) { |
| 48 | + + if ((ctrl->bRequestType & USB_DIR_IN) && ctrl->wLength) { |
| 49 | + pipe = usb_rcvctrlpipe(dev, 0); |
| 50 | + - usb_fill_control_urb(urb, dev, pipe, (unsigned char *) dr, tbuf, |
| 51 | + - wLength, NULL, NULL); |
| 52 | + - snoop_urb(dev, NULL, pipe, wLength, tmo, SUBMIT, NULL, 0); |
| 53 | + + snoop_urb(dev, NULL, pipe, ctrl->wLength, tmo, SUBMIT, NULL, 0); |
| 54 | + |
| 55 | + usb_unlock_device(dev); |
| 56 | +++<<<<<<< HEAD |
| 57 | + + i = usb_control_msg(dev, pipe, ctrl->bRequest, |
| 58 | + + ctrl->bRequestType, ctrl->wValue, ctrl->wIndex, |
| 59 | + + tbuf, ctrl->wLength, tmo); |
| 60 | +++======= |
| 61 | ++ i = usbfs_start_wait_urb(urb, tmo, &actlen); |
| 62 | ++ |
| 63 | ++ /* Linger a bit, prior to the next control message. */ |
| 64 | ++ if (dev->quirks & USB_QUIRK_DELAY_CTRL_MSG) |
| 65 | ++ msleep(200); |
| 66 | +++>>>>>>> 0543e4e8852e (usb: core: Don't hold the device lock while sleeping in do_proc_control()) |
| 67 | + usb_lock_device(dev); |
| 68 | + - snoop_urb(dev, NULL, pipe, actlen, i, COMPLETE, tbuf, actlen); |
| 69 | + - if (!i && actlen) { |
| 70 | + - if (copy_to_user(ctrl->data, tbuf, actlen)) { |
| 71 | + + snoop_urb(dev, NULL, pipe, max(i, 0), min(i, 0), COMPLETE, |
| 72 | + + tbuf, max(i, 0)); |
| 73 | + + if ((i > 0) && ctrl->wLength) { |
| 74 | + + if (copy_to_user(ctrl->data, tbuf, i)) { |
| 75 | + ret = -EFAULT; |
| 76 | + goto done; |
| 77 | + } |
| 78 | +@@@ -1166,15 -1229,18 +1174,23 @@@ |
| 79 | + } |
| 80 | + } |
| 81 | + pipe = usb_sndctrlpipe(dev, 0); |
| 82 | + - usb_fill_control_urb(urb, dev, pipe, (unsigned char *) dr, tbuf, |
| 83 | + - wLength, NULL, NULL); |
| 84 | + - snoop_urb(dev, NULL, pipe, wLength, tmo, SUBMIT, tbuf, wLength); |
| 85 | + + snoop_urb(dev, NULL, pipe, ctrl->wLength, tmo, SUBMIT, |
| 86 | + + tbuf, ctrl->wLength); |
| 87 | + |
| 88 | + usb_unlock_device(dev); |
| 89 | +++<<<<<<< HEAD |
| 90 | + + i = usb_control_msg(dev, pipe, ctrl->bRequest, |
| 91 | + + ctrl->bRequestType, ctrl->wValue, ctrl->wIndex, |
| 92 | + + tbuf, ctrl->wLength, tmo); |
| 93 | +++======= |
| 94 | ++ i = usbfs_start_wait_urb(urb, tmo, &actlen); |
| 95 | ++ |
| 96 | ++ /* Linger a bit, prior to the next control message. */ |
| 97 | ++ if (dev->quirks & USB_QUIRK_DELAY_CTRL_MSG) |
| 98 | ++ msleep(200); |
| 99 | +++>>>>>>> 0543e4e8852e (usb: core: Don't hold the device lock while sleeping in do_proc_control()) |
| 100 | + usb_lock_device(dev); |
| 101 | + - snoop_urb(dev, NULL, pipe, actlen, i, COMPLETE, NULL, 0); |
| 102 | + + snoop_urb(dev, NULL, pipe, max(i, 0), min(i, 0), COMPLETE, NULL, 0); |
| 103 | + } |
| 104 | + if (i < 0 && i != -EPIPE) { |
| 105 | + dev_printk(KERN_DEBUG, &dev->dev, "usbfs: USBDEVFS_CONTROL " |
| 106 | +@@@ -1182,8 -1248,11 +1198,13 @@@ |
| 107 | + current->comm, ctrl->bRequestType, ctrl->bRequest, |
| 108 | + ctrl->wLength, i); |
| 109 | + } |
| 110 | +++<<<<<<< HEAD |
| 111 | + + ret = i; |
| 112 | +++======= |
| 113 | ++ ret = (i < 0 ? i : actlen); |
| 114 | ++ |
| 115 | +++>>>>>>> 0543e4e8852e (usb: core: Don't hold the device lock while sleeping in do_proc_control()) |
| 116 | + done: |
| 117 | + - kfree(dr); |
| 118 | + - usb_free_urb(urb); |
| 119 | + free_page((unsigned long) tbuf); |
| 120 | + usbfs_decrease_memory_usage(PAGE_SIZE + sizeof(struct urb) + |
| 121 | + sizeof(struct usb_ctrlrequest)); |
| 122 | +* Unmerged path drivers/usb/core/devio.c |
0 commit comments