| From 68035c80e129c4cfec659aac4180354530b26527 Mon Sep 17 00:00:00 2001 |
| From: Will Deacon <will@kernel.org> |
| Date: Fri, 8 Nov 2019 16:48:38 +0100 |
| Subject: media: uvcvideo: Avoid cyclic entity chains due to malformed USB descriptors |
| |
| From: Will Deacon <will@kernel.org> |
| |
| commit 68035c80e129c4cfec659aac4180354530b26527 upstream. |
| |
| Way back in 2017, fuzzing the 4.14-rc2 USB stack with syzkaller kicked |
| up the following WARNING from the UVC chain scanning code: |
| |
| | list_add double add: new=ffff880069084010, prev=ffff880069084010, |
| | next=ffff880067d22298. |
| | ------------[ cut here ]------------ |
| | WARNING: CPU: 1 PID: 1846 at lib/list_debug.c:31 __list_add_valid+0xbd/0xf0 |
| | Modules linked in: |
| | CPU: 1 PID: 1846 Comm: kworker/1:2 Not tainted |
| | 4.14.0-rc2-42613-g1488251d1a98 #238 |
| | Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 |
| | Workqueue: usb_hub_wq hub_event |
| | task: ffff88006b01ca40 task.stack: ffff880064358000 |
| | RIP: 0010:__list_add_valid+0xbd/0xf0 lib/list_debug.c:29 |
| | RSP: 0018:ffff88006435ddd0 EFLAGS: 00010286 |
| | RAX: 0000000000000058 RBX: ffff880067d22298 RCX: 0000000000000000 |
| | RDX: 0000000000000058 RSI: ffffffff85a58800 RDI: ffffed000c86bbac |
| | RBP: ffff88006435dde8 R08: 1ffff1000c86ba52 R09: 0000000000000000 |
| | R10: 0000000000000002 R11: 0000000000000000 R12: ffff880069084010 |
| | R13: ffff880067d22298 R14: ffff880069084010 R15: ffff880067d222a0 |
| | FS: 0000000000000000(0000) GS:ffff88006c900000(0000) knlGS:0000000000000000 |
| | CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 |
| | CR2: 0000000020004ff2 CR3: 000000006b447000 CR4: 00000000000006e0 |
| | Call Trace: |
| | __list_add ./include/linux/list.h:59 |
| | list_add_tail+0x8c/0x1b0 ./include/linux/list.h:92 |
| | uvc_scan_chain_forward.isra.8+0x373/0x416 |
| | drivers/media/usb/uvc/uvc_driver.c:1471 |
| | uvc_scan_chain drivers/media/usb/uvc/uvc_driver.c:1585 |
| | uvc_scan_device drivers/media/usb/uvc/uvc_driver.c:1769 |
| | uvc_probe+0x77f2/0x8f00 drivers/media/usb/uvc/uvc_driver.c:2104 |
| |
| Looking into the output from usbmon, the interesting part is the |
| following data packet: |
| |
| ffff880069c63e00 30710169 C Ci:1:002:0 0 143 = 09028f00 01030080 |
| 00090403 00000e01 00000924 03000103 7c003328 010204db |
| |
| If we drop the lead configuration and interface descriptors, we're left |
| with an output terminal descriptor describing a generic display: |
| |
| /* Output terminal descriptor */ |
| buf[0] 09 |
| buf[1] 24 |
| buf[2] 03 /* UVC_VC_OUTPUT_TERMINAL */ |
| buf[3] 00 /* ID */ |
| buf[4] 01 /* type == 0x0301 (UVC_OTT_DISPLAY) */ |
| buf[5] 03 |
| buf[6] 7c |
| buf[7] 00 /* source ID refers to self! */ |
| buf[8] 33 |
| |
| The problem with this descriptor is that it is self-referential: the |
| source ID of 0 matches itself! This causes the 'struct uvc_entity' |
| representing the display to be added to its chain list twice during |
| 'uvc_scan_chain()': once via 'uvc_scan_chain_entity()' when it is |
| processed directly from the 'dev->entities' list and then again |
| immediately afterwards when trying to follow the source ID in |
| 'uvc_scan_chain_forward()' |
| |
| Add a check before adding an entity to a chain list to ensure that the |
| entity is not already part of a chain. |
| |
| Link: https://lore.kernel.org/linux-media/CAAeHK+z+Si69jUR+N-SjN9q4O+o5KFiNManqEa-PjUta7EOb7A@mail.gmail.com/ |
| |
| Cc: <stable@vger.kernel.org> |
| Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver") |
| Reported-by: Andrey Konovalov <andreyknvl@google.com> |
| Signed-off-by: Will Deacon <will@kernel.org> |
| Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> |
| Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/media/usb/uvc/uvc_driver.c | 12 ++++++++++++ |
| 1 file changed, 12 insertions(+) |
| |
| --- a/drivers/media/usb/uvc/uvc_driver.c |
| +++ b/drivers/media/usb/uvc/uvc_driver.c |
| @@ -1482,6 +1482,11 @@ static int uvc_scan_chain_forward(struct |
| break; |
| if (forward == prev) |
| continue; |
| + if (forward->chain.next || forward->chain.prev) { |
| + uvc_trace(UVC_TRACE_DESCR, "Found reference to " |
| + "entity %d already in chain.\n", forward->id); |
| + return -EINVAL; |
| + } |
| |
| switch (UVC_ENTITY_TYPE(forward)) { |
| case UVC_VC_EXTENSION_UNIT: |
| @@ -1563,6 +1568,13 @@ static int uvc_scan_chain_backward(struc |
| return -1; |
| } |
| |
| + if (term->chain.next || term->chain.prev) { |
| + uvc_trace(UVC_TRACE_DESCR, "Found reference to " |
| + "entity %d already in chain.\n", |
| + term->id); |
| + return -EINVAL; |
| + } |
| + |
| if (uvc_trace_param & UVC_TRACE_PROBE) |
| printk(KERN_CONT " %d", term->id); |
| |