Linux Media Controller development
 help / color / mirror / Atom feed
From: Noam Ben Shimon <noambs2999@gmail.com>
To: laurent.pinchart@ideasonboard.com, hansg@kernel.org, mchehab@kernel.org
Cc: ribalda@chromium.org, linux-media@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Noam Ben Shimon <noambs2999@gmail.com>,
	stable@vger.kernel.org
Subject: [PATCH v2] media: uvcvideo: Fix integer overflow in frame buffer size calculation
Date: Wed, 12 Aug 2026 13:32:51 +0300	[thread overview]
Message-ID: <20260812103251.18309-1-noambs2999@gmail.com> (raw)
In-Reply-To: <20260807101433.54886-1-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 reject the frame descriptor. An uncompressed frame this
large is probably not a real device and rejection is consistent with
the other checks over malformed-descriptors in this function.

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>
---
Changes in v2:
- Used a shift instead of division (Ricardo Ribalda) (Thanks!)
- Add Cc: stable@vger.kernel.org (Ricardo Ribalda)

Compile-tested using W=1 and no warnings.

Tested with a UVC gadget over dummy_hcd with WSL. A frame descriptor
declaring 32 bpp at 40000x40000 (computed size = 6400000000, which is
above U32_MAX) is then rejected as expected, and the streaming interface
is not registered:

  uvcvideo 1-1:1.0: Found format YUYV little-endian (0x56595559)
  uvcvideo 1-1:1.0: device 2 videostreaming interface 1 FRAME 1: computed buffer size overflows
  uvcvideo 1-1:1.0: No streaming interface found for terminal 32771.

I was working on a certain device that had a variation of the Linux
kernel. During my work, I had searched for memory mismanagement and
misallocation in media drivers. At some point I stumbled into the
uvc_driver.c and found a flaw that is not a vulnerability, but still a
flaw. I figured that it was worth letting you know rather than shrug it
off.

 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 e289cc71ba98..29e23f94751c 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -296,9 +296,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 = ((u64)format->bpp * frame->wWidth * frame->wHeight) >> 3;
+		if (bufsize > U32_MAX) {
+			uvc_dbg(dev, DESCR,
+				"device %d videostreaming interface %d FRAME %u: computed buffer size overflows\n",
+				dev->udev->devnum,
+				alts->desc.bInterfaceNumber,
+				frame->bFrameIndex);
+			return -EINVAL;
+		}
+
+		frame->dwMaxVideoFrameBufferSize = bufsize;
+	}
 
 	/*
 	 * Clamp the default frame interval to the boundaries. A zero

base-commit: f9a2394a23482bfd330911e9c8295b71724feacd
-- 
2.34.1


  parent reply	other threads:[~2026-08-12 10:33 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 ` Noam Ben Shimon [this message]
2026-08-18  6:45   ` [PATCH v2] " Natasha Klaus
2026-08-18  6:54     ` Ricardo Ribalda
2026-08-18  6:57       ` Ricardo Ribalda
2026-08-18  7:59   ` [PATCH] media: uvcvideo: Skip frame descriptors with a zero computed size Natasha Klaus
2026-08-18  8:31     ` 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=20260812103251.18309-1-noambs2999@gmail.com \
    --to=noambs2999@gmail.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=ribalda@chromium.org \
    --cc=stable@vger.kernel.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