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

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

[Natasha Klaus]

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

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

[PATCH 1/3] media: uvcvideo: Let uvc_parse_frame() report a skipped frame

[Natasha Klaus]

uvc_parse_frame() returns the descriptor length on success and a
negative error code on failure, and uvc_parse_format() treats every
negative value as fatal for the whole streaming interface. There is no
way for the parser to say "this frame descriptor is unusable, but the
rest of the format is fine".

Change the return convention so it can. Return 0 on success and let the
caller advance by buffer[0], which is the value the function returned
anyway. Report a truncated descriptor with -ENODATA, which stays fatal,
and leave every other negative value to mean "skip this frame descriptor
and carry on with the next one".

-ENODATA is currently the only error the function can return, so the
skip path is unreachable until later patches add checks that use it. The
one behavioural change is the truncated-descriptor diagnostic, which
moves from uvc_dbg() to dev_warn() so a malformed descriptor is reported
without the DESCR debug flag. That leaves the local alts variable
unused, and the kernel builds -Wunused-variable as an error, so it goes
too.

Suggested-by: Ricardo Ribalda <ribalda@chromium.org>
Link: https://lore.kernel.org/linux-media/CANiDSCue8yyiGubzbAybRqSUTTFuB=-Y2TZy6yvOx32SpAASWg@mail.gmail.com/
Cc: stable@vger.kernel.org
Signed-off-by: Natasha Klaus <natalie.klaus@runtimeverification.com>
---
The Cc: stable line is present without a Fixes: tag because this patch is
a prerequisite for 2/3 rather than a fix in its own right. Stable needs
both or neither: backported alone, 2/3's -EINVAL would revert to meaning
"discard the whole streaming interface".

 drivers/media/usb/uvc/uvc_driver.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index e289cc71ba98..0cc0e351d139 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -230,7 +230,6 @@ static int uvc_parse_frame(struct uvc_device *dev,
 			   u32 **intervals, u8 ftype, int width_multiplier,
 			   const unsigned char *buffer, int buflen)
 {
-	struct usb_host_interface *alts = streaming->intf->cur_altsetting;
 	unsigned int maxIntervalIndex;
 	unsigned int interval;
 	unsigned int i, n;
@@ -243,10 +242,10 @@ static int uvc_parse_frame(struct uvc_device *dev,
 	n = n ? n : 3;
 
 	if (buflen < 26 + 4 * n) {
-		uvc_dbg(dev, DESCR,
-			"device %d videostreaming interface %d FRAME error\n",
-			dev->udev->devnum, alts->desc.bInterfaceNumber);
-		return -EINVAL;
+		dev_warn(&streaming->intf->dev,
+			 "UVC non compliance: FRAME descriptor is %d bytes, expected at least %u.\n",
+			 buflen, 26 + 4 * n);
+		return -ENODATA;
 	}
 
 	frame->bFrameIndex = buffer[3];
@@ -329,7 +328,7 @@ static int uvc_parse_frame(struct uvc_device *dev,
 
 	*intervals += n;
 
-	return buffer[0];
+	return 0;
 }
 
 static int uvc_parse_format(struct uvc_device *dev,
@@ -492,11 +491,12 @@ static int uvc_parse_format(struct uvc_device *dev,
 			ret = uvc_parse_frame(dev, streaming, format, frame,
 					      intervals, ftype, width_multiplier,
 					      buffer, buflen);
-			if (ret < 0)
+			if (!ret)
+				format->nframes++;
+			if (ret == -ENODATA)
 				return ret;
-			format->nframes++;
-			buflen -= ret;
-			buffer += ret;
+			buflen -= buffer[0];
+			buffer += buffer[0];
 		}
 	}
 
-- 
2.34.1

[PATCH 2/3] media: uvcvideo: Fix integer overflow in frame buffer size calculation

[Natasha Klaus]

From: Noam Ben Shimon <noambs2999@gmail.com>

In the function uvc_parse_frame(), it recomputes
dwMaxVideoFrameBufferSize for uncompressed formats. This helps working
around devices that report it wrong:

	frame->dwMaxVideoFrameBufferSize = format->bpp * frame->wWidth
					 * frame->wHeight / 8;

These three arguments originate from the device's own descriptors, and
therefore can be decided by it. bpp is a u8 and wWidth and wHeight are
u16. The expression is evaluated in int, and the maximum value is
255 * 65535 * 65535 (which is roughly 510 times INT_MAX).
A device that declares large dimensions therefore overflows a signed
int here.
The kernel is built using -fno-strict-overflow, so this wraps rather
than being miscompiled, but the wrapped value (which is often negative)
is then divided by 8 and stored in a u32 used as a size.

Two examples for this (using legal field values):

  - 32 bpp, 16384x4096: the product is exactly 2^31 and wraps to
    INT_MIN. After division and conversion to u32 the field has the
    value 4026531840 rather than 268435456.

  - 16 bpp, 16384x16384: the product is exactly 2^32 and wraps to 0.
    The field holds 0, rather than the correct 536870912.

I don't think memory corruption is a consequence of this. The value
reaches uvc_queue_setup() as the vb2 buffer size, and every copy on the
decode path is bounded by buf->length, which uvc_buffer_prepare() gets
from vb2_plane_size() rather than this field. What a wrapped value does
instead is make the driver describe the stream inconsistently.
For an uncompressed format uvc_fixup_video_ctrl() copies it into
ctrl->dwMaxVideoFrameSize unconditionally, and that becomes the
sizeimage reported by VIDIOC_G_FMT. This is while width, height and
bytesperline continue to describe the full frame.
This also makes uvc_video_validate_buffer() mark error on all frames,
because it is comparing bytesused against the same number.

Compute the size in 64-bit, and if the result does not fit in the u32
field then skip the frame descriptor. An uncompressed frame this large
is probably not a real device, and skipping it leaves the rest of the
format and the streaming interface usable.

The rounding also changes from truncation to round-up. Truncation is
pre-existing rather than introduced here: the original expression used
integer division, so it has rounded a partial trailing byte away since
the driver was merged. Rounding up is the right direction for a buffer
size, and DIV_ROUND_UP() against BITS_PER_BYTE is what the rest of the
media tree uses for this computation, including uvc_parse_format()
itself for the FORCE_BPP quirk.

Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver")
Cc: stable@vger.kernel.org
Signed-off-by: Noam Ben Shimon <noambs2999@gmail.com>
Signed-off-by: Natasha Klaus <natalie.klaus@runtimeverification.com>
---
Changes from Noam's v2:
- Rebased onto patch 1/3; this patch no longer applies standalone.
- Diagnostic changed from uvc_dbg(dev, DESCR, ...) to dev_warn() on
  &streaming->intf->dev, reworded to begin "UVC non compliance: " and to
  say the frame is skipped. The explicit device and interface numbers are
  dropped from the message text because dev_warn() already identifies the
  interface. (The original could not be kept as-is in any case: patch 1/3
  removes the local alts variable it referenced.)
- bpp, wWidth and wHeight added to the message text (David Laight), matching
  the wording of the diagnostic in 3/3.
- The >> 3 replaced with DIV_ROUND_UP() against BITS_PER_BYTE (David Laight),
  so a partial trailing byte is no longer dropped. The overflow check is
  applied to the rounded-up value.
- The return value is still -EINVAL; what changed is its meaning, which
  patch 1/3 redefines as "skip this frame descriptor" rather than "fail the
  whole streaming interface".
- Last paragraph of the commit message reworded from "reject the frame
  descriptor ... rejection is consistent with the other checks over
  malformed-descriptors in this function" to describe skipping instead, and
  a paragraph added on the rounding change.
- Ricardo Ribalda's Reviewed-by dropped, as it was given on the unmodified
  v2.
- Submitter's Signed-off-by added.
- Fixes: and Cc: stable lines unchanged.

Rounding up also moves the U32_MAX boundary: two inputs within the field
limits (bpp=79 at 10077x43161 and bpp=237 at 3359x43161) landed exactly on
U32_MAX with the old truncation and are now rejected, since the rounded-up
size is not representable.

Build tested on x86_64 only. gcc-multilib was not available in my
environment, so the 32-bit code generation for the DIV_ROUND_UP() on a u64
was not verified; the divisor is the power-of-two constant BITS_PER_BYTE, so
I expect a shift rather than a libgcc 64-bit division helper, but I have not
confirmed it. No hardware and no UVC gadget were used.

 drivers/media/usb/uvc/uvc_driver.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 0cc0e351d139..eb7177ea291d 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -295,9 +295,21 @@ static int uvc_parse_frame(struct uvc_device *dev,
 	 * information. For uncompressed formats this can be fixed by computing
 	 * the value from the frame size.
 	 */
-	if (!(format->flags & UVC_FMT_FLAG_COMPRESSED))
-		frame->dwMaxVideoFrameBufferSize = format->bpp * frame->wWidth
-						 * frame->wHeight / 8;
+	if (!(format->flags & UVC_FMT_FLAG_COMPRESSED)) {
+		u64 bufsize;
+
+		bufsize = DIV_ROUND_UP((u64)format->bpp * frame->wWidth *
+				       frame->wHeight, BITS_PER_BYTE);
+		if (bufsize > U32_MAX) {
+			dev_warn(&streaming->intf->dev,
+				 "UVC non compliance: FRAME %u computed buffer size overflows (%ux%u, %u bpp), skipping it.\n",
+				 frame->bFrameIndex, frame->wWidth,
+				 frame->wHeight, format->bpp);
+			return -EINVAL;
+		}
+
+		frame->dwMaxVideoFrameBufferSize = bufsize;
+	}
 
 	/*
 	 * Clamp the default frame interval to the boundaries. A zero
-- 
2.34.1

[PATCH 3/3] media: uvcvideo: Skip frame descriptors with a zero computed size

[Natasha Klaus]

For uncompressed formats uvc_parse_frame() recomputes
dwMaxVideoFrameBufferSize from the frame dimensions and the bits per
pixel. All three operands are read straight from the descriptor bytes
with no range check, so the computed size is zero whenever any of them
is zero.

A zero size is not harmless. It is copied into
ctrl->dwMaxVideoFrameSize by uvc_fixup_video_ctrl() and reaches
uvc_queue_setup() as the vb2 plane size, where it trips
WARN_ON(!plane_sizes[i]) in vb2_core_reqbufs() at
drivers/media/common/videobuf2/videobuf2-core.c:951 and fails
VIDIOC_REQBUFS with -EINVAL. On a kernel built with panic_on_warn that
WARN is fatal.

Such a frame can also become the active one without any application
asking for it: when no frame matches the device's default bFrameIndex,
uvc_video_init() falls back to frames[0], so a device that also has
usable frames can come up unusable.

Skip the frame rather than rejecting the descriptor, which would discard
the whole streaming interface and every valid format on it. This follows
commit 81f3affa19d6 ("media: uvcvideo: Don't expose unsupported formats
to userspace"), which drops a format descriptor the driver cannot use
for the same reason: to keep it from reaching userspace and triggering a
WARN_ON.

Signed-off-by: Natasha Klaus <natalie.klaus@runtimeverification.com>
---
Depends on 1/3 for -EINVAL to mean "skip this frame", and on 2/3 for the
bufsize local.

After 2/3 rounds up instead of truncating, the computed size is zero only
when one of bpp, wWidth or wHeight is zero; the "product below 8 truncates
to zero" case no longer exists.

 drivers/media/usb/uvc/uvc_driver.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index eb7177ea291d..3bd8d31e1378 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -308,6 +308,20 @@ static int uvc_parse_frame(struct uvc_device *dev,
 			return -EINVAL;
 		}
 
+		/*
+		 * A zero-sized frame is unusable: it reaches vb2 as a zero
+		 * plane size, and it is reported to userspace as a 0x0 frame
+		 * with a zero sizeimage. Skip the frame descriptor, the
+		 * caller moves on to the next one.
+		 */
+		if (!bufsize) {
+			dev_warn(&streaming->intf->dev,
+				 "UVC non compliance: FRAME %u has zero size (%ux%u, %u bpp), skipping it.\n",
+				 frame->bFrameIndex, frame->wWidth,
+				 frame->wHeight, format->bpp);
+			return -EINVAL;
+		}
+
 		frame->dwMaxVideoFrameBufferSize = bufsize;
 	}
 
-- 
2.34.1

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

[Ricardo Ribalda]

Hi Natasha

On Thu, 20 Aug 2026 at 11:57, Natasha Klaus
<natalie.klaus@runtimeverification.com> wrote:
>
> From: Noam Ben Shimon <noambs2999@gmail.com>
>
> In the function uvc_parse_frame(), it recomputes
> dwMaxVideoFrameBufferSize for uncompressed formats. This helps working
> around devices that report it wrong:
>
>         frame->dwMaxVideoFrameBufferSize = format->bpp * frame->wWidth
>                                          * frame->wHeight / 8;
>
> These three arguments originate from the device's own descriptors, and
> therefore can be decided by it. bpp is a u8 and wWidth and wHeight are
> u16. The expression is evaluated in int, and the maximum value is
> 255 * 65535 * 65535 (which is roughly 510 times INT_MAX).
> A device that declares large dimensions therefore overflows a signed
> int here.
> The kernel is built using -fno-strict-overflow, so this wraps rather
> than being miscompiled, but the wrapped value (which is often negative)
> is then divided by 8 and stored in a u32 used as a size.
>
> Two examples for this (using legal field values):
>
>   - 32 bpp, 16384x4096: the product is exactly 2^31 and wraps to
>     INT_MIN. After division and conversion to u32 the field has the
>     value 4026531840 rather than 268435456.
>
>   - 16 bpp, 16384x16384: the product is exactly 2^32 and wraps to 0.
>     The field holds 0, rather than the correct 536870912.
>
> I don't think memory corruption is a consequence of this. The value
> reaches uvc_queue_setup() as the vb2 buffer size, and every copy on the
> decode path is bounded by buf->length, which uvc_buffer_prepare() gets
> from vb2_plane_size() rather than this field. What a wrapped value does
> instead is make the driver describe the stream inconsistently.
> For an uncompressed format uvc_fixup_video_ctrl() copies it into
> ctrl->dwMaxVideoFrameSize unconditionally, and that becomes the
> sizeimage reported by VIDIOC_G_FMT. This is while width, height and
> bytesperline continue to describe the full frame.
> This also makes uvc_video_validate_buffer() mark error on all frames,
> because it is comparing bytesused against the same number.
>
> Compute the size in 64-bit, and if the result does not fit in the u32
> field then skip the frame descriptor. An uncompressed frame this large
> is probably not a real device, and skipping it leaves the rest of the
> format and the streaming interface usable.
>
> The rounding also changes from truncation to round-up. Truncation is
> pre-existing rather than introduced here: the original expression used
> integer division, so it has rounded a partial trailing byte away since
> the driver was merged. Rounding up is the right direction for a buffer
> size, and DIV_ROUND_UP() against BITS_PER_BYTE is what the rest of the
> media tree uses for this computation, including uvc_parse_format()
> itself for the FORCE_BPP quirk.
>
> Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Noam Ben Shimon <noambs2999@gmail.com>
> Signed-off-by: Natasha Klaus <natalie.klaus@runtimeverification.com>
> ---
> Changes from Noam's v2:
> - Rebased onto patch 1/3; this patch no longer applies standalone.
> - Diagnostic changed from uvc_dbg(dev, DESCR, ...) to dev_warn() on
>   &streaming->intf->dev, reworded to begin "UVC non compliance: " and to
>   say the frame is skipped. The explicit device and interface numbers are
>   dropped from the message text because dev_warn() already identifies the
>   interface. (The original could not be kept as-is in any case: patch 1/3
>   removes the local alts variable it referenced.)
> - bpp, wWidth and wHeight added to the message text (David Laight), matching
>   the wording of the diagnostic in 3/3.
> - The >> 3 replaced with DIV_ROUND_UP() against BITS_PER_BYTE (David Laight),
>   so a partial trailing byte is no longer dropped. The overflow check is
>   applied to the rounded-up value.
> - The return value is still -EINVAL; what changed is its meaning, which
>   patch 1/3 redefines as "skip this frame descriptor" rather than "fail the
>   whole streaming interface".
> - Last paragraph of the commit message reworded from "reject the frame
>   descriptor ... rejection is consistent with the other checks over
>   malformed-descriptors in this function" to describe skipping instead, and
>   a paragraph added on the rounding change.
> - Ricardo Ribalda's Reviewed-by dropped, as it was given on the unmodified
>   v2.
> - Submitter's Signed-off-by added.
> - Fixes: and Cc: stable lines unchanged.
>
> Rounding up also moves the U32_MAX boundary: two inputs within the field
> limits (bpp=79 at 10077x43161 and bpp=237 at 3359x43161) landed exactly on
> U32_MAX with the old truncation and are now rejected, since the rounded-up
> size is not representable.
>
> Build tested on x86_64 only. gcc-multilib was not available in my
> environment, so the 32-bit code generation for the DIV_ROUND_UP() on a u64
> was not verified; the divisor is the power-of-two constant BITS_PER_BYTE, so
> I expect a shift rather than a libgcc 64-bit division helper, but I have not
> confirmed it. No hardware and no UVC gadget were used.
>
>  drivers/media/usb/uvc/uvc_driver.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 0cc0e351d139..eb7177ea291d 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -295,9 +295,21 @@ static int uvc_parse_frame(struct uvc_device *dev,
>          * information. For uncompressed formats this can be fixed by computing
>          * the value from the frame size.
>          */
> -       if (!(format->flags & UVC_FMT_FLAG_COMPRESSED))
> -               frame->dwMaxVideoFrameBufferSize = format->bpp * frame->wWidth
> -                                                * frame->wHeight / 8;
> +       if (!(format->flags & UVC_FMT_FLAG_COMPRESSED)) {
> +               u64 bufsize;
> +
> +               bufsize = DIV_ROUND_UP((u64)format->bpp * frame->wWidth *
> +                                      frame->wHeight, BITS_PER_BYTE);

I believe this should be DIV_ROUND_UP_ULL. Eventhough BITS_PER_BYTE is
a power-of-2, some compilers might have brain damage and complain
about 64 bit division.

> +               if (bufsize > U32_MAX) {
> +                       dev_warn(&streaming->intf->dev,
> +                                "UVC non compliance: FRAME %u computed buffer size overflows (%ux%u, %u bpp), skipping it.\n",
> +                                frame->bFrameIndex, frame->wWidth,
> +                                frame->wHeight, format->bpp);
> +                       return -EINVAL;
> +               }
> +
> +               frame->dwMaxVideoFrameBufferSize = bufsize;
> +       }
>
>         /*
>          * Clamp the default frame interval to the boundaries. A zero
> --
> 2.34.1
>
With that change, feel free to add my:

Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>

-- 
Ricardo Ribalda

Re: [PATCH 1/3] media: uvcvideo: Let uvc_parse_frame() report a skipped frame

[Ricardo Ribalda]

Hi Natasha

On Thu, 20 Aug 2026 at 11:56, Natasha Klaus
<natalie.klaus@runtimeverification.com> wrote:
>
> uvc_parse_frame() returns the descriptor length on success and a
> negative error code on failure, and uvc_parse_format() treats every
> negative value as fatal for the whole streaming interface. There is no
> way for the parser to say "this frame descriptor is unusable, but the
> rest of the format is fine".
>
> Change the return convention so it can. Return 0 on success and let the
> caller advance by buffer[0], which is the value the function returned
> anyway. Report a truncated descriptor with -ENODATA, which stays fatal,
> and leave every other negative value to mean "skip this frame descriptor
> and carry on with the next one".
>
> -ENODATA is currently the only error the function can return, so the
> skip path is unreachable until later patches add checks that use it. The
> one behavioural change is the truncated-descriptor diagnostic, which
> moves from uvc_dbg() to dev_warn() so a malformed descriptor is reported
> without the DESCR debug flag. That leaves the local alts variable
> unused, and the kernel builds -Wunused-variable as an error, so it goes
> too.
>
> Suggested-by: Ricardo Ribalda <ribalda@chromium.org>
> Link: https://lore.kernel.org/linux-media/CANiDSCue8yyiGubzbAybRqSUTTFuB=-Y2TZy6yvOx32SpAASWg@mail.gmail.com/
> Cc: stable@vger.kernel.org
> Signed-off-by: Natasha Klaus <natalie.klaus@runtimeverification.com>
> ---
> The Cc: stable line is present without a Fixes: tag because this patch is
> a prerequisite for 2/3 rather than a fix in its own right. Stable needs
> both or neither: backported alone, 2/3's -EINVAL would revert to meaning
> "discard the whole streaming interface".
>
>  drivers/media/usb/uvc/uvc_driver.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index e289cc71ba98..0cc0e351d139 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -230,7 +230,6 @@ static int uvc_parse_frame(struct uvc_device *dev,
>                            u32 **intervals, u8 ftype, int width_multiplier,
>                            const unsigned char *buffer, int buflen)
>  {
> -       struct usb_host_interface *alts = streaming->intf->cur_altsetting;
>         unsigned int maxIntervalIndex;
>         unsigned int interval;
>         unsigned int i, n;
> @@ -243,10 +242,10 @@ static int uvc_parse_frame(struct uvc_device *dev,
>         n = n ? n : 3;
>
>         if (buflen < 26 + 4 * n) {
> -               uvc_dbg(dev, DESCR,
> -                       "device %d videostreaming interface %d FRAME error\n",
> -                       dev->udev->devnum, alts->desc.bInterfaceNumber);
> -               return -EINVAL;
> +               dev_warn(&streaming->intf->dev,
> +                        "UVC non compliance: FRAME descriptor is %d bytes, expected at least %u.\n",
> +                        buflen, 26 + 4 * n);
> +               return -ENODATA;
>         }
>
>         frame->bFrameIndex = buffer[3];
> @@ -329,7 +328,7 @@ static int uvc_parse_frame(struct uvc_device *dev,
>
>         *intervals += n;
>
> -       return buffer[0];
> +       return 0;
>  }
>
>  static int uvc_parse_format(struct uvc_device *dev,
> @@ -492,11 +491,12 @@ static int uvc_parse_format(struct uvc_device *dev,
>                         ret = uvc_parse_frame(dev, streaming, format, frame,
>                                               intervals, ftype, width_multiplier,
>                                               buffer, buflen);
> -                       if (ret < 0)
> +                       if (!ret)
> +                               format->nframes++;
> +                       if (ret == -ENODATA)
>                                 return ret;

I know this is my proposal, but if you resubmit the series I think it
is more correct to swap the order (sorry about that):
 +                       if (ret == -ENODATA)
 +                                return ret;
 +                       if (!ret)
 +                               format->nframes++;

> -                       format->nframes++;
> -                       buflen -= ret;
> -                       buffer += ret;
> +                       buflen -= buffer[0];
> +                       buffer += buffer[0];
>                 }
>         }
>
> --
> 2.34.1
>
With that minor nitpick, regardless of whether you change it or not.

Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>

-- 
Ricardo Ribalda

Re: [PATCH 3/3] media: uvcvideo: Skip frame descriptors with a zero computed size

[Ricardo Ribalda]

Hi Natasha

On Thu, 20 Aug 2026 at 11:57, Natasha Klaus
<natalie.klaus@runtimeverification.com> wrote:
>
> For uncompressed formats uvc_parse_frame() recomputes
> dwMaxVideoFrameBufferSize from the frame dimensions and the bits per
> pixel. All three operands are read straight from the descriptor bytes
> with no range check, so the computed size is zero whenever any of them
> is zero.
>
> A zero size is not harmless. It is copied into
> ctrl->dwMaxVideoFrameSize by uvc_fixup_video_ctrl() and reaches
> uvc_queue_setup() as the vb2 plane size, where it trips
> WARN_ON(!plane_sizes[i]) in vb2_core_reqbufs() at
> drivers/media/common/videobuf2/videobuf2-core.c:951 and fails
> VIDIOC_REQBUFS with -EINVAL. On a kernel built with panic_on_warn that
> WARN is fatal.
>
> Such a frame can also become the active one without any application
> asking for it: when no frame matches the device's default bFrameIndex,
> uvc_video_init() falls back to frames[0], so a device that also has
> usable frames can come up unusable.
>
> Skip the frame rather than rejecting the descriptor, which would discard
> the whole streaming interface and every valid format on it. This follows
> commit 81f3affa19d6 ("media: uvcvideo: Don't expose unsupported formats
> to userspace"), which drops a format descriptor the driver cannot use
> for the same reason: to keep it from reaching userspace and triggering a
> WARN_ON.
>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> Signed-off-by: Natasha Klaus <natalie.klaus@runtimeverification.com>
> ---
> Depends on 1/3 for -EINVAL to mean "skip this frame", and on 2/3 for the
> bufsize local.
>
> After 2/3 rounds up instead of truncating, the computed size is zero only
> when one of bpp, wWidth or wHeight is zero; the "product below 8 truncates
> to zero" case no longer exists.
>
>  drivers/media/usb/uvc/uvc_driver.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index eb7177ea291d..3bd8d31e1378 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -308,6 +308,20 @@ static int uvc_parse_frame(struct uvc_device *dev,
>                         return -EINVAL;
>                 }
>
> +               /*
> +                * A zero-sized frame is unusable: it reaches vb2 as a zero
> +                * plane size, and it is reported to userspace as a 0x0 frame
> +                * with a zero sizeimage. Skip the frame descriptor, the
> +                * caller moves on to the next one.
> +                */
> +               if (!bufsize) {
> +                       dev_warn(&streaming->intf->dev,
> +                                "UVC non compliance: FRAME %u has zero size (%ux%u, %u bpp), skipping it.\n",
> +                                frame->bFrameIndex, frame->wWidth,
> +                                frame->wHeight, format->bpp);
> +                       return -EINVAL;
> +               }
> +
>                 frame->dwMaxVideoFrameBufferSize = bufsize;
>         }
>
> --
> 2.34.1
>


-- 
Ricardo Ribalda