Linux Media Controller development
 help / color / mirror / Atom feed
From: Natasha Klaus <natalie.klaus@runtimeverification.com>
To: laurent.pinchart@ideasonboard.com, hansg@kernel.org, mchehab@kernel.org
Cc: ribalda@chromium.org, noambs2999@gmail.com,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	Natasha Klaus <natalie.klaus@runtimeverification.com>
Subject: [PATCH] media: uvcvideo: Skip frame descriptors with a zero computed size
Date: Tue, 18 Aug 2026 10:59:56 +0300	[thread overview]
Message-ID: <20260818075956.212624-1-natalie.klaus@runtimeverification.com> (raw)
In-Reply-To: <20260812103251.18309-1-noambs2999@gmail.com>

For uncompressed formats uvc_parse_frame() recomputes
dwMaxVideoFrameBufferSize from the frame dimensions and the bits per
pixel. All three operands come from the frame and format descriptors and
none of them is validated: wWidth and wHeight are read at
uvc_driver.c:254 and uvc_driver.c:255, and bpp at uvc_driver.c:382.

The computed size is therefore zero whenever any operand is zero, and
also whenever the product is below 8 and truncates to zero on the shift,
for instance bpp=1 with a 2x3 frame.

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] at
drivers/media/usb/uvc/uvc_video.c:2298, so a device that also has
usable frames can come up unusable.

Skip the frame descriptor instead of rejecting it. Rejecting the
descriptor would discard the whole streaming interface, including every
valid format on it. Skipping follows the convention introduced by
commit 81f3affa19d6 ("media: uvcvideo: Don't expose unsupported formats
to userspace"), which drops a format descriptor the driver cannot use
rather than failing the parse, for the same reason: to keep an unusable
descriptor from reaching userspace and triggering a WARN_ON. Extend the
existing "return 0 means skip this descriptor" handling from the format
loop to the frame loop so parsing continues with the next frame and the
rest of the format survives.

Frame based compressed formats are not affected. They legitimately
carry a zero dwMaxVideoFrameBufferSize, set unconditionally at
uvc_driver.c:265 because the frame based frame descriptor has no such
field, and they never enter this branch because it is guarded by
!UVC_FMT_FLAG_COMPRESSED.

Signed-off-by: Natasha Klaus <natalie.klaus@runtimeverification.com>
---
Applies on top of Noam Ben Shimon's v2:
https://lore.kernel.org/linux-media/20260812103251.18309-1-noambs2999@gmail.com/
It sits directly after his overflow check and will not apply without it.

One consequence worth naming: if every frame of the default format is
zero-sized, nframes ends up 0 and uvc_video_init() fails probe at
uvc_video.c:2286. This cascade is not new here. 81f3affa19d6 already has
it one level up, where skipping enough formats leaves nformats == 0 and
trips the same guard at uvc_video.c:2226. Such a device has nothing to
stream either way, but the outcome is no node rather than a node that
fails at REQBUFS, so it is a judgement call I would rather leave to you.

This does not cover compressed formats. For UVC 1.10 and later
uvc_fixup_video_ctrl() does not overwrite dwMaxVideoFrameSize, so a zero
in the device's probe response reaches vb2 unchecked and no parse-time
check can see it.

Not tested on hardware or a UVC gadget. Built and verified against the
isolated expression only.

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

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 29e23f94751c..e5858cec7ee4 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -309,6 +309,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 0;
+		}
+
 		frame->dwMaxVideoFrameBufferSize = bufsize;
 	}
 
@@ -506,6 +520,11 @@ static int uvc_parse_format(struct uvc_device *dev,
 					      buffer, buflen);
 			if (ret < 0)
 				return ret;
+			if (!ret) {
+				buflen -= buffer[0];
+				buffer += buffer[0];
+				continue;
+			}
 			format->nframes++;
 			buflen -= ret;
 			buffer += ret;

base-commit: bae860246e920a7d24256858b69133c9c5f1f6a1
-- 
2.34.1


  parent reply	other threads:[~2026-08-18  8:00 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-07 10:14 [PATCH] media: uvcvideo: Fix integer overflow in frame buffer size calculation Noam Ben Shimon
2026-08-10 14:12 ` Ricardo Ribalda
2026-08-12 10:32 ` [PATCH v2] " Noam Ben Shimon
2026-08-18  6:45   ` Natasha Klaus
2026-08-18  6:54     ` Ricardo Ribalda
2026-08-18  6:57       ` Ricardo Ribalda
2026-08-18  7:59   ` Natasha Klaus [this message]
2026-08-18  8:31     ` [PATCH] media: uvcvideo: Skip frame descriptors with a zero computed size Ricardo Ribalda
2026-08-18  9:40       ` Natasha Klaus
2026-08-18  9:53         ` Natasha Klaus
2026-08-18 10:18         ` Ricardo Ribalda
2026-08-18 10:32       ` Natasha Klaus
2026-08-18 10:40         ` Ricardo Ribalda
2026-08-18  8:28 ` [PATCH] media: uvcvideo: Fix integer overflow in frame buffer size calculation David Laight

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260818075956.212624-1-natalie.klaus@runtimeverification.com \
    --to=natalie.klaus@runtimeverification.com \
    --cc=hansg@kernel.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=noambs2999@gmail.com \
    --cc=ribalda@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox