All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] media: uvcvideo: Fix integer overflow in frame buffer size calculation
@ 2026-08-07 10:14 Noam Ben Shimon
  2026-08-10 14:12 ` Ricardo Ribalda
  2026-08-12 10:32 ` [PATCH v2] " Noam Ben Shimon
  0 siblings, 2 replies; 3+ messages in thread
From: Noam Ben Shimon @ 2026-08-07 10:14 UTC (permalink / raw)
  To: laurent.pinchart, hansg, mchehab
  Cc: linux-media, linux-kernel, Noam Ben Shimon

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")
Signed-off-by: Noam Ben Shimon <noambs2999@gmail.com>
---
 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..d319368f9d21 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 / 8;
+		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
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.

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] media: uvcvideo: Fix integer overflow in frame buffer size calculation
  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
  1 sibling, 0 replies; 3+ messages in thread
From: Ricardo Ribalda @ 2026-08-10 14:12 UTC (permalink / raw)
  To: Noam Ben Shimon
  Cc: laurent.pinchart, hansg, mchehab, linux-media, linux-kernel

Hi Noam

On Fri, 7 Aug 2026 at 12:26, Noam Ben Shimon <noambs2999@gmail.com> wrote:
>
> 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>
> ---
>  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..d319368f9d21 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 / 8;
I'd rather avoid doing a u64 division (even knowing that the compiler
is often clever enough to avoid it)
What about:

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
> 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.
Thanks for the report. Most of the times we want to trust the hw...
but for usb cameras is totally worth it to be extra cautious.

With my change (and if you test it :P) you can add my:
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>

>


-- 
Ricardo Ribalda

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH v2] media: uvcvideo: Fix integer overflow in frame buffer size calculation
  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
  1 sibling, 0 replies; 3+ messages in thread
From: Noam Ben Shimon @ 2026-08-12 10:32 UTC (permalink / raw)
  To: laurent.pinchart, hansg, mchehab
  Cc: ribalda, linux-media, linux-kernel, Noam Ben Shimon, stable

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-12 10:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.