Re: [PATCH] media: uvcvideo: skip bulk output URBs smaller than the payload header

[Ricardo Ribalda]

HI Louis

Thanks for your patch

On Tue, 13 Oct 2026 at 14:56, Louis Clinckx <clinckx.louis@xxxxxxxxx> wrote:
>
> For a bulk video output stream, uvc_video_encode_bulk() starts each
> payload by writing a fixed 2-byte header with uvc_video_encode_header()
> and subtracts it from a signed length counter initialised to the URB
> size:

Out of curiosity, do you have a UVC output device? I would really like
to test one out, but I could not find anything on the market.

>
>         int len = stream->urb_size;
>         ...
>         len -= uvc_video_encode_header(...);    /* always returns 2 */
>
> An output device can describe a bulk endpoint with wMaxPacketSize 1 and
> negotiate dwMaxPayloadTransferSize 1, which leaves stream->urb_size == 1.
> len then becomes -1, and uvc_video_encode_data() casts it to unsigned:
>
>         nbytes = min((unsigned int)len, buf->bytesused - queue->buf_used);
>
> so (unsigned)-1 does not clamp and the following memcpy() copies the
> whole frame past the one-byte URB buffer.
>
> A URB that cannot hold the 2-byte header cannot carry a payload, so skip
> it instead of letting the length underflow.
>
> Signed-off-by: Louis Clinckx <clinckx.louis@xxxxxxxxx>
> ---
>  drivers/media/usb/uvc/uvc_video.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1701,7 +1701,14 @@ static void uvc_video_encode_bulk(struct uvc_urb *uvc_urb,
>         if (buf == NULL) {
>                 urb->transfer_buffer_length = 0;
>                 return;
>         }
>
> +       /* The 2-byte payload header must fit; a smaller URB would make len
> +        * underflow and uvc_video_encode_data() copy past the buffer. */
> +       if (len < 2) {
> +               urb->transfer_buffer_length = 0;
> +               return;
> +       }
> +
>         /* If the URB is the first of its payload, add the header. */
>         if (stream->bulk.header_size == 0) {
>
I'd also detect that the configuration is broken, maybe in
uvc_init_video_bulk() ?

and a bit nitpick. I think this is slightly more idiomatic than your check.


diff --git a/drivers/media/usb/uvc/uvc_video.c
b/drivers/media/usb/uvc/uvc_video.c
index ec76595f3c4b..03940b62b250 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1373,6 +1373,8 @@ static void uvc_video_decode_end(struct
uvc_streaming *stream,
 static int uvc_video_encode_header(struct uvc_streaming *stream,
                struct uvc_buffer *buf, u8 *data, int len)
 {
+       if (len < 2)
+               return -ENOSPC;
        data[0] = 2;    /* Header length */
        data[1] = UVC_STREAM_EOH | UVC_STREAM_EOF
                | (stream->last_fid & UVC_STREAM_FID);
@@ -1656,6 +1658,10 @@ static void uvc_video_encode_bulk(struct
uvc_urb *uvc_urb,
        /* If the URB is the first of its payload, add the header. */
        if (stream->bulk.header_size == 0) {
                ret = uvc_video_encode_header(stream, buf, mem, len);
+               if (ret < 0) {
+                       urb->transfer_buffer_length = 0;
+                       return;
+               }
                stream->bulk.header_size = ret;
                stream->bulk.payload_size += ret;
                mem += ret;

-- 
Ricardo Ribalda