diff mbox series

[RESEND,RESEND] media: uvc: Avoid cyclic entity chains due to malformed USB descriptors

Message ID 20191108154838.21487-1-will@kernel.org (mailing list archive)
State Accepted, archived
Delegated to: Laurent Pinchart
Headers
Series [RESEND,RESEND] media: uvc: Avoid cyclic entity chains due to malformed USB descriptors |

Commit Message

Will Deacon Nov. 8, 2019, 3:48 p.m. UTC 
  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-PjUta7EOb7A@mail.gmail.com/
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(+)

Comments

Laurent Pinchart Nov. 8, 2019, 3:55 p.m. UTC | #1 
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:
> 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-PjUta7EOb7A@mail.gmail.com/
> 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);
>  
> -- 
> 2.24.0.rc1.363.gb1bccd3e3d-goog
>
Will Deacon Nov. 8, 2019, 3:59 p.m. UTC | #2 
Hi Laurent,

On Fri, Nov 08, 2019 at 05:55:03PM +0200, Laurent Pinchart wrote:
> 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.

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-the-truck/
Will Deacon Dec. 16, 2019, 12:16 p.m. UTC | #3 
Hi Laurent,

On Fri, Nov 08, 2019 at 05:55:03PM +0200, Laurent Pinchart wrote:
> 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.

Gentle reminder on this, now you've been back a month ;)

Will
Andrey Konovalov Dec. 16, 2019, 1:17 p.m. UTC | #4 
On Mon, Dec 16, 2019 at 1:16 PM Will Deacon <will@kernel.org> wrote:
>
> Hi Laurent,
>
> On Fri, Nov 08, 2019 at 05:55:03PM +0200, Laurent Pinchart wrote:
> > 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.
>
> Gentle reminder on this, now you've been back a month ;)

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!
Will Deacon Dec. 18, 2019, 11:41 a.m. UTC | #5 
On Mon, Dec 16, 2019 at 02:17:52PM +0100, Andrey Konovalov wrote:
> On Mon, Dec 16, 2019 at 1:16 PM Will Deacon <will@kernel.org> wrote:
> > On Fri, Nov 08, 2019 at 05:55:03PM +0200, Laurent Pinchart wrote:
> > > 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.
> >
> > Gentle reminder on this, now you've been back a month ;)
> 
> 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 :)

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
Greg KH Dec. 18, 2019, 12:23 p.m. UTC | #6 
On Wed, Dec 18, 2019 at 11:41:38AM +0000, Will Deacon wrote:
> On Mon, Dec 16, 2019 at 02:17:52PM +0100, Andrey Konovalov wrote:
> > On Mon, Dec 16, 2019 at 1:16 PM Will Deacon <will@kernel.org> wrote:
> > > On Fri, Nov 08, 2019 at 05:55:03PM +0200, Laurent Pinchart wrote:
> > > > 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.
> > >
> > > Gentle reminder on this, now you've been back a month ;)
> > 
> > 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 :)
> 
> 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.

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
Andrey Konovalov Dec. 18, 2019, 12:46 p.m. UTC | #7 
On Wed, Dec 18, 2019 at 1:23 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Dec 18, 2019 at 11:41:38AM +0000, Will Deacon wrote:
> > On Mon, Dec 16, 2019 at 02:17:52PM +0100, Andrey Konovalov wrote:
> > > On Mon, Dec 16, 2019 at 1:16 PM Will Deacon <will@kernel.org> wrote:
> > > > On Fri, Nov 08, 2019 at 05:55:03PM +0200, Laurent Pinchart wrote:
> > > > > 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.
> > > >
> > > > Gentle reminder on this, now you've been back a month ;)
> > >
> > > 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 :)
> >
> > 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.

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! =)

>
> > 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
Laurent Pinchart Dec. 18, 2019, 4:51 p.m. UTC | #8 
On Wed, Dec 18, 2019 at 01:46:00PM +0100, Andrey Konovalov wrote:
> On Wed, Dec 18, 2019 at 1:23 PM Greg Kroah-Hartman wrote:
> > On Wed, Dec 18, 2019 at 11:41:38AM +0000, Will Deacon wrote:
> >> On Mon, Dec 16, 2019 at 02:17:52PM +0100, Andrey Konovalov wrote:
> >>> On Mon, Dec 16, 2019 at 1:16 PM Will Deacon <will@kernel.org> wrote:
> >>>> On Fri, Nov 08, 2019 at 05:55:03PM +0200, Laurent Pinchart wrote:
> >>>>> 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.
> >>>>
> >>>> Gentle reminder on this, now you've been back a month ;)
> >>>
> >>> 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 :)
> >>
> >> 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.
> 
> 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! =)

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

and merged in my tree. I'm so sorry for the way too long delay.

