Skip to content

Commit ae8709b

Browse files
AlanSterngregkh
AlanStern
authored and
gregkh
committed
USB: core: Make do_proc_control() and do_proc_bulk() killable
The USBDEVFS_CONTROL and USBDEVFS_BULK ioctls invoke usb_start_wait_urb(), which contains an uninterruptible wait with a user-specified timeout value. If timeout value is very large and the device being accessed does not respond in a reasonable amount of time, the kernel will complain about "Task X blocked for more than N seconds", as found in testing by syzbot: INFO: task syz-executor.0:8700 blocked for more than 143 seconds. Not tainted 5.14.0-rc7-syzkaller #0 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:syz-executor.0 state:D stack:23192 pid: 8700 ppid: 8455 flags:0x00004004 Call Trace: context_switch kernel/sched/core.c:4681 [inline] __schedule+0xc07/0x11f0 kernel/sched/core.c:5938 schedule+0x14b/0x210 kernel/sched/core.c:6017 schedule_timeout+0x98/0x2f0 kernel/time/timer.c:1857 do_wait_for_common+0x2da/0x480 kernel/sched/completion.c:85 __wait_for_common kernel/sched/completion.c:106 [inline] wait_for_common kernel/sched/completion.c:117 [inline] wait_for_completion_timeout+0x46/0x60 kernel/sched/completion.c:157 usb_start_wait_urb+0x167/0x550 drivers/usb/core/message.c:63 do_proc_bulk+0x978/0x1080 drivers/usb/core/devio.c:1236 proc_bulk drivers/usb/core/devio.c:1273 [inline] usbdev_do_ioctl drivers/usb/core/devio.c:2547 [inline] usbdev_ioctl+0x3441/0x6b10 drivers/usb/core/devio.c:2713 ... To fix this problem, this patch replaces usbfs's calls to usb_control_msg() and usb_bulk_msg() with special-purpose code that does essentially the same thing (as recommended in the comment for usb_start_wait_urb()), except that it always uses a killable wait and it uses GFP_KERNEL rather than GFP_NOIO. Reported-and-tested-by: syzbot+ada0f7d3d9fd2016d927@syzkaller.appspotmail.com Suggested-by: Oliver Neukum <oneukum@suse.com> Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Link: https://lore.kernel.org/r/20210903175312.GA468440@rowland.harvard.edu Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 7042b10 commit ae8709b

File tree

1 file changed

+111
-33
lines changed

1 file changed

+111
-33
lines changed

‎drivers/usb/core/devio.c‎

Lines changed: 111 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include <linux/usb.h>
3333
#include <linux/usbdevice_fs.h>
3434
#include <linux/usb/hcd.h> /* for usbcore internals */
35+
#include <linux/usb/quirks.h>
3536
#include <linux/cdev.h>
3637
#include <linux/notifier.h>
3738
#include <linux/security.h>
@@ -1102,14 +1103,55 @@ static int usbdev_release(struct inode *inode, struct file *file)
11021103
return 0;
11031104
}
11041105

