| Message ID | 67e26157.0c0a0220.36adcd.506e@mx.google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: Fix invalid link creation when source entity has 0 pads | expand |
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 >
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
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.
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
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;