drivers/media/usb/uvc/uvc_driver.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
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.1Hi Natasha, hi Noam
On Tue, 18 Aug 2026 at 10:00, Natasha Klaus
<natalie.klaus@runtimeverification.com> wrote:
>
> 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.
I think we need to have some consistency. We cannot have one
condition returning -EINVAL and the other skipping it.
How does this plan sound to you:
1) Refactor a bit uvc_parse_frame (warning! not tested)
diff --git a/drivers/media/usb/uvc/uvc_driver.c
b/drivers/media/usb/uvc/uvc_driver.c
index e289cc71ba98..6cbeaf10d2e0 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -243,10 +243,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: device %d videostreaming
interface %d FRAME error\n",
+ dev->udev->devnum, alts->desc.bInterfaceNumber);
+ return -ENODATA;
}
frame->bFrameIndex = buffer[3];
@@ -312,6 +312,8 @@ static int uvc_parse_frame(struct uvc_device *dev,
frame->dwFrameInterval[0],
frame->dwFrameInterval[maxIntervalIndex]);
+ // Your overflow and zero checks go here and return -EINVAL
+
/*
* Some devices report frame intervals that are not functional. If the
* corresponding quirk is set, restrict operation to the first interval
@@ -329,7 +331,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 +494,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)
+ format->nframes++;
+ if (ret == -ENODATA)
return ret;
- format->nframes++;
- buflen -= ret;
- buffer += ret;
+ buflen -= buffer[0];
+ buffer += buffer[0];
}
}
2) apply a modified version of Noam patch with the fixed error message
3) Apply Natasha's patch
If Noam is okay with this, perhaps Natasha could prepare a patchset
with the 3 patches? (keeping Noams author on his patch)
WDYY?
Regards
>
> 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
>
--
Ricardo Ribalda
Understood on the placement, thanks.
One finding while building your refactor: it does not compile as written.
Dropping the uvc_dbg() removes the last use of alts in uvc_parse_frame(), and
the kernel treats that as an error:
drivers/media/usb/uvc/uvc_driver.c:233:36: error: unused variable 'alts'
[-Werror=unused-variable]
I removed the now-dead declaration, which adds a hunk you did not specify. Say
if you would rather keep alts and identify the device explicitly in the
dev_warn() instead.
I also wrote the dev_warn() text, since you specified the level but not the
wording:
dev_warn(&streaming->intf->dev,
"UVC non compliance: FRAME descriptor is %d bytes, expected at least %u.\n",
buflen, 26 + 4 * n);
Happy to change it.
To carry your refactor as 1/3 with you as author I need your Signed-off-by.
Could you send it, or tell me if you would rather I take authorship with a
Suggested-by: line pointing at your message.
Natasha
Hi Natasha On Tue, 18 Aug 2026 at 12:32, Natasha Klaus <natalie.klaus@runtimeverification.com> wrote: > > Understood on the placement, thanks. > > One finding while building your refactor: it does not compile as written. > Dropping the uvc_dbg() removes the last use of alts in uvc_parse_frame(), and > the kernel treats that as an error: > > drivers/media/usb/uvc/uvc_driver.c:233:36: error: unused variable 'alts' > [-Werror=unused-variable] > > I removed the now-dead declaration, which adds a hunk you did not specify. Say > if you would rather keep alts and identify the device explicitly in the > dev_warn() instead. > > I also wrote the dev_warn() text, since you specified the level but not the > wording: > > dev_warn(&streaming->intf->dev, > "UVC non compliance: FRAME descriptor is %d bytes, expected at least %u.\n", > buflen, 26 + 4 * n); I believe that I wrote :) : + dev_warn(&streaming->intf->dev, + "UVC non compliance: device %d videostreaming interface %d FRAME error\n", + dev->udev->devnum, alts->desc.bInterfaceNumber); But anyway... I think dev_warn with intf->dev is more than enough. I prefer your message. Thanks for that > > Happy to change it. > > To carry your refactor as 1/3 with you as author I need your Signed-off-by. > Could you send it, or tell me if you would rather I take authorship with a > Suggested-by: line pointing at your message. Suggested-by is more than enough. Thanks! > > Natasha -- Ricardo Ribalda
On Tue, Aug 18, 2026, Ricardo Ribalda wrote: > I think we need to have some consistency. We cannot have one > condition returning -EINVAL and the other skipping it. Agreed, and your shape is cleaner than mine. > If Noam is okay with this, perhaps Natasha could prepare a patchset > with the 3 patches? (keeping Noams author on his patch) Happy to. Noam, are you okay with me carrying your patch in a series? One question on placement before I write it. Your comment puts the checks after the frame interval parsing, which is outside the !UVC_FMT_FLAG_COMPRESSED branch. Frame based formats legitimately carry a zero dwMaxVideoFrameBufferSize, set unconditionally at uvc_driver.c:265 because the frame based frame descriptor has no such field, so a zero check there would skip every frame of a conformant frame based device. Did you mean the checks stay inside the branch with only the return value changing to -EINVAL, or outside with an explicit frame based exemption? I will test the refactor before sending. The Media CI failure on my patch is the missing dependency on Noam's v2. The series fixes that. Natasha
Yes, thank you. Sorry for the slow reply, I was away for a few days. I'm happy with the changes being folded in. Let me know if you need anything from me for it :) I'll be away again for about two weeks from Sunday, but I'll try to stay reachable where I can. Thanks! On Tue, 18 Aug 2026 at 12:40, Natasha Klaus <natalie.klaus@runtimeverification.com> wrote: > > On Tue, Aug 18, 2026, Ricardo Ribalda wrote: > > I think we need to have some consistency. We cannot have one > > condition returning -EINVAL and the other skipping it. > > Agreed, and your shape is cleaner than mine. > > > If Noam is okay with this, perhaps Natasha could prepare a patchset > > with the 3 patches? (keeping Noams author on his patch) > > Happy to. Noam, are you okay with me carrying your patch in a series? > > One question on placement before I write it. Your comment puts the checks after > the frame interval parsing, which is outside the !UVC_FMT_FLAG_COMPRESSED > branch. Frame based formats legitimately carry a zero > dwMaxVideoFrameBufferSize, set unconditionally at uvc_driver.c:265 because the > frame based frame descriptor has no such field, so a zero check there would skip > every frame of a conformant frame based device. Did you mean the checks stay > inside the branch with only the return value changing to -EINVAL, or outside > with an explicit frame based exemption? > > I will test the refactor before sending. > > The Media CI failure on my patch is the missing dependency on Noam's v2. The > series fixes that. > > Natasha
Hi Natasha On Tue, 18 Aug 2026 at 11:40, Natasha Klaus <natalie.klaus@runtimeverification.com> wrote: > > On Tue, Aug 18, 2026, Ricardo Ribalda wrote: > > I think we need to have some consistency. We cannot have one > > condition returning -EINVAL and the other skipping it. > > Agreed, and your shape is cleaner than mine. > > > If Noam is okay with this, perhaps Natasha could prepare a patchset > > with the 3 patches? (keeping Noams author on his patch) > > Happy to. Noam, are you okay with me carrying your patch in a series? > > One question on placement before I write it. Your comment puts the checks after > the frame interval parsing, which is outside the !UVC_FMT_FLAG_COMPRESSED > branch. Frame based formats legitimately carry a zero > dwMaxVideoFrameBufferSize, set unconditionally at uvc_driver.c:265 because the > frame based frame descriptor has no such field, so a zero check there would skip > every frame of a conformant frame based device. Did you mean the checks stay > inside the branch with only the return value changing to -EINVAL, or outside > with an explicit frame based exemption? I meant keep the checks on the position of your patch and Noam patch. Sorry for the misunderstanding. > > I will test the refactor before sending. > > The Media CI failure on my patch is the missing dependency on Noam's v2. The > series fixes that. > > Natasha -- Ricardo Ribalda
Three more things on the refactor, from reading it rather than running it. Placement: bufsize is declared at uvc_driver.c:300 and the block closes at 327, while interval parsing runs 329-354, so at the proposed position the variable is out of scope. And the exemption a check outside the branch would need is format->flags & UVC_FMT_FLAG_COMPRESSED, which is the branch condition at line 299 itself. The compressed case is worse than frame based alone. For UVC 1.10 and later uvc_fixup_video_ctrl() at uvc_video.c:214-218 never consumes the descriptor value, the size comes from the probe response. An MJPEG descriptor reporting zero is inert on those devices today, so a check outside the branch would skip those frames and break cameras that stream fine. -ENODATA already appears in this driver with the opposite polarity, at uvc_video.c:1284 and :1310, where it means drop this payload and carry on. Using it for the fatal case reads backwards against that. The mechanical parts of your refactor hold: buffer[0] cannot be zero because the USB core truncates the config at the first bLength < 2 descriptor (config.c:706, :785), the interval array is pre-counted at uvc_driver.c:718-729 with *intervals += n after every skip return, and all frames[] access is positional. Not tested on hardware. I will build and test before sending the series. Natasha
© 2016 - 2026 Red Hat, Inc.