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