Re: [PATCH] media: uvcvideo: Fix integer overflow in frame buffer size calculation

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Thu, Aug 20, 2026 at 11:31:29AM +0100, David Laight wrote:
> On Thu, 20 Aug 2026 12:13:39 +0300 Natasha Klaus wrote:
> > On Tue, 18 Aug 2026, David Laight wrote:
> > > I'd bet there is a requirement that width*bpp is a multiple of 8 (or even 32)?
> > > You definitely don't want the divide rounding down!  
> > 
> > There is no such check. bpp is a raw descriptor byte at uvc_driver.c:405, and
> > wWidth is unvalidated. The rounding is pre-existing, c0efd232929c has the same
> > /8, so it is not a regression, but it should be DIV_ROUND_UP. uvc_driver.c:431
> > already uses it thirty lines below.
> 
> I'd guess that the only values of bpp that have ever been used are 1, 2, 4, 8,
> 16, 24?, 32 and maybe 64 (for 16bit colour).

Looking at the descriptors for 456 devices, I get 6 different bpp values
with the following number of formats for each bpp:

     43 bBitsPerPixel 8
      1 bBitsPerPixel 10
     18 bBitsPerPixel 12
    446 bBitsPerPixel 16
     12 bBitsPerPixel 24
      1 bBitsPerPixel 32

We of course can't rule out other values. For cameras outputting raw
data (very uncommon), 14, 18, 20 or 22 could be seen.

This being said, I expect width * bpp to always be a multiple of 8.
Splitting a byte across two lines wouldn't make sense.

> Anything else won't go through a hardware FIFO.
> Similarly the hardware wants to do a whole number of memory reads for each
> video line - otherwise it all gets hard to get the porches right.
> 
> > > > +		if (bufsize > U32_MAX) {  
> > > 
> > > Should that be >= ?  
> > 
> > No, U32_MAX itself fits.
> 
> Brain fade :-)
> 
> David
> 
> > > I'd include the bpp, width and height values in the trace.  
> > 
> > Agreed, will do.
> > 
> > I am carrying this as part of a three-patch series at Ricardo's request. Will
> > fold in the rounding and the trace values.
> > 
> > Natasha

-- 
Regards,

Laurent Pinchart