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.
Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Mauro Carvalho Chehab mchehab@kernel.org Cc: Dmitry Vyukov dvyukov@google.com Cc: Kostya Serebryany kcc@google.com Cc: stable@vger.kernel.org Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver") Reported-by: Andrey Konovalov andreyknvl@google.com Link: https://lore.kernel.org/linux-media/CAAeHK+z+Si69jUR+N-SjN9q4O+o5KFiNManqEa-... Signed-off-by: Will Deacon will@kernel.org ---
That's right, it's the same patch again! No changes since either of:
http://lkml.kernel.org/r/20191002112753.21630-1-will@kernel.org https://lore.kernel.org/lkml/20191016195800.22099-1-will@kernel.org
Please consider merging.
drivers/media/usb/uvc/uvc_driver.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 66ee168ddc7e..e24420b1750a 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -1493,6 +1493,11 @@ static int uvc_scan_chain_forward(struct uvc_video_chain *chain, 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: @@ -1574,6 +1579,13 @@ static int uvc_scan_chain_backward(struct uvc_video_chain *chain, 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);
Hi Will,
Thank you for the patch.
I'm sorry for the delay, and will have to ask you to be a bit more patient I'm afraid. I will leave tomorrow for a week without computer access and will only be able to go through my backlog when I will be back on the 17th.
On Fri, Nov 08, 2019 at 03:48:38PM +0000, Will Deacon wrote:
Hi Laurent,
On Fri, Nov 08, 2019 at 05:55:03PM +0200, Laurent Pinchart wrote:
Ok, thanks for letting me know. I'll poke you again when you're back if I don't hear anything -- I haven't actually changed the patch for ages, since I don't think it needs further work [1].
Will
[1] https://lore.kernel.org/linux-media/20191007162709.3vrtbcpoymmnqikl@willie-t...
On Mon, Dec 16, 2019 at 1:16 PM Will Deacon will@kernel.org wrote:
Hi Will,
I think we now have a reproducer for this issue that syzbot just reported:
https://syzkaller.appspot.com/bug?extid=0a5c96772a9b26f2a876
You can try you patch on it :)
There's also another one, which looks related:
https://syzkaller.appspot.com/bug?extid=0b0095300dfeb8a83dc8
Thanks!
On Mon, Dec 16, 2019 at 02:17:52PM +0100, Andrey Konovalov wrote:
Oh wow, I *really* like the raw USB gadget thingy you have to reproduce these! I also really like that this patch fixes the issue. Logs below.
Laurent -- can we please merge this now?
Will
--->8
Before:
bash-5.0# ./repro [ 31.749418][ T92] usb 1-1: new high-speed USB device number 2 using dummy_hcd [ 31.989356][ T92] usb 1-1: Using ep0 maxpacket: 8 [ 32.109448][ T92] usb 1-1: config index 0 descriptor too short (expected 51150, got 70) [ 32.111898][ T92] usb 1-1: config 0 contains an unexpected descriptor of type 0x2, skipping [ 32.114317][ T92] usb 1-1: config 0 has an invalid descriptor of length 0, skipping remainder of the config [ 32.117145][ T92] usb 1-1: config 0 interface 0 altsetting 0 has 0 endpoint descriptors, different from the interface descriptor's value: 16 [ 32.120554][ T92] usb 1-1: New USB device found, idVendor=0bd3, idProduct=0755, bcdDevice=69.f1 [ 32.122875][ T92] usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0 [ 32.126602][ T92] usb 1-1: config 0 descriptor?? [ 32.399436][ T92] usb 1-1: string descriptor 0 read error: -71 [ 32.401266][ T92] uvcvideo: Found UVC 0.00 device <unnamed> (0bd3:0755) [ 32.403266][ T92] ------------[ cut here ]------------ [ 32.404790][ T92] list_add double add: new=ffff888015992010, prev=ffff888015992010, next=ffff8880146c6a18. [ 32.407819][ T92] WARNING: CPU: 2 PID: 92 at lib/list_debug.c:31 __list_add_valid+0xab/0xe0 [ 32.410214][ T92] Modules linked in: [ 32.411071][ T92] CPU: 2 PID: 92 Comm: kworker/2:1 Not tainted 5.5.0-rc2+ #1 [ 32.412432][ T92] Workqueue: usb_hub_wq hub_event [ 32.413364][ T92] RIP: 0010:__list_add_valid+0xab/0xe0 [ 32.414382][ T92] Code: 48 c7 c7 a0 ae fa 85 48 89 de e8 19 eb 2a ff 0f 0b 31 c0 eb cc 48 89 f2 48 89 d9 48 89 ee 48 c7 c7 20 af fa 85 e8 fe ea 2a ff <0f> 0b 31 c0 eb b1 48 89 34 24 e8 36 e8 7e ff 48 8b 34 24 e9 68 ff [ 32.418007][ T92] RSP: 0018:ffff8880158d7008 EFLAGS: 00010286 [ 32.419127][ T92] RAX: 0000000000000000 RBX: ffff8880146c6a18 RCX: ffffffff81293978 [ 32.420589][ T92] RDX: 0000000000000000 RSI: ffffffff812990fc RDI: 0000000000000006 [ 32.421692][ T92] RBP: ffff888015992010 R08: ffff88801551de80 R09: fffffbfff11ea4b5 [ 32.422744][ T92] R10: fffffbfff11ea4b4 R11: ffffffff88f525a7 R12: dffffc0000000000 [ 32.423784][ T92] R13: ffff888015992000 R14: ffff8880146c6a20 R15: ffff8880146c6a18 [ 32.424838][ T92] FS: 0000000000000000(0000) GS:ffff888016800000(0000) knlGS:0000000000000000 [ 32.425996][ T92] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 32.426867][ T92] CR2: 0000000000478f10 CR3: 000000001327e005 CR4: 0000000000760ea0 [ 32.427935][ T92] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 32.428972][ T92] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 32.430012][ T92] PKRU: 55555554 [ 32.430473][ T92] Call Trace: [ 32.430944][ T92] uvc_scan_chain_forward.isra.9+0x4df/0x635 [ 32.431600][ T92] uvc_probe.cold.19+0x1ef2/0x29bc [ 32.432175][ T92] ? __lock_acquire+0xeda/0x41a0 [ 32.432712][ T92] ? mark_lock+0xbe/0x10f0 [ 32.433209][ T92] ? pm_runtime_enable+0x2a/0x310 [ 32.433773][ T92] ? find_held_lock+0x33/0x1c0 [ 32.434307][ T92] ? usb_probe_interface+0x307/0x7b0 [ 32.434869][ T92] usb_probe_interface+0x307/0x7b0 [ 32.435414][ T92] ? usb_probe_device+0xf0/0xf0 [ 32.435938][ T92] really_probe+0x281/0x700 [ 32.436424][ T92] ? driver_allows_async_probing+0x150/0x150 [ 32.437065][ T92] driver_probe_device+0x105/0x200 [ 32.437611][ T92] __device_attach_driver+0x1b9/0x230 [ 32.438190][ T92] bus_for_each_drv+0x156/0x1d0 [ 32.438708][ T92] ? bus_rescan_devices+0x20/0x20 [ 32.439248][ T92] ? lockdep_hardirqs_on+0x388/0x570 [ 32.439812][ T92] __device_attach+0x20b/0x350 [ 32.440323][ T92] ? device_bind_driver+0xc0/0xc0 [ 32.440870][ T92] bus_probe_device+0x1e5/0x290 [ 32.441386][ T92] device_add+0x1420/0x1b90 [ 32.441887][ T92] ? wait_for_completion+0x3c0/0x3c0 [ 32.442466][ T92] ? device_link_remove+0x150/0x150 [ 32.443037][ T92] usb_set_configuration+0xd6f/0x1750 [ 32.443633][ T92] generic_probe+0x95/0xcd [ 32.444146][ T92] usb_probe_device+0x97/0xf0 [ 32.444650][ T92] ? usb_suspend+0x630/0x630 [ 32.445151][ T92] really_probe+0x281/0x700 [ 32.445642][ T92] ? driver_allows_async_probing+0x150/0x150 [ 32.446299][ T92] driver_probe_device+0x105/0x200 [ 32.446857][ T92] __device_attach_driver+0x1b9/0x230 [ 32.447448][ T92] bus_for_each_drv+0x156/0x1d0 [ 32.447981][ T92] ? bus_rescan_devices+0x20/0x20 [ 32.448523][ T92] ? lockdep_hardirqs_on+0x388/0x570 [ 32.449095][ T92] __device_attach+0x20b/0x350 [ 32.449612][ T92] ? device_bind_driver+0xc0/0xc0 [ 32.450167][ T92] bus_probe_device+0x1e5/0x290 [ 32.450686][ T92] device_add+0x1420/0x1b90 [ 32.451164][ T92] ? device_link_remove+0x150/0x150 [ 32.451715][ T92] ? _raw_spin_unlock_irq+0x1f/0x30 [ 32.452267][ T92] usb_new_device.cold.65+0x66e/0xe63 [ 32.452835][ T92] hub_event+0x1ebd/0x3810 [ 32.453300][ T92] ? hub_port_debounce+0x270/0x270 [ 32.453837][ T92] ? __lock_acquire+0xeda/0x41a0 [ 32.454389][ T92] ? find_held_lock+0x33/0x1c0 [ 32.454904][ T92] ? process_one_work+0x8fc/0x1720 [ 32.455445][ T92] ? mark_held_locks+0x110/0x110 [ 32.455954][ T92] ? rcu_read_lock_sched_held+0x9c/0xd0 [ 32.456536][ T92] ? rcu_read_lock_bh_held+0xb0/0xb0 [ 32.457093][ T92] process_one_work+0x9f2/0x1720 [ 32.457616][ T92] ? mark_held_locks+0x110/0x110 [ 32.458138][ T92] ? pwq_dec_nr_in_flight+0x310/0x310 [ 32.458701][ T92] ? do_raw_spin_lock+0x11b/0x280 [ 32.459237][ T92] worker_thread+0x8c/0xd10 [ 32.459715][ T92] ? process_one_work+0x1720/0x1720 [ 32.460266][ T92] kthread+0x352/0x420 [ 32.460702][ T92] ? kthread_create_on_node+0xe0/0xe0 [ 32.461275][ T92] ret_from_fork+0x24/0x30 [ 32.461738][ T92] irq event stamp: 2238 [ 32.462183][ T92] hardirqs last enabled at (2237): [<ffffffff81293b92>] console_unlock+0x8f2/0xc40 [ 32.463174][ T92] hardirqs last disabled at (2238): [<ffffffff8100468d>] trace_hardirqs_off_thunk+0x1a/0x1c [ 32.464244][ T92] softirqs last enabled at (1196): [<ffffffff85c00643>] __do_softirq+0x643/0x8fc [ 32.465225][ T92] softirqs last disabled at (1187): [<ffffffff8115a035>] irq_exit+0x175/0x1a0 [ 32.466155][ T92] ---[ end trace ef28d8c60b68a46d ]--- [ 32.466781][ T92] uvcvideo: No valid video chain found. [ 32.468076][ T92] usb 1-1: USB disconnect, device number 2
After:
bash-5.0# ./repro [ 19.067221][ T92] usb 1-1: new high-speed USB device number 2 using dummy_hcd [ 19.307154][ T92] usb 1-1: Using ep0 maxpacket: 8 [ 19.427261][ T92] usb 1-1: config index 0 descriptor too short (expected 51150, got 70) [ 19.429709][ T92] usb 1-1: config 0 contains an unexpected descriptor of type 0x2, skipping [ 19.432150][ T92] usb 1-1: config 0 has an invalid descriptor of length 0, skipping remainder of the config [ 19.435003][ T92] usb 1-1: config 0 interface 0 altsetting 0 has 0 endpoint descriptors, different from the interface descriptor's value: 16 [ 19.438655][ T92] usb 1-1: New USB device found, idVendor=0bd3, idProduct=0755, bcdDevice=69.f1 [ 19.441166][ T92] usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0 [ 19.445163][ T92] usb 1-1: config 0 descriptor?? [ 19.717195][ T92] usb 1-1: string descriptor 0 read error: -71 [ 19.719038][ T92] uvcvideo: Found UVC 0.00 device <unnamed> (0bd3:0755) [ 19.721087][ T92] uvcvideo: No valid video chain found. [ 19.725262][ T92] usb 1-1: USB disconnect, device number 2
On Wed, Dec 18, 2019 at 11:41:38AM +0000, Will Deacon wrote:
Ok, that's a good poke for me to go review that raw gadget code to see if it can be merged upstream :)
Laurent -- can we please merge this now?
Yes, that would be good to have, as this obviously fixes a problem, and I can take it off of my "patches to track" list....
thanks,
greg k-h
On Wed, Dec 18, 2019 at 1:23 PM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
Thanks! An easier way to test the patch would be to issue a syz test command, but I'm glad you managed to set up raw gadget manually and it worked for you.
Ok, that's a good poke for me to go review that raw gadget code to see if it can be merged upstream :)
Looking forward to it! =)
On Wed, Dec 18, 2019 at 01:46:00PM +0100, Andrey Konovalov wrote:
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
and merged in my tree. I'm so sorry for the way too long delay.
linux-stable-mirror@lists.linaro.org