Re: [PATCH] media: msi2500: return buffers as QUEUED in buf_queue() before start_streaming()

[Hans Verkuil]

On 06/11/2026 09:41, zhangdaolong wrote:
> When vb2_start_streaming() is invoked, it first passes all pre-queued
> buffers on q->queued_list to the driver via buf_queue() before setting
> q->start_streaming_called and invoking start_streaming(). If
> start_streaming() subsequently returns an error, videobuf2 requires all
> buffers passed to the driver to be returned with VB2_BUF_STATE_QUEUED
> and warns via WARN_ON(!list_empty(&q->done_list)) if any buffer was
> completed with VB2_BUF_STATE_ERROR or VB2_BUF_STATE_DONE.
> 
> If the USB device disconnects before vb2_start_streaming() runs (for
> example when v4l2_read() races with msi2500_disconnect()), dev->udev is
> already NULL when vb2_start_streaming() enters. msi2500_buf_queue() sees
> !dev->udev and immediately completes each pre-queued buffer with
> VB2_BUF_STATE_ERROR (moving it onto q->done_list) instead of adding it
> to dev->queued_bufs. Right after that, msi2500_start_streaming() also
> sees !dev->udev and returns -ENODEV, triggering
> WARN_ON(!list_empty(&q->done_list)) in vb2_start_streaming().
> 
> Fix this by checking vb2_start_streaming_called(vb->vb2_queue) in
> msi2500_buf_queue() when !dev->udev: return VB2_BUF_STATE_QUEUED when
> start_streaming() has not been called yet, and VB2_BUF_STATE_ERROR when
> streaming is already active.
> 
> Reported-by: syzbot+930e6201080a31e9d0c8@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=930e6201080a31e9d0c8
> Fixes: 977e444f59ad ("[media] Mirics MSi3101 SDR Dongle driver")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: zhangdaolong <dlzhangak@xxxxxxxxxxxxxx>
> ---
>  drivers/media/usb/msi2500/msi2500.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/msi2500/msi2500.c b/drivers/media/usb/msi2500/msi2500.c
> index 282256ab812a..d89db57f924f 100644
> --- a/drivers/media/usb/msi2500/msi2500.c
> +++ b/drivers/media/usb/msi2500/msi2500.c
> @@ -625,7 +625,9 @@ static void msi2500_buf_queue(struct vb2_buffer *vb)
>  
>  	/* Check the device has not disconnected between prep and queuing */
>  	if (unlikely(!dev->udev)) {
> -		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
> +		vb2_buffer_done(&buf->vb.vb2_buf,
> +				vb2_start_streaming_called(vb->vb2_queue) ?
> +				VB2_BUF_STATE_ERROR : VB2_BUF_STATE_QUEUED);
>  		return;
>  	}

I think the real problem is that msi2500_buf_queue is calling vb2_buffer_done
at all.

What happens if you just remove this 'if' completely?

I don't think I have seen vb2_buffer_done being called from buf_queue before.
It's very dubious code.

Regards,

	Hans

>