Skip to content

Commit 9fc9075

Browse files
PlaidCatPlaidCat
committed
usb: core: Don't hold the device lock while sleeping in do_proc_control()
jira LE-3201 cve CVE-2021-47582 Rebuild_History Non-Buildable kernel-rt-4.18.0-553.22.1.rt7.363.el8_10 commit-author Tasos Sahanidis <tasos@tasossah.com> commit 0543e4e Empty-Commit: Cherry-Pick Conflicts during history rebuild. Will be included in final tarball splat. Ref for failed cherry-pick at: ciq/ciq_backports/kernel-rt-4.18.0-553.22.1.rt7.363.el8_10/0543e4e8.failed Since commit ae8709b ("USB: core: Make do_proc_control() and do_proc_bulk() killable") if a device has the USB_QUIRK_DELAY_CTRL_MSG quirk set, it will temporarily block all other URBs (e.g. interrupts) while sleeping due to a control. This results in noticeable delays when, for example, a userspace usbfs application is sending URB interrupts at a high rate to a keyboard and simultaneously updates the lock indicators using controls. Interrupts with direction set to IN are also affected by this, meaning that delivery of HID reports (containing scancodes) to the usbfs application is delayed as well. This patch fixes the regression by calling msleep() while the device mutex is unlocked, as was the case originally with usb_control_msg(). Fixes: ae8709b ("USB: core: Make do_proc_control() and do_proc_bulk() killable") Cc: stable <stable@kernel.org> Acked-by: Alan Stern <stern@rowland.harvard.edu> Signed-off-by: Tasos Sahanidis <tasos@tasossah.com> Link: https://lore.kernel.org/r/3e299e2a-13b9-ddff-7fee-6845e868bc06@tasossah.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> (cherry picked from commit 0543e4e) Signed-off-by: Jonathan Maple <jmaple@ciq.com> # Conflicts: # drivers/usb/core/devio.c
1 parent 5eee57b commit 9fc9075

File tree

1 file changed

+122
-0
lines changed

1 file changed

+122
-0
lines changed

‎ciq/ciq_backports/kernel-rt-4.18.0-553.22.1.rt7.363.el8_10/0543e4e8.failed‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
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

Comments
 (0)