Re: [PATCH v2 0/3] media: uvcvideo: harden the frame buffer size computation

[Noam Ben]

Hi Natasha,

I tested v2 of the series on a UVC gadget over dummy_hcd.

One uncompressed format at 32 bpp with three frames: 40000x40000
(6400000000 bytes, over U32_MAX), 0x480 (zero size), and 640x480
(which is valid). Both bad frames are skipped and the parsing
continues to the good one:

  uvcvideo 1-1:1.0: Found format YUYV little-endian (0x56595559)
  uvcvideo 1-1:1.1: UVC non compliance: FRAME 1 computed buffer size
overflows (40000x40000, 32 bpp), skipping it.
  uvcvideo 1-1:1.1: UVC non compliance: FRAME 2 has zero size (0x480,
32 bpp), skipping it.
  uvcvideo 1-1:1.0: - 640x480 (30.0 fps)
  uvcvideo 1-1:1.0: Found UVC 1.10 device UVC series test (1d6b:0104)

My original standalone patch, on a comparable descriptor, discarded
the whole streaming interface, so 1/3's return convention does what
it is meant to :)

My build was clean with W=1.

Tested-by: Noam Ben Shimon <noambs2999@xxxxxxxxx>

I did not cover the -ENODATA truncated-descriptor path, nor the
32-bit code generation for DIV_ROUND_UP_ULL - x86_64 only here.

On Thu, 20 Aug 2026 at 13:47, Natasha Klaus
<natalie.klaus@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> uvc_parse_frame() recomputes dwMaxVideoFrameBufferSize for uncompressed
> formats from three descriptor fields that nothing validates. The product
> is evaluated in 32-bit signed arithmetic, so it wraps, and the driver
> stores a size that is usually far too small and sometimes exactly zero.
>
> Noam Ben Shimon reported and fixed the overflow. Reviewing it surfaced a
> second route to a zero size, and Ricardo Ribalda asked for a series
> rather than two independent patches, so the two cases are not handled
> inconsistently.
>
>   1/3  changes the return convention of uvc_parse_frame() so it can
>        report "skip this frame descriptor" separately from a fatal
>        error. Suggested by Ricardo.
>   2/3  Noam's overflow check, adapted to skip rather than reject, with
>        DIV_ROUND_UP_ULL and the operand values in the diagnostic per David
>        Laight's review.
>   3/3  the zero-size case.
>
> On stable: 1/3 carries Cc: stable with no Fixes: tag of its own. It is a
> prerequisite, since 2/3 and 3/3 need -EINVAL to mean "skip". Backporting
> 2/3 without 1/3 is not broken, it reverts to discarding the streaming
> interface, but the commit message would then describe something the
> backport does not do. Both or neither, please.
>
> Carrying 2/3 on Noam's behalf, with his agreement on the list.
>
> Build tested on x86_64 only, no hardware and no UVC gadget.
>
> Changes in v2:
> - 1/3: check -ENODATA before counting the frame (Ricardo Ribalda)
> - 2/3: DIV_ROUND_UP -> DIV_ROUND_UP_ULL (Ricardo Ribalda)
> - Reviewed-by from Ricardo Ribalda added to all three
>
> Natasha Klaus (2):
>   media: uvcvideo: Let uvc_parse_frame() report a skipped frame
>   media: uvcvideo: Skip frame descriptors with a zero computed size
>
> Noam Ben Shimon (1):
>   media: uvcvideo: Fix integer overflow in frame buffer size calculation
>
>  drivers/media/usb/uvc/uvc_driver.c | 52 ++++++++++++++++++++++--------
>  1 file changed, 39 insertions(+), 13 deletions(-)
>
> --
> 2.34.1
>