> >> 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....
Will Deacon Dec. 18, 2019, 4:59 p.m. UTC | #9 
On Wed, Dec 18, 2019 at 06:51:53PM +0200, Laurent Pinchart wrote:
> On Wed, Dec 18, 2019 at 01:46:00PM +0100, Andrey Konovalov wrote:
> > On Wed, Dec 18, 2019 at 1:23 PM Greg Kroah-Hartman wrote:
> > > On Wed, Dec 18, 2019 at 11:41:38AM +0000, Will Deacon wrote:
> > >> On Mon, Dec 16, 2019 at 02:17:52PM +0100, Andrey Konovalov wrote:
> > >>> On Mon, Dec 16, 2019 at 1:16 PM Will Deacon <will@kernel.org> wrote:
> > >>>> On Fri, Nov 08, 2019 at 05:55:03PM +0200, Laurent Pinchart wrote:
> > >>>>> 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.
> > >>>>
> > >>>> Gentle reminder on this, now you've been back a month ;)
> > >>>
> > >>> 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 :)
> > >>
> > >> 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.
> > 
> > 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! =)
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> and merged in my tree. I'm so sorry for the way too long delay.

Thanks, Laurent.

Will
Will Deacon Jan. 21, 2020, 7:01 p.m. UTC | #10 
Hi Laurent,

On Wed, Dec 18, 2019 at 06:51:53PM +0200, Laurent Pinchart wrote:
> On Wed, Dec 18, 2019 at 01:46:00PM +0100, Andrey Konovalov wrote:
> > On Wed, Dec 18, 2019 at 1:23 PM Greg Kroah-Hartman wrote:
> > > On Wed, Dec 18, 2019 at 11:41:38AM +0000, Will Deacon wrote:
> > >> On Mon, Dec 16, 2019 at 02:17:52PM +0100, Andrey Konovalov wrote:
> > >>> On Mon, Dec 16, 2019 at 1:16 PM Will Deacon <will@kernel.org> wrote:
> > >>>> On Fri, Nov 08, 2019 at 05:55:03PM +0200, Laurent Pinchart wrote:
> > >>>>> 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.
> > >>>>
> > >>>> Gentle reminder on this, now you've been back a month ;)
> > >>>
> > >>> 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 :)
> > >>
> > >> 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.
> > 
> > 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! =)
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> and merged in my tree. I'm so sorry for the way too long delay.

Please can you send this upstream and/or put it in linux-next? I can't see
it anywhere at the moment :(

Thanks,

Will
Laurent Pinchart Jan. 21, 2020, 10:53 p.m. UTC | #11 
Hi Will,

On Tue, Jan 21, 2020 at 07:01:42PM +0000, Will Deacon wrote:
> On Wed, Dec 18, 2019 at 06:51:53PM +0200, Laurent Pinchart wrote:
> > On Wed, Dec 18, 2019 at 01:46:00PM +0100, Andrey Konovalov wrote:
> > > On Wed, Dec 18, 2019 at 1:23 PM Greg Kroah-Hartman wrote:
> > > > On Wed, Dec 18, 2019 at 11:41:38AM +0000, Will Deacon wrote:
> > > >> On Mon, Dec 16, 2019 at 02:17:52PM +0100, Andrey Konovalov wrote:
> > > >>> On Mon, Dec 16, 2019 at 1:16 PM Will Deacon <will@kernel.org> wrote:
> > > >>>> On Fri, Nov 08, 2019 at 05:55:03PM +0200, Laurent Pinchart wrote:
> > > >>>>> 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.
> > > >>>>
> > > >>>> Gentle reminder on this, now you've been back a month ;)
> > > >>>
> > > >>> 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 :)
> > > >>
> > > >> 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.
> > > 
> > > 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! =)
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > and merged in my tree. I'm so sorry for the way too long delay.
> 
> Please can you send this upstream and/or put it in linux-next? I can't see
> it anywhere at the moment :(

I've now sent the pull request. Seems I failed the schedule from A to Z
with this patch. I'm extremely sorry :-(
Will Deacon Jan. 22, 2020, 8:58 a.m. UTC | #12 
On Wed, Jan 22, 2020 at 12:53:05AM +0200, Laurent Pinchart wrote:
> On Tue, Jan 21, 2020 at 07:01:42PM +0000, Will Deacon wrote:
> > On Wed, Dec 18, 2019 at 06:51:53PM +0200, Laurent Pinchart wrote:
> > > On Wed, Dec 18, 2019 at 01:46:00PM +0100, Andrey Konovalov wrote:
> > > > On Wed, Dec 18, 2019 at 1:23 PM Greg Kroah-Hartman wrote:
> > > > > On Wed, Dec 18, 2019 at 11:41:38AM +0000, Will Deacon wrote:
> > > > >> On Mon, Dec 16, 2019 at 02:17:52PM +0100, Andrey Konovalov wrote:
> > > > >>> On Mon, Dec 16, 2019 at 1:16 PM Will Deacon <will@kernel.org> wrote:
> > > > >>>> On Fri, Nov 08, 2019 at 05:55:03PM +0200, Laurent Pinchart wrote:
> > > > >>>>> 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.
> > > > >>>>
> > > > >>>> Gentle reminder on this, now you've been back a month ;)
> > > > >>>
> > > > >>> 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 :)
> > > > >>
> > > > >> 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.
> > > > 
> > > > 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! =)
> > > 
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > 
> > > and merged in my tree. I'm so sorry for the way too long delay.
> > 
> > Please can you send this upstream and/or put it in linux-next? I can't see
> > it anywhere at the moment :(
> 
> I've now sent the pull request.

Thanks, Laurent.

> Seems I failed the schedule from A to Z with this patch. I'm extremely
> sorry :-(

Well, at least you were consistent ;)

Will
diff mbox series

Patch

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);