1106+
static void usbfs_blocking_completion(struct urb *urb)
1107+
{
1108+
complete((struct completion *) urb->context);
1109+
}
1110+
1111+
/*
1112+
* Much like usb_start_wait_urb, but returns status separately from
1113+
* actual_length and uses a killable wait.
1114+
*/
1115+
static int usbfs_start_wait_urb(struct urb *urb, int timeout,
1116+
unsigned int *actlen)
1117+
{
1118+
DECLARE_COMPLETION_ONSTACK(ctx);
1119+
unsigned long expire;
1120+
int rc;
1121+
1122+
urb->context = &ctx;
1123+
urb->complete = usbfs_blocking_completion;
1124+
*actlen = 0;
1125+
rc = usb_submit_urb(urb, GFP_KERNEL);
1126+
if (unlikely(rc))
1127+
return rc;
1128+
1129+
expire = (timeout ? msecs_to_jiffies(timeout) : MAX_SCHEDULE_TIMEOUT);
1130+
rc = wait_for_completion_killable_timeout(&ctx, expire);
1131+
if (rc <= 0) {
1132+
usb_kill_urb(urb);
1133+
*actlen = urb->actual_length;
1134+
if (urb->status != -ENOENT)
1135+
; /* Completed before it was killed */
1136+
else if (rc < 0)
1137+
return -EINTR;
1138+
else
1139+
return -ETIMEDOUT;
1140+
}
1141+
*actlen = urb->actual_length;
1142+
return urb->status;
1143+
}
1144+
11051145
static int do_proc_control(struct usb_dev_state *ps,
11061146
struct usbdevfs_ctrltransfer *ctrl)
11071147
{
11081148
struct usb_device *dev = ps->dev;
11091149
unsigned int tmo;
11101150
unsigned char *tbuf;
1111-
unsigned wLength;
1151+
unsigned int wLength, actlen;
11121152
int i, pipe, ret;
1153+
struct urb *urb = NULL;
1154+
struct usb_ctrlrequest *dr = NULL;
11131155

11141156
ret = check_ctrlrecip(ps, ctrl->bRequestType, ctrl->bRequest,
11151157
ctrl->wIndex);
@@ -1122,60 +1164,79 @@ static int do_proc_control(struct usb_dev_state *ps,
11221164
sizeof(struct usb_ctrlrequest));
11231165
if (ret)
11241166
return ret;
1167+
1168+
ret = -ENOMEM;
11251169
tbuf = (unsigned char *)__get_free_page(GFP_KERNEL);
1126-
if (!tbuf) {
1127-
ret = -ENOMEM;
1170+
if (!tbuf)
11281171
goto done;
1129-
}
1172+
urb = usb_alloc_urb(0, GFP_NOIO);
1173+
if (!urb)
1174+
goto done;
1175+
dr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_NOIO);
1176+
if (!dr)
1177+
goto done;
1178+
1179+
dr->bRequestType = ctrl->bRequestType;
1180+
dr->bRequest = ctrl->bRequest;
1181+
dr->wValue = cpu_to_le16(ctrl->wValue);
1182+
dr->wIndex = cpu_to_le16(ctrl->wIndex);
1183+
dr->wLength = cpu_to_le16(ctrl->wLength);
1184+
11301185
tmo = ctrl->timeout;
11311186
snoop(&dev->dev, "control urb: bRequestType=%02x "
11321187
"bRequest=%02x wValue=%04x "
11331188
"wIndex=%04x wLength=%04x\n",
11341189
ctrl->bRequestType, ctrl->bRequest, ctrl->wValue,
11351190
ctrl->wIndex, ctrl->wLength);
1136-
if ((ctrl->bRequestType & USB_DIR_IN) && ctrl->wLength) {
1191+
1192+
if ((ctrl->bRequestType & USB_DIR_IN) && wLength) {
11371193
pipe = usb_rcvctrlpipe(dev, 0);
1138-
snoop_urb(dev, NULL, pipe, ctrl->wLength, tmo, SUBMIT, NULL, 0);
1194+
usb_fill_control_urb(urb, dev, pipe, (unsigned char *) dr, tbuf,
1195+
wLength, NULL, NULL);
1196+
snoop_urb(dev, NULL, pipe, wLength, tmo, SUBMIT, NULL, 0);
11391197

11401198
usb_unlock_device(dev);
1141-
i = usb_control_msg(dev, pipe, ctrl->bRequest,
1142-
ctrl->bRequestType, ctrl->wValue, ctrl->wIndex,
1143-
tbuf, ctrl->wLength, tmo);
1199+
i = usbfs_start_wait_urb(urb, tmo, &actlen);
11441200
usb_lock_device(dev);
1145-
snoop_urb(dev, NULL, pipe, max(i, 0), min(i, 0), COMPLETE,
1146-
tbuf, max(i, 0));
1147-
if ((i > 0) && ctrl->wLength) {
1148-
if (copy_to_user(ctrl->data, tbuf, i)) {
1201+
snoop_urb(dev, NULL, pipe, actlen, i, COMPLETE, tbuf, actlen);
1202+
if (!i && actlen) {
1203+
if (copy_to_user(ctrl->data, tbuf, actlen)) {
11491204
ret = -EFAULT;
1150-
goto done;
1205+
goto recv_fault;
11511206
}
11521207
}
11531208
} else {
1154-
if (ctrl->wLength) {
1155-
if (copy_from_user(tbuf, ctrl->data, ctrl->wLength)) {
1209+
if (wLength) {
1210+
if (copy_from_user(tbuf, ctrl->data, wLength)) {
11561211
ret = -EFAULT;
11571212
goto done;
11581213
}
11591214
}
11601215
pipe = usb_sndctrlpipe(dev, 0);
1161-
snoop_urb(dev, NULL, pipe, ctrl->wLength, tmo, SUBMIT,
1162-
tbuf, ctrl->wLength);
1216+
usb_fill_control_urb(urb, dev, pipe, (unsigned char *) dr, tbuf,
1217+
wLength, NULL, NULL);
1218+
snoop_urb(dev, NULL, pipe, wLength, tmo, SUBMIT, tbuf, wLength);
11631219

11641220
usb_unlock_device(dev);
1165-
i = usb_control_msg(dev, pipe, ctrl->bRequest,
1166-
ctrl->bRequestType, ctrl->wValue, ctrl->wIndex,
1167-
tbuf, ctrl->wLength, tmo);
1221+
i = usbfs_start_wait_urb(urb, tmo, &actlen);
11681222
usb_lock_device(dev);
1169-
snoop_urb(dev, NULL, pipe, max(i, 0), min(i, 0), COMPLETE, NULL, 0);
1223+
snoop_urb(dev, NULL, pipe, actlen, i, COMPLETE, NULL, 0);
11701224
}
11711225
if (i < 0 && i != -EPIPE) {
11721226
dev_printk(KERN_DEBUG, &dev->dev, "usbfs: USBDEVFS_CONTROL "
11731227
"failed cmd %s rqt %u rq %u len %u ret %d\n",
11741228
current->comm, ctrl->bRequestType, ctrl->bRequest,
11751229
ctrl->wLength, i);
11761230
}
1177-
ret = i;
1231+
ret = (i < 0 ? i : actlen);
1232+
1233+
recv_fault:
1234+
/* Linger a bit, prior to the next control message. */
1235+
if (dev->quirks & USB_QUIRK_DELAY_CTRL_MSG)
1236+
msleep(200);
11781237
done:
1238+
kfree(dr);
1239+
usb_free_urb(urb);
11791240
free_page((unsigned long) tbuf);
11801241
usbfs_decrease_memory_usage(PAGE_SIZE + sizeof(struct urb) +
11811242
sizeof(struct usb_ctrlrequest));
@@ -1195,25 +1256,29 @@ static int do_proc_bulk(struct usb_dev_state *ps,
11951256
struct usbdevfs_bulktransfer *bulk)
11961257
{
11971258
struct usb_device *dev = ps->dev;
1198-
unsigned int tmo, len1, pipe;
1199-
int len2;
1259+
unsigned int tmo, len1, len2, pipe;
12001260
unsigned char *tbuf;
12011261
int i, ret;
1262+
struct urb *urb = NULL;
1263+
struct usb_host_endpoint *ep;
12021264

12031265
ret = findintfep(ps->dev, bulk->ep);
12041266
if (ret < 0)
12051267
return ret;
12061268
ret = checkintf(ps, ret);
12071269
if (ret)
12081270
return ret;
1271+
1272+
len1 = bulk->len;
1273+
if (len1 < 0 || len1 >= (INT_MAX - sizeof(struct urb)))
1274+
return -EINVAL;
1275+
12091276
if (bulk->ep & USB_DIR_IN)
12101277
pipe = usb_rcvbulkpipe(dev, bulk->ep & 0x7f);
12111278
else
12121279
pipe = usb_sndbulkpipe(dev, bulk->ep & 0x7f);
1213-
if (!usb_maxpacket(dev, pipe, !(bulk->ep & USB_DIR_IN)))
1214-
return -EINVAL;
1215-
len1 = bulk->len;
1216-
if (len1 >= (INT_MAX - sizeof(struct urb)))
1280+
ep = usb_pipe_endpoint(dev, pipe);
1281+
if (!ep || !usb_endpoint_maxp(&ep->desc))
12171282
return -EINVAL;
12181283
ret = usbfs_increase_memory_usage(len1 + sizeof(struct urb));
12191284
if (ret)
@@ -1223,17 +1288,29 @@ static int do_proc_bulk(struct usb_dev_state *ps,
12231288
* len1 can be almost arbitrarily large. Don't WARN if it's
12241289
* too big, just fail the request.
12251290
*/
1291+
ret = -ENOMEM;
12261292
tbuf = kmalloc(len1, GFP_KERNEL | __GFP_NOWARN);
1227-
if (!tbuf) {
1228-
ret = -ENOMEM;
1293+
if (!tbuf)
1294+
goto done;
1295+
urb = usb_alloc_urb(0, GFP_KERNEL);
1296+
if (!urb)
12291297
goto done;
1298+
1299+
if ((ep->desc.bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) ==
1300+
USB_ENDPOINT_XFER_INT) {
1301+
pipe = (pipe & ~(3 << 30)) | (PIPE_INTERRUPT << 30);
1302+
usb_fill_int_urb(urb, dev, pipe, tbuf, len1,
1303+
NULL, NULL, ep->desc.bInterval);
1304+
} else {
1305+
usb_fill_bulk_urb(urb, dev, pipe, tbuf, len1, NULL, NULL);
12301306
}
1307+
12311308
tmo = bulk->timeout;
12321309
if (bulk->ep & 0x80) {
12331310
snoop_urb(dev, NULL, pipe, len1, tmo, SUBMIT, NULL, 0);
12341311

12351312
usb_unlock_device(dev);
1236-
i = usb_bulk_msg(dev, pipe, tbuf, len1, &len2, tmo);
1313+
i = usbfs_start_wait_urb(urb, tmo, &len2);
12371314
usb_lock_device(dev);
12381315
snoop_urb(dev, NULL, pipe, len2, i, COMPLETE, tbuf, len2);
12391316

@@ -1253,12 +1330,13 @@ static int do_proc_bulk(struct usb_dev_state *ps,
12531330
snoop_urb(dev, NULL, pipe, len1, tmo, SUBMIT, tbuf, len1);
12541331

12551332
usb_unlock_device(dev);
1256-
i = usb_bulk_msg(dev, pipe, tbuf, len1, &len2, tmo);
1333+
i = usbfs_start_wait_urb(urb, tmo, &len2);
12571334
usb_lock_device(dev);
12581335
snoop_urb(dev, NULL, pipe, len2, i, COMPLETE, NULL, 0);
12591336
}
12601337
ret = (i < 0 ? i : len2);
12611338
done:
1339+
usb_free_urb(urb);
12621340
kfree(tbuf);
12631341
usbfs_decrease_memory_usage(len1 + sizeof(struct urb));
12641342
return ret;

0 commit comments

Comments
 (0)