Re: [PATCH] media: uvcvideo: Fix bounds for descriptor parsing

[Ricardo Ribalda]

On Mon, 28 Sept 2026 at 14:24, Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> On Mon, Sep 28, 2026 at 02:10:26PM +0200, Ricardo Ribalda wrote:
> > On Mon, 28 Sept 2026 at 14:02, Laurent Pinchart wrote:
> > > On Fri, Sep 11, 2026 at 01:23:50PM +0000, Ricardo Ribalda wrote:
> > > > uvc_parse_control() passes descriptor by descriptor to
> > > > uvc_parse_standard_control() with the number of bytes remaining in the
> > > > buffer, not the number of bytes of that descriptor.
> > > >
> > > > Because of this, malformed descriptors could leak over the next
> > > > descriptor, leaving malformed data in our structures.
> > >
> > > What issue does this fix in practice ?
> >
> > Look into uvc_parse_standard_control().
> > Let's take the UVC_VC_HEADER
> >
> > Imagine we have a malformed control at the beginning of the descriptor that has:
> >
> > buffer[0] = 3
> > buffer[2] = UVC_VC_HEADER
> > buffer[3] => Beggining of next control
> >
> > With the current code, buffer[3...X] is parsed as UVC_VC_HEADER.
> >
> > What does it fix in practice? It avoids that data outside the controls
> > is parsed as part of the control.
>
> I understand that, but what does it fix in practice ? And have you
> encountered a device that exhibits such a problem ?

I have not encountered such devices, but even if they do not exist
they are easy to emulate and use for escalation.

We add minimal code and solve a family of bugs. Do you think that it
is a better pattern that uvc_parse_vendor_control() and
uvc_parse_vendor_control() have visibility beyond the control that
they are parsing?

>
> > > > Change the code so we pass the actual length of the descriptor to the
> > > > parser.
> > > >
> > > > Note that this makes the existing check more strict and some devices
> > > > that are wrongly parsed today will not be probed now.
> > > >
> > > > Cc: stable@xxxxxxxxxxxxxxx
> > > > Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver")
> > > > Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> > > > ---
> > > >  drivers/media/usb/uvc/uvc_driver.c | 7 +++++--
> > > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > > > index e289cc71ba98..429f1ab19a2a 100644
> > > > --- a/drivers/media/usb/uvc/uvc_driver.c
> > > > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > > > @@ -1248,11 +1248,14 @@ static int uvc_parse_control(struct uvc_device *dev)
> > > >        */
> > > >
> > > >       while (buflen > 2) {
> > > > -             if (uvc_parse_vendor_control(dev, buffer, buflen) ||
> > > > +             if (buflen < buffer[0] || buffer[0] < 3)
> > > > +                     return -EINVAL;
> > > > +
> > > > +             if (uvc_parse_vendor_control(dev, buffer, buffer[0]) ||
> > > >                   buffer[1] != USB_DT_CS_INTERFACE)
> > > >                       goto next_descriptor;
> > > >
> > > > -             ret = uvc_parse_standard_control(dev, buffer, buflen);
> > > > +             ret = uvc_parse_standard_control(dev, buffer, buffer[0]);
> > > >               if (ret < 0)
> > > >                       return ret;
> > > >
> > > >
> > > > ---
> > > > base-commit: 27953c044974baf7e24dee3e9342fe0103dea80c
> > > > change-id: 20260911-uvc-ctrl-bound-9c1940f06fc7
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda