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

[Laurent Pinchart]

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 ?

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