[PATCH] media: Fix invalid link creation when source entity has 0 pads

[PATCH] media: Fix invalid link creation when source entity has 0 pads

[gshahrouzi]

[-- Attachment #1: Type: text/plain, Size: 1550 bytes --]

>From 307209d175be0145e36b9cf95944e2e62afeab11 Mon Sep 17 00:00:00 2001
From: Gabriel Shahrouzi <gshahrouzi@gmail.com>
Date: Mon, 24 Mar 2025 19:45:55 -0400
Subject: [PATCH] media: Fix invalid link creation when source entity has 0
 pads

This patch addresses the warning triggered in the media_create_pad_link()
function, specifically related to the check WARN_ON(source_pad >=
source->num_pads). The fix proposed adds an additional check to ensure that
source->num_pads is non-zero before proceeding with the
media_create_pad_link() function.

Reported-by: syzbot+701fc9cc0cb44e2b0fe9@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=701fc9cc0cb44e2b0fe9
Tested-by: syzbot+701fc9cc0cb44e2b0fe9@syzkaller.appspotmail.com
Fixes: a3fbc2e6bb05 ("media: mc-entity.c: use WARN_ON, validate link pads")
Signed-off-by: Gabriel Shahrouzi <gshahrouzi@gmail.com>
---
 drivers/media/usb/uvc/uvc_entity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/uvc/uvc_entity.c b/drivers/media/usb/uvc/uvc_entity.c
index cc68dd24eb42..5397ce76c218 100644
--- a/drivers/media/usb/uvc/uvc_entity.c
+++ b/drivers/media/usb/uvc/uvc_entity.c
@@ -43,7 +43,7 @@ static int uvc_mc_create_links(struct uvc_video_chain *chain,
 		source = (UVC_ENTITY_TYPE(remote) == UVC_TT_STREAMING)
 		       ? (remote->vdev ? &remote->vdev->entity : NULL)
 		       : &remote->subdev.entity;
-		if (source == NULL)
+		if (source == NULL || source->num_pads == 0)
 			continue;
 
 		remote_pad = remote->num_pads - 1;
-- 
2.43.0

Re: [PATCH] media: Fix invalid link creation when source entity has 0 pads

[Ricardo Ribalda]

Hi Gabriel

On Tue, 25 Mar 2025 at 08:55, <gshahrouzi@gmail.com> wrote:
>
> >From 307209d175be0145e36b9cf95944e2e62afeab11 Mon Sep 17 00:00:00 2001
> From: Gabriel Shahrouzi <gshahrouzi@gmail.com>
> Date: Mon, 24 Mar 2025 19:45:55 -0400
> Subject: [PATCH] media: Fix invalid link creation when source entity has 0
>  pads
>
> This patch addresses the warning triggered in the media_create_pad_link()
> function, specifically related to the check WARN_ON(source_pad >=
> source->num_pads). The fix proposed adds an additional check to ensure that
> source->num_pads is non-zero before proceeding with the
> media_create_pad_link() function.
>
> Reported-by: syzbot+701fc9cc0cb44e2b0fe9@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=701fc9cc0cb44e2b0fe9
I cannot reach that URL
> Tested-by: syzbot+701fc9cc0cb44e2b0fe9@syzkaller.appspotmail.com
> Fixes: a3fbc2e6bb05 ("media: mc-entity.c: use WARN_ON, validate link pads")
Shouldn't it be? :
Fixes: 4ffc2d89f38a ("[media] uvcvideo: Register subdevices for each entity")

> Signed-off-by: Gabriel Shahrouzi <gshahrouzi@gmail.com>
> ---
>  drivers/media/usb/uvc/uvc_entity.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_entity.c b/drivers/media/usb/uvc/uvc_entity.c
> index cc68dd24eb42..5397ce76c218 100644
> --- a/drivers/media/usb/uvc/uvc_entity.c
> +++ b/drivers/media/usb/uvc/uvc_entity.c
> @@ -43,7 +43,7 @@ static int uvc_mc_create_links(struct uvc_video_chain *chain,
>                 source = (UVC_ENTITY_TYPE(remote) == UVC_TT_STREAMING)
>                        ? (remote->vdev ? &remote->vdev->entity : NULL)
>                        : &remote->subdev.entity;
> -               if (source == NULL)
> +               if (source == NULL || source->num_pads == 0)
Shouldn't source->num_pads be the same as remote->num_pads?

Are you sure that your kernel does not contain?
https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/media/usb/uvc/uvc_entity.c?id=41ddb251c68ac75c101d3a50a68c4629c9055e4c

Regards!

>                         continue;
>
>                 remote_pad = remote->num_pads - 1;
> --
> 2.43.0
>


-- 
Ricardo Ribalda

Re: [PATCH] media: Fix invalid link creation when source entity has 0 pads

[Gabriel]

Hi Ricardo,

> I cannot reach that URL
I was unable to access the URL from my email client when I initially
sent the email, but a couple of hours later, I was able to. Initially,
copying and pasting the URL into the browser provided a workaround.

> Shouldn't it be?:
> Fixes: 4ffc2d89f38a ("[media] uvcvideo: Register subdevices for each entity")
You're right, I incorrectly referenced the wrong commit. However, I’m
not certain if it should reference a96aa5342d57 (Fixes: a96aa5342d57 -
'[media] uvcvideo: Ignore entities for terminals with no supported
format') as it's the latest commit affecting the line I'm changing or
the one you mentioned.

> Shouldn't source->num_pads be the same as remote->num_pads?
The fuzzer (Syzkaller) that triggered the warning appears to have
encountered a case where source->num_pads and remote->num_pads were
different. When analyzing the case in GDB, remote->num_pads was 1,
while source->num_pads was 0.

> Are you sure that your kernel does not contain?
> https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/media/usb/uvc/uvc_entity.c?id=41ddb251c68ac75c101d3a50a68c4629c9055e4c
Yes, it should be included since I am running the upstream kernel.

Regards,
Gabriel

Re: [PATCH] media: Fix invalid link creation when source entity has 0 pads

[Laurent Pinchart]

On Tue, Mar 25, 2025 at 06:05:00PM -0400, Gabriel wrote:
> Hi Ricardo,
> 
> > I cannot reach that URL
> I was unable to access the URL from my email client when I initially
> sent the email, but a couple of hours later, I was able to. Initially,
> copying and pasting the URL into the browser provided a workaround.
> 
> > Shouldn't it be?:
> > Fixes: 4ffc2d89f38a ("[media] uvcvideo: Register subdevices for each entity")
> You're right, I incorrectly referenced the wrong commit. However, I’m
> not certain if it should reference a96aa5342d57 (Fixes: a96aa5342d57 -
> '[media] uvcvideo: Ignore entities for terminals with no supported
> format') as it's the latest commit affecting the line I'm changing or
> the one you mentioned.
> 
> > Shouldn't source->num_pads be the same as remote->num_pads?
> The fuzzer (Syzkaller) that triggered the warning appears to have
> encountered a case where source->num_pads and remote->num_pads were
> different. When analyzing the case in GDB, remote->num_pads was 1,
> while source->num_pads was 0.

This seems like the real bug that should be fixed.

> > Are you sure that your kernel does not contain?
> > https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/media/usb/uvc/uvc_entity.c?id=41ddb251c68ac75c101d3a50a68c4629c9055e4c
> Yes, it should be included since I am running the upstream kernel.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: Fix invalid link creation when source entity has 0 pads

[Gabriel]

Hi Laurent,

I’ve analyzed the bug report, and the root cause of the
"WARNING-media_create_pad_link" issue is a mismatch in terminal
references in the USB descriptor.

The format type descriptor references terminal ID 6, while the audio
streaming interface descriptor points to terminal ID 5. This
discrepancy triggers the warning: "No streaming interface found for
terminal 6", followed by the media pad link warning.

I confirmed this by changing the terminal ID in the format descriptor
from 6 to 5, which eliminates both warnings. This shows the warning is
correctly identifying an invalid descriptor configuration, not a
kernel bug.

Since the USB descriptor is invalid, I believe the warning is
necessary and should remain. The code should stay as is.

Regards,
Gabriel

On Tue, Mar 25, 2025 at 8:13 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Tue, Mar 25, 2025 at 06:05:00PM -0400, Gabriel wrote:
> > Hi Ricardo,
> >
> > > I cannot reach that URL
> > I was unable to access the URL from my email client when I initially
> > sent the email, but a couple of hours later, I was able to. Initially,
> > copying and pasting the URL into the browser provided a workaround.
> >
> > > Shouldn't it be?:
> > > Fixes: 4ffc2d89f38a ("[media] uvcvideo: Register subdevices for each entity")
> > You're right, I incorrectly referenced the wrong commit. However, I’m
> > not certain if it should reference a96aa5342d57 (Fixes: a96aa5342d57 -
> > '[media] uvcvideo: Ignore entities for terminals with no supported
> > format') as it's the latest commit affecting the line I'm changing or
> > the one you mentioned.
> >
> > > Shouldn't source->num_pads be the same as remote->num_pads?
> > The fuzzer (Syzkaller) that triggered the warning appears to have
> > encountered a case where source->num_pads and remote->num_pads were
> > different. When analyzing the case in GDB, remote->num_pads was 1,
> > while source->num_pads was 0.
>
> This seems like the real bug that should be fixed.
>
> > > Are you sure that your kernel does not contain?
> > > https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/media/usb/uvc/uvc_entity.c?id=41ddb251c68ac75c101d3a50a68c4629c9055e4c
> > Yes, it should be included since I am running the upstream kernel.
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH] media: Fix invalid link creation when source entity has 0 pads

[Laurent Pinchart]

Hi Gabriel,

On Sat, Mar 29, 2025 at 01:50:00PM -0400, Gabriel wrote:
> Hi Laurent,
> 
> I’ve analyzed the bug report, and the root cause of the
> "WARNING-media_create_pad_link" issue is a mismatch in terminal
> references in the USB descriptor.
> 
> The format type descriptor references terminal ID 6, while the audio
> streaming interface descriptor points to terminal ID 5. This
> discrepancy triggers the warning: "No streaming interface found for
> terminal 6", followed by the media pad link warning.

Can you share the USB descriptors.

> I confirmed this by changing the terminal ID in the format descriptor
> from 6 to 5, which eliminates both warnings. This shows the warning is
> correctly identifying an invalid descriptor configuration, not a
> kernel bug.

There's still something not quite right. uvc_entity->num_pads should
always be equal to the corresponding media_entity->num_pads. That's not
the case here, and I think it indicates a bug.

> Since the USB descriptor is invalid, I believe the warning is
> necessary and should remain. The code should stay as is.

There should be a warning, but I think it needs to be caught in a
different place, earlier.

> On Tue, Mar 25, 2025 at 8:13 PM Laurent Pinchart wrote:
> >
> > On Tue, Mar 25, 2025 at 06:05:00PM -0400, Gabriel wrote:
> > > Hi Ricardo,
> > >
> > > > I cannot reach that URL
> > > I was unable to access the URL from my email client when I initially
> > > sent the email, but a couple of hours later, I was able to. Initially,
> > > copying and pasting the URL into the browser provided a workaround.
> > >
> > > > Shouldn't it be?:
> > > > Fixes: 4ffc2d89f38a ("[media] uvcvideo: Register subdevices for each entity")
> > > You're right, I incorrectly referenced the wrong commit. However, I’m
> > > not certain if it should reference a96aa5342d57 (Fixes: a96aa5342d57 -
> > > '[media] uvcvideo: Ignore entities for terminals with no supported
> > > format') as it's the latest commit affecting the line I'm changing or
> > > the one you mentioned.
> > >
> > > > Shouldn't source->num_pads be the same as remote->num_pads?
> > > The fuzzer (Syzkaller) that triggered the warning appears to have
> > > encountered a case where source->num_pads and remote->num_pads were
> > > different. When analyzing the case in GDB, remote->num_pads was 1,
> > > while source->num_pads was 0.
> >
> > This seems like the real bug that should be fixed.
> >
> > > > Are you sure that your kernel does not contain?
> > > > https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/media/usb/uvc/uvc_entity.c?id=41ddb251c68ac75c101d3a50a68c4629c9055e4c
> > > Yes, it should be included since I am running the upstream kernel.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: Fix invalid link creation when source entity has 0 pads

[Gabriel Shahrouzi]

On Tue, Apr 1, 2025 at 8:30 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Gabriel,
>
> On Sat, Mar 29, 2025 at 01:50:00PM -0400, Gabriel wrote:
> > Hi Laurent,
> >
> > I’ve analyzed the bug report, and the root cause of the
> > "WARNING-media_create_pad_link" issue is a mismatch in terminal
> > references in the USB descriptor.
> >
> > The format type descriptor references terminal ID 6, while the audio
> > streaming interface descriptor points to terminal ID 5. This
> > discrepancy triggers the warning: "No streaming interface found for
> > terminal 6", followed by the media pad link warning.
>
> Can you share the USB descriptors.
The USB descriptors via the Syzkaller reproducer:
         "\x12\x01\x00\x00\xfb\x5d\x7d\x08\x6d\x04\xc3\x08\x16\x6b\x01\x02\x03"
         "\x01\x09\x02\x50\x00\x01\x00\x00\x00\x00\x09\x04\x1f\x00\x00\xff\x01"
         "\x00\x00\x0a\x24\x02\x00\x00\x05\x02\x01\x02\x07\x24\x07\x05\x00\x00"
         "\x18\xc2\x24\x08\x05\x04\x00\x04\x96\x0d\x24\x06\x01\x01\x03\x02\x00"
         "\x01\x00\x06\x00\x06\x09\x24\x03\x05\x05\x03\x06\x05\x81\x09\x24\x03"
         "\x06\x01\x01\x04\x05\x05\x07\x24\x04\x05\x01\x00\x9c\xbd\x89"
>
> > I confirmed this by changing the terminal ID in the format descriptor
> > from 6 to 5, which eliminates both warnings. This shows the warning is
> > correctly identifying an invalid descriptor configuration, not a
> > kernel bug.
>
> There's still something not quite right. uvc_entity->num_pads should
> always be equal to the corresponding media_entity->num_pads. That's not
> the case here, and I think it indicates a bug.
Ah ok - the mismatch itself shouldn't happen regardless of the descriptor
>
> > Since the USB descriptor is invalid, I believe the warning is
> > necessary and should remain. The code should stay as is.
>
> There should be a warning, but I think it needs to be caught in a
> different place, earlier.
Got it.
>
> > On Tue, Mar 25, 2025 at 8:13 PM Laurent Pinchart wrote:
> > >
> > > On Tue, Mar 25, 2025 at 06:05:00PM -0400, Gabriel wrote:
> > > > Hi Ricardo,
> > > >
> > > > > I cannot reach that URL
> > > > I was unable to access the URL from my email client when I initially
> > > > sent the email, but a couple of hours later, I was able to. Initially,
> > > > copying and pasting the URL into the browser provided a workaround.
> > > >
> > > > > Shouldn't it be?:
> > > > > Fixes: 4ffc2d89f38a ("[media] uvcvideo: Register subdevices for each entity")
> > > > You're right, I incorrectly referenced the wrong commit. However, I’m
> > > > not certain if it should reference a96aa5342d57 (Fixes: a96aa5342d57 -
> > > > '[media] uvcvideo: Ignore entities for terminals with no supported
> > > > format') as it's the latest commit affecting the line I'm changing or
> > > > the one you mentioned.
> > > >
> > > > > Shouldn't source->num_pads be the same as remote->num_pads?
> > > > The fuzzer (Syzkaller) that triggered the warning appears to have
> > > > encountered a case where source->num_pads and remote->num_pads were
> > > > different. When analyzing the case in GDB, remote->num_pads was 1,
> > > > while source->num_pads was 0.
> > >
> > > This seems like the real bug that should be fixed.
> > >
> > > > > Are you sure that your kernel does not contain?
> > > > > https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/media/usb/uvc/uvc_entity.c?id=41ddb251c68ac75c101d3a50a68c4629c9055e4c
> > > > Yes, it should be included since I am running the upstream kernel.
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH] media: Fix invalid link creation when source entity has 0 pads

[Laurent Pinchart]

Hi Gabriel,

On Tue, Apr 08, 2025 at 01:35:00AM -0400, Gabriel Shahrouzi wrote:
> On Tue, Apr 1, 2025 at 8:30 PM Laurent Pinchart wrote:
> > On Sat, Mar 29, 2025 at 01:50:00PM -0400, Gabriel wrote:
> > > Hi Laurent,
> > >
> > > I’ve analyzed the bug report, and the root cause of the
> > > "WARNING-media_create_pad_link" issue is a mismatch in terminal
> > > references in the USB descriptor.
> > >
> > > The format type descriptor references terminal ID 6, while the audio
> > > streaming interface descriptor points to terminal ID 5. This
> > > discrepancy triggers the warning: "No streaming interface found for
> > > terminal 6", followed by the media pad link warning.
> >
> > Can you share the USB descriptors.
>
> The USB descriptors via the Syzkaller reproducer:
>          "\x12\x01\x00\x00\xfb\x5d\x7d\x08\x6d\x04\xc3\x08\x16\x6b\x01\x02\x03"
>          "\x01\x09\x02\x50\x00\x01\x00\x00\x00\x00\x09\x04\x1f\x00\x00\xff\x01"
>          "\x00\x00\x0a\x24\x02\x00\x00\x05\x02\x01\x02\x07\x24\x07\x05\x00\x00"
>          "\x18\xc2\x24\x08\x05\x04\x00\x04\x96\x0d\x24\x06\x01\x01\x03\x02\x00"
>          "\x01\x00\x06\x00\x06\x09\x24\x03\x05\x05\x03\x06\x05\x81\x09\x24\x03"
>          "\x06\x01\x01\x04\x05\x05\x07\x24\x04\x05\x01\x00\x9c\xbd\x89"

If I haven't made any mistake in the manual decode process (is there any
Linux tool that can decode a binary descriptors dump the same way lsusb
decodes descriptors from a device ?), the relevant UVC descriptors there
are

0x0a		bLength
0x24		bDescriptorType USB_DT_CS_INTERFACE
0x02		bDescriptorSubtype VC_INPUT_TERMINAL
0x00		bTerminalID 0 (invalid)
0x00, 0x05	bTerminalType 0x0500 (invalid)
0x02		bAssocTerminal 2 (invalid)
0x01		iTerminal 1 
0x02, 0x07

0x09		bLength
0x24		bDescriptorType USB_DT_CS_INTERFACE
0x03		bDescriptorSubtype VC_OUTPUT_TERMINAL 
0x06		bTerminalID 6 
0x01, 0x01	bTerminalType TT_STREAMING
0x04		bAssocTerminal 4 (invalid)
0x05		bSourceID 5 
0x05		iTerminal 5

0x07		bLength
0x24		bDescriptorType USB_DT_CS_INTERFACE
0x04		bDescriptorSubtype VC_SELECTOR_UNIT 
0x05		bUnitID 5 
0x01		bNrInPins 1 
0x00		baSourceID(1) 0 
0x9c		iSelector 156

Ignoring a few invalid values (bTerminalID shouldb't be 0, bTerminalType
0x0500 is defined by the specification, and the two bAssocTerminal ids
are also invalid), this creates the following chain:

VC_INPUT_TERMINAL (0) -> VC_SELECTOR_UNIT (5) -> VC_OUTPUT_TERMINAL (6)

Looking at uvc_mc_init_entity() where the media_entity->num_pads field
gets assigned by calling media_entity_pads_init(), a media entity is
only initialized when the entity type is not TT_STREAMING (so it's a
subdev), or when the entity has an associated video device. I think
that what is happening here is that the second entity in the above list
(VC_OUTPUT_TERMINAL, id 6) fails to initialize properly in
uvc_register_terms() is there is no corresponding streaming interface in
the device. This is confirmed by the

usb 1-1: No streaming interface found for terminal 6.

message in the syzbot kernel log. No video device is created for the
terminal, and no media_entity is initialized. Trying to later link the
entity in uvc_mc_create_links() then fails.

I don't want to address this in uvc_mc_create_links() as the invalid
terminal in the chain means we could have other issues elsewhere. One
option is to fail turn the missing streaming interface check in a hard
failure, at least for the chain being registered. The driver could still
proceed to registering other chains.

There's a small risk of regression for buggy devices. If that's a
problem, we could instead remove invalid terminals from the device
entities list before we proceed to scanning chains.

> > > I confirmed this by changing the terminal ID in the format descriptor
> > > from 6 to 5, which eliminates both warnings. This shows the warning is
> > > correctly identifying an invalid descriptor configuration, not a
> > > kernel bug.
> >
> > There's still something not quite right. uvc_entity->num_pads should
> > always be equal to the corresponding media_entity->num_pads. That's not
> > the case here, and I think it indicates a bug.
>
> Ah ok - the mismatch itself shouldn't happen regardless of the descriptor
>
> > > Since the USB descriptor is invalid, I believe the warning is
> > > necessary and should remain. The code should stay as is.
> >
> > There should be a warning, but I think it needs to be caught in a
> > different place, earlier.
>
> Got it.
>
> > > On Tue, Mar 25, 2025 at 8:13 PM Laurent Pinchart wrote:
> > > > On Tue, Mar 25, 2025 at 06:05:00PM -0400, Gabriel wrote:
> > > > > Hi Ricardo,
> > > > >
> > > > > > I cannot reach that URL
> > > > > I was unable to access the URL from my email client when I initially
> > > > > sent the email, but a couple of hours later, I was able to. Initially,
> > > > > copying and pasting the URL into the browser provided a workaround.
> > > > >
> > > > > > Shouldn't it be?:
> > > > > > Fixes: 4ffc2d89f38a ("[media] uvcvideo: Register subdevices for each entity")
> > > > > You're right, I incorrectly referenced the wrong commit. However, I’m
> > > > > not certain if it should reference a96aa5342d57 (Fixes: a96aa5342d57 -
> > > > > '[media] uvcvideo: Ignore entities for terminals with no supported
> > > > > format') as it's the latest commit affecting the line I'm changing or
> > > > > the one you mentioned.
> > > > >
> > > > > > Shouldn't source->num_pads be the same as remote->num_pads?
> > > > > The fuzzer (Syzkaller) that triggered the warning appears to have
> > > > > encountered a case where source->num_pads and remote->num_pads were
> > > > > different. When analyzing the case in GDB, remote->num_pads was 1,
> > > > > while source->num_pads was 0.
> > > >
> > > > This seems like the real bug that should be fixed.
> > > >
> > > > > > Are you sure that your kernel does not contain?
> > > > > > https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/media/usb/uvc/uvc_entity.c?id=41ddb251c68ac75c101d3a50a68c4629c9055e4c
> > > > > Yes, it should be included since I am running the upstream kernel.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: Fix invalid link creation when source entity has 0 pads

[Gabriel Shahrouzi]

> > > Can you share the USB descriptors.
> >
> > The USB descriptors via the Syzkaller reproducer:
> >          "\x12\x01\x00\x00\xfb\x5d\x7d\x08\x6d\x04\xc3\x08\x16\x6b\x01\x02\x03"
> >          "\x01\x09\x02\x50\x00\x01\x00\x00\x00\x00\x09\x04\x1f\x00\x00\xff\x01"
> >          "\x00\x00\x0a\x24\x02\x00\x00\x05\x02\x01\x02\x07\x24\x07\x05\x00\x00"
> >          "\x18\xc2\x24\x08\x05\x04\x00\x04\x96\x0d\x24\x06\x01\x01\x03\x02\x00"
> >          "\x01\x00\x06\x00\x06\x09\x24\x03\x05\x05\x03\x06\x05\x81\x09\x24\x03"
> >          "\x06\x01\x01\x04\x05\x05\x07\x24\x04\x05\x01\x00\x9c\xbd\x89"
>
> If I haven't made any mistake in the manual decode process (is there any
> Linux tool that can decode a binary descriptors dump the same way lsusb
> decodes descriptors from a device ?), the relevant UVC descriptors there
> are
I tried looking around and couldn’t find anything. The closest I could
find was defining the structure and then using a parser or reverse
engineering it using wireshark. Do you think there is a need for such
a tool? I would be interested in making one.

>
> 0x0a            bLength
> 0x24            bDescriptorType USB_DT_CS_INTERFACE
> 0x02            bDescriptorSubtype VC_INPUT_TERMINAL
> 0x00            bTerminalID 0 (invalid)
> 0x00, 0x05      bTerminalType 0x0500 (invalid)
> 0x02            bAssocTerminal 2 (invalid)
> 0x01            iTerminal 1
> 0x02, 0x07
>
> 0x09            bLength
> 0x24            bDescriptorType USB_DT_CS_INTERFACE
> 0x03            bDescriptorSubtype VC_OUTPUT_TERMINAL
> 0x06            bTerminalID 6
> 0x01, 0x01      bTerminalType TT_STREAMING
> 0x04            bAssocTerminal 4 (invalid)
> 0x05            bSourceID 5
> 0x05            iTerminal 5
>
> 0x07            bLength
> 0x24            bDescriptorType USB_DT_CS_INTERFACE
> 0x04            bDescriptorSubtype VC_SELECTOR_UNIT
> 0x05            bUnitID 5
> 0x01            bNrInPins 1
> 0x00            baSourceID(1) 0
> 0x9c            iSelector 156
This looks correct. I compared it with the Universal Serial Bus Device
Class Definition for Video Devices: Video Device Examples (revision
1.5 2012). I only got slightly different results (only formatting
related - w prefix instead of b for 2 bytes).

>
> Ignoring a few invalid values (bTerminalID shouldb't be 0, bTerminalType
> 0x0500 is defined by the specification, and the two bAssocTerminal ids
> are also invalid), this creates the following chain:
>
> VC_INPUT_TERMINAL (0) -> VC_SELECTOR_UNIT (5) -> VC_OUTPUT_TERMINAL (6)
>
> Looking at uvc_mc_init_entity() where the media_entity->num_pads field
> gets assigned by calling media_entity_pads_init(), a media entity is
> only initialized when the entity type is not TT_STREAMING (so it's a
> subdev), or when the entity has an associated video device. I think
> that what is happening here is that the second entity in the above list
> (VC_OUTPUT_TERMINAL, id 6) fails to initialize properly in
> uvc_register_terms() is there is no corresponding streaming interface in
> the device. This is confirmed by the
>
> usb 1-1: No streaming interface found for terminal 6.
>
> message in the syzbot kernel log. No video device is created for the
> terminal, and no media_entity is initialized. Trying to later link the
> entity in uvc_mc_create_links() then fails.
>
> I don't want to address this in uvc_mc_create_links() as the invalid
> terminal in the chain means we could have other issues elsewhere. One
> option is to fail turn the missing streaming interface check in a hard
> failure, at least for the chain being registered. The driver could still
> proceed to registering other chains.
>
> There's a small risk of regression for buggy devices. If that's a
> problem, we could instead remove invalid terminals from the device
> entities list before we proceed to scanning chains.
Which solution do you recommend? I can try implementing the one for
the hard failure while letting it proceed to register other chains. It
seems like testing for a regression would require feedback from the
community which might be hard since it would most likely only affect a
few devices.





>
> > > > I confirmed this by changing the terminal ID in the format descriptor
> > > > from 6 to 5, which eliminates both warnings. This shows the warning is
> > > > correctly identifying an invalid descriptor configuration, not a
> > > > kernel bug.
> > >
> > > There's still something not quite right. uvc_entity->num_pads should
> > > always be equal to the corresponding media_entity->num_pads. That's not
> > > the case here, and I think it indicates a bug.
> >
> > Ah ok - the mismatch itself shouldn't happen regardless of the descriptor
> >
> > > > Since the USB descriptor is invalid, I believe the warning is
> > > > necessary and should remain. The code should stay as is.
> > >
> > > There should be a warning, but I think it needs to be caught in a
> > > different place, earlier.
> >
> > Got it.
> >
> > > > On Tue, Mar 25, 2025 at 8:13 PM Laurent Pinchart wrote:
> > > > > On Tue, Mar 25, 2025 at 06:05:00PM -0400, Gabriel wrote:
> > > > > > Hi Ricardo,
> > > > > >
> > > > > > > I cannot reach that URL
> > > > > > I was unable to access the URL from my email client when I initially
> > > > > > sent the email, but a couple of hours later, I was able to. Initially,
> > > > > > copying and pasting the URL into the browser provided a workaround.
> > > > > >
> > > > > > > Shouldn't it be?:
> > > > > > > Fixes: 4ffc2d89f38a ("[media] uvcvideo: Register subdevices for each entity")
> > > > > > You're right, I incorrectly referenced the wrong commit. However, I’m
> > > > > > not certain if it should reference a96aa5342d57 (Fixes: a96aa5342d57 -
> > > > > > '[media] uvcvideo: Ignore entities for terminals with no supported
> > > > > > format') as it's the latest commit affecting the line I'm changing or
> > > > > > the one you mentioned.
> > > > > >
> > > > > > > Shouldn't source->num_pads be the same as remote->num_pads?
> > > > > > The fuzzer (Syzkaller) that triggered the warning appears to have
> > > > > > encountered a case where source->num_pads and remote->num_pads were
> > > > > > different. When analyzing the case in GDB, remote->num_pads was 1,
> > > > > > while source->num_pads was 0.
> > > > >
> > > > > This seems like the real bug that should be fixed.
> > > > >
> > > > > > > Are you sure that your kernel does not contain?
> > > > > > > https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/media/usb/uvc/uvc_entity.c?id=41ddb251c68ac75c101d3a50a68c4629c9055e4c
> > > > > > Yes, it should be included since I am running the upstream kernel.
>
> --
> Regards,
>
> Laurent Pinchart