Re: [PATCH] media: uvcvideo: Fix integer overflow in frame buffer size calculation
From: Laurent Pinchart
Date: Mon Sep 28 2026 - 16:49:51 EST
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