* [PATCH v2 1/3] media: uvcvideo: Let uvc_parse_frame() report a skipped frame
2026-08-20 10:47 [PATCH v2 0/3] media: uvcvideo: harden the frame buffer size computation Natasha Klaus
@ 2026-08-20 10:47 ` Natasha Klaus
2026-08-20 10:47 ` [PATCH v2 2/3] media: uvcvideo: Fix integer overflow in frame buffer size calculation Natasha Klaus
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Natasha Klaus @ 2026-08-20 10:47 UTC (permalink / raw)
To: laurent.pinchart, hansg, mchehab
Cc: ribalda, noambs2999, david.laight.linux, linux-media,
linux-kernel, Natasha Klaus, stable
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
Reviewed-by: Ricardo Ribalda <ribalda@chromium.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".
The caller checks -ENODATA before counting the frame, per Ricardo's review.
Behaviour is unchanged either way, since -ENODATA is non-zero, but the fatal
case reads better first.
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..b94fe5366e55 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 == -ENODATA)
return ret;
- format->nframes++;
- buflen -= ret;
- buffer += ret;
+ if (!ret)
+ format->nframes++;
+ buflen -= buffer[0];
+ buffer += buffer[0];
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 2/3] media: uvcvideo: Fix integer overflow in frame buffer size calculation
2026-08-20 10:47 [PATCH v2 0/3] media: uvcvideo: harden the frame buffer size computation Natasha Klaus
2026-08-20 10:47 ` [PATCH v2 1/3] media: uvcvideo: Let uvc_parse_frame() report a skipped frame Natasha Klaus
@ 2026-08-20 10:47 ` Natasha Klaus
2026-08-20 10:47 ` [PATCH v2 3/3] media: uvcvideo: Skip frame descriptors with a zero computed size Natasha Klaus
2026-08-20 11:01 ` [PATCH v2 0/3] media: uvcvideo: harden the frame buffer size computation Noam Ben
3 siblings, 0 replies; 5+ messages in thread
From: Natasha Klaus @ 2026-08-20 10:47 UTC (permalink / raw)
To: laurent.pinchart, hansg, mchehab
Cc: ribalda, noambs2999, david.laight.linux, linux-media,
linux-kernel, stable, 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>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
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.
- DIV_ROUND_UP changed to DIV_ROUND_UP_ULL per Ricardo's review.
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. 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 b94fe5366e55..d7d71418f875 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_ULL((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
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 3/3] media: uvcvideo: Skip frame descriptors with a zero computed size
2026-08-20 10:47 [PATCH v2 0/3] media: uvcvideo: harden the frame buffer size computation Natasha Klaus
2026-08-20 10:47 ` [PATCH v2 1/3] media: uvcvideo: Let uvc_parse_frame() report a skipped frame Natasha Klaus
2026-08-20 10:47 ` [PATCH v2 2/3] media: uvcvideo: Fix integer overflow in frame buffer size calculation Natasha Klaus
@ 2026-08-20 10:47 ` Natasha Klaus
2026-08-20 11:01 ` [PATCH v2 0/3] media: uvcvideo: harden the frame buffer size computation Noam Ben
3 siblings, 0 replies; 5+ messages in thread
From: Natasha Klaus @ 2026-08-20 10:47 UTC (permalink / raw)
To: laurent.pinchart, hansg, mchehab
Cc: ribalda, noambs2999, david.laight.linux, linux-media,
linux-kernel, 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.
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 d7d71418f875..0bb4e17711cf 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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2 0/3] media: uvcvideo: harden the frame buffer size computation
2026-08-20 10:47 [PATCH v2 0/3] media: uvcvideo: harden the frame buffer size computation Natasha Klaus
` (2 preceding siblings ...)
2026-08-20 10:47 ` [PATCH v2 3/3] media: uvcvideo: Skip frame descriptors with a zero computed size Natasha Klaus
@ 2026-08-20 11:01 ` Noam Ben
3 siblings, 0 replies; 5+ messages in thread
From: Noam Ben @ 2026-08-20 11:01 UTC (permalink / raw)
To: Natasha Klaus
Cc: laurent.pinchart, hansg, mchehab, ribalda, david.laight.linux,
linux-media, linux-kernel
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@gmail.com>
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@runtimeverification.com> 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
>
^ permalink raw reply [flat|nested] 5+ messages in thread