[PATCH] media: uvcvideo: defer cancelled buffer completion until copies finish

[PATCH] media: uvcvideo: defer cancelled buffer completion until copies finish

[huhb1]

From: huhb04 <huhb04@gmail.com>

uvc_queue_cancel() returns queued buffers to videobuf2 directly via
vb2_buffer_done(). This is unsafe because asynchronous memcpy workers may
still hold a reference on the same uvc_buffer. Once vb2_buffer_done() has
run, userspace can dequeue and requeue the buffer while the old worker is
still running, leading to the same list_head being inserted into irqqueue
twice and corrupting the list.

The crash manifests in two ways depending on the drop-corrupted-frames
module parameter:

1. With the drop corrupted frames enabled, the old worker's
   final kref_put reaches uvc_queue_buffer_complete(), sees buf->error set,
   and calls uvc_queue_buffer_requeue(). This performs list_add_tail() on
   buf->queue even though userspace has already QBUF'd the same buffer and
   added it to irqqueue.

2. With nodrop=1, uvc_queue_buffer_complete() calls vb2_buffer_done() a
   second time. If userspace has already dequeued and requeued the buffer,
   the second completion exposes it to userspace again, and the next
   DQBUF/QBUF inserts the still-linked list_head into irqqueue a second
   time.

Both cases produce the kind of list_del corruption seen here:

    list_del corruption. next->prev should be ..., but was ...
    kernel BUG at lib/list_debug.c:64!

Fix the cancel path to remove buffers from irqqueue and drop the queue's
reference through uvc_queue_buffer_release() (i.e. kref_put()). The final
VB2 completion then only happens from uvc_queue_buffer_complete() once the
last async reference is released, so userspace cannot reuse the buffer
while an old worker still accesses it.

A buffer whose state is already UVC_BUF_STATE_ERROR is forced to complete
as an error and must not be requeued by the corrupted-frame policy. Normal
malformed frames are still requeued/dropped as before.

Fixes: 01e90464e42e ("media: uvcvideo: queue: Support asynchronous buffer handling")
Cc: stable@vger.kernel.org
Signed-off-by: huhb04 <huhb04@gmail.com>
---
 drivers/media/usb/uvc/uvc_queue.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
index 3c002c8f44..9206e0a355 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -289,10 +289,19 @@ int uvc_queue_init(struct uvc_streaming *stream, struct uvc_video_queue *queue,
  */
 void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect)
 {
+	struct uvc_buffer *buf, *next;
+	struct list_head local_list;
 	unsigned long flags;
 
+	INIT_LIST_HEAD(&local_list);
+
 	spin_lock_irqsave(&queue->irqlock, flags);
-	__uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
+	while (!list_empty(&queue->irqqueue)) {
+		buf = list_first_entry(&queue->irqqueue, struct uvc_buffer, queue);
+		list_del(&buf->queue);
+		buf->state = UVC_BUF_STATE_ERROR;
+		list_add_tail(&buf->queue, &local_list);
+	}
 	/*
 	 * This must be protected by the irqlock spinlock to avoid race
 	 * conditions between uvc_buffer_queue and the disconnection event that
@@ -303,6 +312,17 @@ void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect)
 	if (disconnect)
 		queue->flags |= UVC_QUEUE_DISCONNECTED;
 	spin_unlock_irqrestore(&queue->irqlock, flags);
+
+	/*
+	 * Release the queue-owned kref outside the irqlock. The final VB2
+	 * completion only happens from uvc_queue_buffer_complete() once all
+	 * asynchronous copy references have been released, preventing userspace
+	 * from reusing the buffer while an old worker still accesses it.
+	 */
+	list_for_each_entry_safe(buf, next, &local_list, queue) {
+		list_del(&buf->queue);
+		uvc_queue_buffer_release(buf);
+	}
 }
 
 /*
@@ -356,7 +376,13 @@ static void uvc_queue_buffer_complete(struct kref *ref)
 	struct vb2_buffer *vb = &buf->buf.vb2_buf;
 	struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);
 
-	if (buf->error && !uvc_no_drop_param) {
+	/*
+	 * Buffers cancelled from uvc_queue_cancel() are forced to complete as
+	 * errors. They must not be requeued by the corrupted-frame policy even
+	 * when buf->error is set.
+	 */
+	if (buf->state != UVC_BUF_STATE_ERROR &&
+	    buf->error && !uvc_no_drop_param) {
 		uvc_queue_buffer_requeue(queue, buf);
 		return;
 	}
-- 
2.34.1

Re: [PATCH] media: uvcvideo: defer cancelled buffer completion until copies finish

[Laurent Pinchart]

On Thu, Sep 03, 2026 at 09:11:41PM +0800, huhb1 wrote:
> From: huhb04 <huhb04@gmail.com>
> 
> uvc_queue_cancel() returns queued buffers to videobuf2 directly via
> vb2_buffer_done(). This is unsafe because asynchronous memcpy workers may
> still hold a reference on the same uvc_buffer. Once vb2_buffer_done() has
> run, userspace can dequeue and requeue the buffer while the old worker is
> still running, leading to the same list_head being inserted into irqqueue
> twice and corrupting the list.
> 
> The crash manifests in two ways depending on the drop-corrupted-frames
> module parameter:
> 
> 1. With the drop corrupted frames enabled, the old worker's
>    final kref_put reaches uvc_queue_buffer_complete(), sees buf->error set,
>    and calls uvc_queue_buffer_requeue(). This performs list_add_tail() on
>    buf->queue even though userspace has already QBUF'd the same buffer and
>    added it to irqqueue.
> 
> 2. With nodrop=1, uvc_queue_buffer_complete() calls vb2_buffer_done() a
>    second time. If userspace has already dequeued and requeued the buffer,
>    the second completion exposes it to userspace again, and the next
>    DQBUF/QBUF inserts the still-linked list_head into irqqueue a second
>    time.
> 
> Both cases produce the kind of list_del corruption seen here:
> 
>     list_del corruption. next->prev should be ..., but was ...
>     kernel BUG at lib/list_debug.c:64!
> 
> Fix the cancel path to remove buffers from irqqueue and drop the queue's
> reference through uvc_queue_buffer_release() (i.e. kref_put()). The final
> VB2 completion then only happens from uvc_queue_buffer_complete() once the
> last async reference is released, so userspace cannot reuse the buffer
> while an old worker still accesses it.
> 
> A buffer whose state is already UVC_BUF_STATE_ERROR is forced to complete
> as an error and must not be requeued by the corrupted-frame policy. Normal
> malformed frames are still requeued/dropped as before.
> 
> Fixes: 01e90464e42e ("media: uvcvideo: queue: Support asynchronous buffer handling")
> Cc: stable@vger.kernel.org
> Signed-off-by: huhb04 <huhb04@gmail.com>

We need a real name here and in the author field.

> ---
>  drivers/media/usb/uvc/uvc_queue.c | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
> index 3c002c8f44..9206e0a355 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -289,10 +289,19 @@ int uvc_queue_init(struct uvc_streaming *stream, struct uvc_video_queue *queue,
>   */
>  void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect)
>  {
> +	struct uvc_buffer *buf, *next;
> +	struct list_head local_list;
>  	unsigned long flags;
>  
> +	INIT_LIST_HEAD(&local_list);
> +
>  	spin_lock_irqsave(&queue->irqlock, flags);
> -	__uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
> +	while (!list_empty(&queue->irqqueue)) {
> +		buf = list_first_entry(&queue->irqqueue, struct uvc_buffer, queue);
> +		list_del(&buf->queue);
> +		buf->state = UVC_BUF_STATE_ERROR;
> +		list_add_tail(&buf->queue, &local_list);
> +	}
>  	/*
>  	 * This must be protected by the irqlock spinlock to avoid race
>  	 * conditions between uvc_buffer_queue and the disconnection event that
> @@ -303,6 +312,17 @@ void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect)
>  	if (disconnect)
>  		queue->flags |= UVC_QUEUE_DISCONNECTED;
>  	spin_unlock_irqrestore(&queue->irqlock, flags);
> +
> +	/*
> +	 * Release the queue-owned kref outside the irqlock. The final VB2
> +	 * completion only happens from uvc_queue_buffer_complete() once all
> +	 * asynchronous copy references have been released, preventing userspace
> +	 * from reusing the buffer while an old worker still accesses it.
> +	 */
> +	list_for_each_entry_safe(buf, next, &local_list, queue) {
> +		list_del(&buf->queue);
> +		uvc_queue_buffer_release(buf);
> +	}
>  }
>  
>  /*
> @@ -356,7 +376,13 @@ static void uvc_queue_buffer_complete(struct kref *ref)
>  	struct vb2_buffer *vb = &buf->buf.vb2_buf;
>  	struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);
>  
> -	if (buf->error && !uvc_no_drop_param) {
> +	/*
> +	 * Buffers cancelled from uvc_queue_cancel() are forced to complete as
> +	 * errors. They must not be requeued by the corrupted-frame policy even
> +	 * when buf->error is set.
> +	 */
> +	if (buf->state != UVC_BUF_STATE_ERROR &&
> +	    buf->error && !uvc_no_drop_param) {
>  		uvc_queue_buffer_requeue(queue, buf);
>  		return;
>  	}

-- 
Regards,

Laurent Pinchart

[PATCH v2] media: uvcvideo: defer cancelled buffer completion until copies finish

[Hongbing Hu]

From: Hongbing Hu <huhb04@gmail.com>

uvc_queue_cancel() returns queued buffers to videobuf2 directly via
vb2_buffer_done(). This is unsafe because asynchronous memcpy workers may
still hold a reference on the same uvc_buffer. Once vb2_buffer_done() has
run, userspace can dequeue and requeue the buffer while the old worker is
still running, leading to the same list_head being inserted into irqqueue
twice and corrupting the list.

The crash manifests in two ways depending on the drop-corrupted-frames
module parameter:

1. With the drop corrupted frames enabled, the old worker's
   final kref_put reaches uvc_queue_buffer_complete(), sees buf->error set,
   and calls uvc_queue_buffer_requeue(). This performs list_add_tail() on
   buf->queue even though userspace has already QBUF'd the same buffer and
   added it to irqqueue.

2. With nodrop=1, uvc_queue_buffer_complete() calls vb2_buffer_done() a
   second time. If userspace has already dequeued and requeued the buffer,
   the second completion exposes it to userspace again, and the next
   DQBUF/QBUF inserts the still-linked list_head into irqqueue a second
   time.

Both cases produce the kind of list_del corruption seen here:

    list_del corruption. next->prev should be ..., but was ...
    kernel BUG at lib/list_debug.c:64!

Fix the cancel path to remove buffers from irqqueue and drop the queue's
reference through uvc_queue_buffer_release() (i.e. kref_put()). The final
VB2 completion then only happens from uvc_queue_buffer_complete() once the
last async reference is released, so userspace cannot reuse the buffer
while an old worker still accesses it.

A buffer whose state is already UVC_BUF_STATE_ERROR is forced to complete
as an error and must not be requeued by the corrupted-frame policy. Normal
malformed frames are still requeued/dropped as before.

Fixes: 01e90464e42e ("media: uvcvideo: queue: Support asynchronous buffer handling")
Cc: stable@vger.kernel.org
Signed-off-by: Hongbing Hu <huhb04@gmail.com>
---
v2: corrected author real name in From/SOB per maintainer feedback

 drivers/media/usb/uvc/uvc_queue.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
index 3c002c8f44..9206e0a355 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -289,10 +289,19 @@ int uvc_queue_init(struct uvc_streaming *stream, struct uvc_video_queue *queue,
  */
 void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect)
 {
+	struct uvc_buffer *buf, *next;
+	struct list_head local_list;
 	unsigned long flags;
 
+	INIT_LIST_HEAD(&local_list);
+
 	spin_lock_irqsave(&queue->irqlock, flags);
-	__uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
+	while (!list_empty(&queue->irqqueue)) {
+		buf = list_first_entry(&queue->irqqueue, struct uvc_buffer, queue);
+		list_del(&buf->queue);
+		buf->state = UVC_BUF_STATE_ERROR;
+		list_add_tail(&buf->queue, &local_list);
+	}
 	/*
 	 * This must be protected by the irqlock spinlock to avoid race
 	 * conditions between uvc_buffer_queue and the disconnection event that
@@ -303,6 +312,17 @@ void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect)
 	if (disconnect)
 		queue->flags |= UVC_QUEUE_DISCONNECTED;
 	spin_unlock_irqrestore(&queue->irqlock, flags);
+
+	/*
+	 * Release the queue-owned kref outside the irqlock. The final VB2
+	 * completion only happens from uvc_queue_buffer_complete() once all
+	 * asynchronous copy references have been released, preventing userspace
+	 * from reusing the buffer while an old worker still accesses it.
+	 */
+	list_for_each_entry_safe(buf, next, &local_list, queue) {
+		list_del(&buf->queue);
+		uvc_queue_buffer_release(buf);
+	}
 }
 
 /*
@@ -356,7 +376,13 @@ static void uvc_queue_buffer_complete(struct kref *ref)
 	struct vb2_buffer *vb = &buf->buf.vb2_buf;
 	struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);
 
-	if (buf->error && !uvc_no_drop_param) {
+	/*
+	 * Buffers cancelled from uvc_queue_cancel() are forced to complete as
+	 * errors. They must not be requeued by the corrupted-frame policy even
+	 * when buf->error is set.
+	 */
+	if (buf->state != UVC_BUF_STATE_ERROR &&
+	    buf->error && !uvc_no_drop_param) {
 		uvc_queue_buffer_requeue(queue, buf);
 		return;
 	}
-- 
2.34.1

Re: [PATCH v2] media: uvcvideo: defer cancelled buffer completion until copies finish

[Ricardo Ribalda]

Hi Hongbing

Let me rewrite the commit message for me, because it took me a while
to understand what was going on. (totally on me). Please correct me if
I miss-understood the issue.

The UVC driver processes frames in urbs. A frame usually is divided in
multiple urbs. The urb handler parses the header and the metadata, but
leaves the expensive memcpy to a workqueue.

At any given moment, during streamon we will have:
- the current frame with (0-N) async memcpys waiting to happen. The
current frame is in queue->irqqueue. It has a refcnt of 1 + N memcpy
- Previous frames with (N) async memcpys waiting to happen. They are
not in queue->irqqueue. They have a refcnt of N memcpy

With the current behaviour, when there is urb with an error we flush
the queue->irqque() irrespetively if they have any pending memcpys.

There can be situations where the memcpy occurs after we have flushed
the frames and they are returned to the user. Instead, we should
exploit the refcnt mechanism to return the frames only after the
memcpys have been completed.

This patch does so

On Thu, 3 Sept 2026 at 16:33, Hongbing Hu <huhb04@gmail.com> wrote:
>
> From: Hongbing Hu <huhb04@gmail.com>
>
> uvc_queue_cancel() returns queued buffers to videobuf2 directly via
> vb2_buffer_done(). This is unsafe because asynchronous memcpy workers may
> still hold a reference on the same uvc_buffer. Once vb2_buffer_done() has
> run, userspace can dequeue and requeue the buffer while the old worker is
> still running, leading to the same list_head being inserted into irqqueue
> twice and corrupting the list.
>
> The crash manifests in two ways depending on the drop-corrupted-frames
> module parameter:
>
> 1. With the drop corrupted frames enabled, the old worker's
>    final kref_put reaches uvc_queue_buffer_complete(), sees buf->error set,
>    and calls uvc_queue_buffer_requeue(). This performs list_add_tail() on
>    buf->queue even though userspace has already QBUF'd the same buffer and
>    added it to irqqueue.
>
> 2. With nodrop=1, uvc_queue_buffer_complete() calls vb2_buffer_done() a
>    second time. If userspace has already dequeued and requeued the buffer,
>    the second completion exposes it to userspace again, and the next
>    DQBUF/QBUF inserts the still-linked list_head into irqqueue a second
>    time.
>
> Both cases produce the kind of list_del corruption seen here:
>
>     list_del corruption. next->prev should be ..., but was ...
>     kernel BUG at lib/list_debug.c:64!
>
> Fix the cancel path to remove buffers from irqqueue and drop the queue's
> reference through uvc_queue_buffer_release() (i.e. kref_put()). The final
> VB2 completion then only happens from uvc_queue_buffer_complete() once the
> last async reference is released, so userspace cannot reuse the buffer
> while an old worker still accesses it.
>
> A buffer whose state is already UVC_BUF_STATE_ERROR is forced to complete
> as an error and must not be requeued by the corrupted-frame policy. Normal
> malformed frames are still requeued/dropped as before.
>
> Fixes: 01e90464e42e ("media: uvcvideo: queue: Support asynchronous buffer handling")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongbing Hu <huhb04@gmail.com>
> ---
> v2: corrected author real name in From/SOB per maintainer feedback
>
>  drivers/media/usb/uvc/uvc_queue.c | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
> index 3c002c8f44..9206e0a355 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -289,10 +289,19 @@ int uvc_queue_init(struct uvc_streaming *stream, struct uvc_video_queue *queue,
>   */
>  void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect)
>  {
> +       struct uvc_buffer *buf, *next;
> +       struct list_head local_list;
>         unsigned long flags;
>
> +       INIT_LIST_HEAD(&local_list);
> +
>         spin_lock_irqsave(&queue->irqlock, flags);
> -       __uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
> +       while (!list_empty(&queue->irqqueue)) {
> +               buf = list_first_entry(&queue->irqqueue, struct uvc_buffer, queue);
> +               list_del(&buf->queue);
> +               buf->state = UVC_BUF_STATE_ERROR;
> +               list_add_tail(&buf->queue, &local_list);
> +       }
>         /*
>          * This must be protected by the irqlock spinlock to avoid race
>          * conditions between uvc_buffer_queue and the disconnection event that
> @@ -303,6 +312,17 @@ void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect)
>         if (disconnect)
>                 queue->flags |= UVC_QUEUE_DISCONNECTED;
>         spin_unlock_irqrestore(&queue->irqlock, flags);


I *think* that you do not need to call uvc_queue_buffer_release
outisde spin_unlock_irqrestore(). You could do it
inside the previous while() and simplify the code.

The only way to deadlock is  if uvc_queue_buffer_requeue() is called
and you are actively blocking that in your next snippet.

I would recommend to add a comment in uvc_queue_buffer_complete if you
go that way.
> +
> +       /*
> +        * Release the queue-owned kref outside the irqlock. The final VB2
> +        * completion only happens from uvc_queue_buffer_complete() once all
> +        * asynchronous copy references have been released, preventing userspace
> +        * from reusing the buffer while an old worker still accesses it.
> +        */
> +       list_for_each_entry_safe(buf, next, &local_list, queue) {
> +               list_del(&buf->queue);
> +               uvc_queue_buffer_release(buf);
> +       }
>  }
>
>  /*
> @@ -356,7 +376,13 @@ static void uvc_queue_buffer_complete(struct kref *ref)
>         struct vb2_buffer *vb = &buf->buf.vb2_buf;
>         struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);

If we called uvc_queue_cancel() buf->error is not 1, but the frame is
not complete.

You cannot return VB2_BUF_STATE_DONE in that situation. I think you
should set buf->error =1 inside uvc_queue_cancel

>
> -       if (buf->error && !uvc_no_drop_param) {
> +       /*
> +        * Buffers cancelled from uvc_queue_cancel() are forced to complete as
> +        * errors. They must not be requeued by the corrupted-frame policy even
> +        * when buf->error is set.
> +        */
> +       if (buf->state != UVC_BUF_STATE_ERROR &&
> +           buf->error && !uvc_no_drop_param) {
>                 uvc_queue_buffer_requeue(queue, buf);
>                 return;
>         }
> --
> 2.34.1
>
>


-- 
Ricardo Ribalda

Re: [PATCH v2] media: uvcvideo: defer cancelled buffer completion until copies finish

[Ricardo Ribalda]

Hi

On Mon, 7 Sept 2026 at 05:23, 胡洪兵 <huhb04@gmail.com> wrote:
>
> Hi Ricardo,
> Sorry for the late reply and thanks for your review.
> The issue is exactly as you described.
>
> Ricardo Ribalda <ribalda@chromium.org> 于2026年9月4日周五 21:00写道：
>>
>> Hi Hongbing
>>
>> Let me rewrite the commit message for me, because it took me a while
>> to understand what was going on. (totally on me). Please correct me if
>> I miss-understood the issue.
>>
>> The UVC driver processes frames in urbs. A frame usually is divided in
>> multiple urbs. The urb handler parses the header and the metadata, but
>> leaves the expensive memcpy to a workqueue.
>>
>> At any given moment, during streamon we will have:
>> - the current frame with (0-N) async memcpys waiting to happen. The
>> current frame is in queue->irqqueue. It has a refcnt of 1 + N memcpy
>> - Previous frames with (N) async memcpys waiting to happen. They are
>> not in queue->irqqueue. They have a refcnt of N memcpy
>>
>> With the current behaviour, when there is urb with an error we flush
>> the queue->irqque() irrespetively if they have any pending memcpys.
>>
>> There can be situations where the memcpy occurs after we have flushed
>> the frames and they are returned to the user. Instead, we should
>> exploit the refcnt mechanism to return the frames only after the
>> memcpys have been completed.
>>
>> This patch does so
>>
>> On Thu, 3 Sept 2026 at 16:33, Hongbing Hu <huhb04@gmail.com> wrote:
>> >
>> > From: Hongbing Hu <huhb04@gmail.com>
>> >
>> > uvc_queue_cancel() returns queued buffers to videobuf2 directly via
>> > vb2_buffer_done(). This is unsafe because asynchronous memcpy workers may
>> > still hold a reference on the same uvc_buffer. Once vb2_buffer_done() has
>> > run, userspace can dequeue and requeue the buffer while the old worker is
>> > still running, leading to the same list_head being inserted into irqqueue
>> > twice and corrupting the list.
>> >
>> > The crash manifests in two ways depending on the drop-corrupted-frames
>> > module parameter:
>> >
>> > 1. With the drop corrupted frames enabled, the old worker's
>> >    final kref_put reaches uvc_queue_buffer_complete(), sees buf->error set,
>> >    and calls uvc_queue_buffer_requeue(). This performs list_add_tail() on
>> >    buf->queue even though userspace has already QBUF'd the same buffer and
>> >    added it to irqqueue.
>> >
>> > 2. With nodrop=1, uvc_queue_buffer_complete() calls vb2_buffer_done() a
>> >    second time. If userspace has already dequeued and requeued the buffer,
>> >    the second completion exposes it to userspace again, and the next
>> >    DQBUF/QBUF inserts the still-linked list_head into irqqueue a second
>> >    time.
>> >
>> > Both cases produce the kind of list_del corruption seen here:
>> >
>> >     list_del corruption. next->prev should be ..., but was ...
>> >     kernel BUG at lib/list_debug.c:64!
>> >
>> > Fix the cancel path to remove buffers from irqqueue and drop the queue's
>> > reference through uvc_queue_buffer_release() (i.e. kref_put()). The final
>> > VB2 completion then only happens from uvc_queue_buffer_complete() once the
>> > last async reference is released, so userspace cannot reuse the buffer
>> > while an old worker still accesses it.
>> >
>> > A buffer whose state is already UVC_BUF_STATE_ERROR is forced to complete
>> > as an error and must not be requeued by the corrupted-frame policy. Normal
>> > malformed frames are still requeued/dropped as before.
>> >
>> > Fixes: 01e90464e42e ("media: uvcvideo: queue: Support asynchronous buffer handling")
>> > Cc: stable@vger.kernel.org
>> > Signed-off-by: Hongbing Hu <huhb04@gmail.com>
>> > ---
>> > v2: corrected author real name in From/SOB per maintainer feedback
>> >
>> >  drivers/media/usb/uvc/uvc_queue.c | 30 ++++++++++++++++++++++++++++--
>> >  1 file changed, 28 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
>> > index 3c002c8f44..9206e0a355 100644
>> > --- a/drivers/media/usb/uvc/uvc_queue.c
>> > +++ b/drivers/media/usb/uvc/uvc_queue.c
>> > @@ -289,10 +289,19 @@ int uvc_queue_init(struct uvc_streaming *stream, struct uvc_video_queue *queue,
>> >   */
>> >  void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect)
>> >  {
>> > +       struct uvc_buffer *buf, *next;
>> > +       struct list_head local_list;
>> >         unsigned long flags;
>> >
>> > +       INIT_LIST_HEAD(&local_list);
>> > +
>> >         spin_lock_irqsave(&queue->irqlock, flags);
>> > -       __uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
>> > +       while (!list_empty(&queue->irqqueue)) {
>> > +               buf = list_first_entry(&queue->irqqueue, struct uvc_buffer, queue);
>> > +               list_del(&buf->queue);
>> > +               buf->state = UVC_BUF_STATE_ERROR;
>> > +               list_add_tail(&buf->queue, &local_list);
>> > +       }
>> >         /*
>> >          * This must be protected by the irqlock spinlock to avoid race
>> >          * conditions between uvc_buffer_queue and the disconnection event that
>> > @@ -303,6 +312,17 @@ void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect)
>> >         if (disconnect)
>> >                 queue->flags |= UVC_QUEUE_DISCONNECTED;
>> >         spin_unlock_irqrestore(&queue->irqlock, flags);
>>
>>
>> I *think* that you do not need to call uvc_queue_buffer_release
>> outisde spin_unlock_irqrestore(). You could do it
>> inside the previous while() and simplify the code.
>>
>> The only way to deadlock is  if uvc_queue_buffer_requeue() is called
>> and you are actively blocking that in your next snippet.
>>
>> I would recommend to add a comment in uvc_queue_buffer_complete if you
>> go that way.
>
> OK， good idea。
> I checked the code again.
> Reusing `buf->state` might make the code look a little confusing. Would it be better to add a new member "buf->cancelled"? For example, modify it like this:
> vc_queue_cancel()：buf->cancelled = true; buf->error = 1;uvc_queue_buffer_release(buf);
> uvc_buffer_prepare()：buf->cancelled = false;
> uvc_queue_buffer_complete: !buf->cancelled && buf->error && !nodrop
>
> What do you think of this change? Thanks!


Both approaches work for me. I have no strong preference for one or
the other. Lets wait for Laurent or Hans. They might think otherwise.

>
> diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
> index 3c002c8f44..1026d7def3 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -122,6 +122,7 @@ static int uvc_buffer_prepare(struct vb2_buffer *vb)
>
>   buf->state = UVC_BUF_STATE_QUEUED;
>   buf->error = 0;
> + buf->cancelled = false;
>   buf->mem = vb2_plane_vaddr(vb, 0);
>   buf->length = vb2_plane_size(vb, 0);
>   if (vb->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> @@ -289,10 +290,17 @@ int uvc_queue_init(struct uvc_streaming *stream, struct uvc_video_queue *queue,
>   */
>  void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect)
>  {
> + struct uvc_buffer *buf;
>   unsigned long flags;
>
>   spin_lock_irqsave(&queue->irqlock, flags);
> - __uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
> + while (!list_empty(&queue->irqqueue)) {
> + buf = list_first_entry(&queue->irqqueue, struct uvc_buffer, queue);
> + list_del(&buf->queue);
> + buf->error = 1;
> + buf->cancelled = true;
> + uvc_queue_buffer_release(buf);
> + }
>   /*
>   * This must be protected by the irqlock spinlock to avoid race
>   * conditions between uvc_buffer_queue and the disconnection event that
> @@ -356,7 +364,12 @@ static void uvc_queue_buffer_complete(struct kref *ref)
>   struct vb2_buffer *vb = &buf->buf.vb2_buf;
>   struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);
>
> - if (buf->error && !uvc_no_drop_param) {
> + /*
> + * Buffers cancelled from uvc_queue_cancel() are forced to complete as
> + * errors. They must not be requeued by the corrupted-frame policy even
> + * when buf->error is set.
> + */
> + if (!buf->cancelled && buf->error && !uvc_no_drop_param) {
>   uvc_queue_buffer_requeue(queue, buf);
>   return;
>   }
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index b6bcee4a22..1b611715fa 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -317,6 +317,7 @@ struct uvc_buffer {
>
>   enum uvc_buffer_state state;
>   unsigned int error;
> + bool cancelled;
>
>   void *mem;
>   unsigned int length;
> --
> 2.34.1
>
>> > +
>> > +       /*
>> > +        * Release the queue-owned kref outside the irqlock. The final VB2
>> > +        * completion only happens from uvc_queue_buffer_complete() once all
>> > +        * asynchronous copy references have been released, preventing userspace
>> > +        * from reusing the buffer while an old worker still accesses it.
>> > +        */
>> > +       list_for_each_entry_safe(buf, next, &local_list, queue) {
>> > +               list_del(&buf->queue);
>> > +               uvc_queue_buffer_release(buf);
>> > +       }
>> >  }
>> >
>> >  /*
>> > @@ -356,7 +376,13 @@ static void uvc_queue_buffer_complete(struct kref *ref)
>> >         struct vb2_buffer *vb = &buf->buf.vb2_buf;
>> >         struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);
>>
>> If we called uvc_queue_cancel() buf->error is not 1, but the frame is
>> not complete.
>>
>> You cannot return VB2_BUF_STATE_DONE in that situation. I think you
>> should set buf->error =1 inside uvc_queue_cancel
>>
>> >
>> > -       if (buf->error && !uvc_no_drop_param) {
>> > +       /*
>> > +        * Buffers cancelled from uvc_queue_cancel() are forced to complete as
>> > +        * errors. They must not be requeued by the corrupted-frame policy even
>> > +        * when buf->error is set.
>> > +        */
>> > +       if (buf->state != UVC_BUF_STATE_ERROR &&
>> > +           buf->error && !uvc_no_drop_param) {
>> >                 uvc_queue_buffer_requeue(queue, buf);
>> >                 return;
>> >         }
>> > --
>> > 2.34.1
>> >
>> >
>>
>>
>> --
>> Ricardo Ribalda



-- 
Ricardo Ribalda

[PATCH v3] media: uvcvideo: defer cancelled buffer completion until copies finish

[Hongbing Hu]

uvc_queue_cancel() returns queued buffers to videobuf2 directly via
vb2_buffer_done(). This is unsafe because asynchronous memcpy workers may
still hold a reference on the same uvc_buffer. Once vb2_buffer_done() has
run, userspace can dequeue and requeue the buffer while the old worker is
still running, leading to the same list_head being inserted into irqqueue
twice and corrupting the list.

The crash manifests in two ways depending on the drop-corrupted-frames
module parameter:

1. With the default (drop corrupted frames enabled), the old worker's
   final kref_put reaches uvc_queue_buffer_complete(), sees buf->error set,
   and calls uvc_queue_buffer_requeue(). This performs list_add_tail() on
   buf->queue even though userspace has already QBUF'd the same buffer and
   added it to irqqueue.

2. With nodrop=1, uvc_queue_buffer_complete() calls vb2_buffer_done() a
   second time. If userspace has already dequeued and requeued the buffer,
   the second completion exposes it to userspace again, and the next
   DQBUF/QBUF inserts the still-linked list_head into irqqueue a second
   time.

Both cases produce the kind of list_del corruption seen here:

    list_del corruption. next->prev should be ..., but was ...
    kernel BUG at lib/list_debug.c:64!

Fix the cancel path to remove buffers from irqqueue and drop the queue's
reference through uvc_queue_buffer_release() (i.e. kref_put()). The final
VB2 completion then only happens from uvc_queue_buffer_complete() once the
last async reference is released, so userspace cannot reuse the buffer
while an old worker still accesses it.

Introduce a dedicated bool cancelled flag in struct uvc_buffer,this keeps
the cancellation semantics explicit regardless of any later state updates
from the packet decoder.
Cancelled buffers are forced to complete as errors; normal malformed frames
continue to be requeued or dropped as before.

Fixes: 01e90464e42e ("media: uvcvideo: queue: Support asynchronous buffer handling")
Cc: stable@vger.kernel.org
Signed-off-by: Hongbing Hu <huhb04@gmail.com>
---
v3:
- invoke uvc_queue_buffer_release() while holding the spinlock to keep it simple
- Set/clear the new cancelled flag in uvc_queue_cancel() and
  uvc_buffer_prepare() instead of overloading buf->state. 
- Force cancelled buffers to complete as errors,
  so the returned buff->state is UVC_BUF_STATE_ERROR.

 drivers/media/usb/uvc/uvc_queue.c | 17 +++++++++++++++--
 drivers/media/usb/uvc/uvcvideo.h  |  1 +
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
index 3c002c8f44..1026d7def3 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -122,6 +122,7 @@ static int uvc_buffer_prepare(struct vb2_buffer *vb)
 
 	buf->state = UVC_BUF_STATE_QUEUED;
 	buf->error = 0;
+	buf->cancelled = false;
 	buf->mem = vb2_plane_vaddr(vb, 0);
 	buf->length = vb2_plane_size(vb, 0);
 	if (vb->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
@@ -289,10 +290,17 @@ int uvc_queue_init(struct uvc_streaming *stream, struct uvc_video_queue *queue,
  */
 void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect)
 {
+	struct uvc_buffer *buf;
 	unsigned long flags;
 
 	spin_lock_irqsave(&queue->irqlock, flags);
-	__uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
+	while (!list_empty(&queue->irqqueue)) {
+		buf = list_first_entry(&queue->irqqueue, struct uvc_buffer, queue);
+		list_del(&buf->queue);
+		buf->error = 1;
+		buf->cancelled = true;
+		uvc_queue_buffer_release(buf);
+	}
 	/*
 	 * This must be protected by the irqlock spinlock to avoid race
 	 * conditions between uvc_buffer_queue and the disconnection event that
@@ -356,7 +364,12 @@ static void uvc_queue_buffer_complete(struct kref *ref)
 	struct vb2_buffer *vb = &buf->buf.vb2_buf;
 	struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);
 
-	if (buf->error && !uvc_no_drop_param) {
+	/*
+	 * Buffers cancelled from uvc_queue_cancel() are forced to complete as
+	 * errors. They must not be requeued by the corrupted-frame policy even
+	 * when buf->error is set.
+	 */
+	if (!buf->cancelled && buf->error && !uvc_no_drop_param) {
 		uvc_queue_buffer_requeue(queue, buf);
 		return;
 	}
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index b6bcee4a22..1b611715fa 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -317,6 +317,7 @@ struct uvc_buffer {
 
 	enum uvc_buffer_state state;
 	unsigned int error;
+	bool cancelled;
 
 	void *mem;
 	unsigned int length;
-- 
2.34.1

Re: [PATCH v3] media: uvcvideo: defer cancelled buffer completion until copies finish

[Ricardo Ribalda]

Hi Hongbing

Something is wrong with your mail agent, it should not add:
In-Reply-To: <20260903131141.6368-1-huhb1@xiaopeng.com>
References: <20260903131141.6368-1-huhb1@xiaopeng.com>

Otherwise it is added to the previous email thread [1]

[1] https://lore.kernel.org/linux-media/20260917144630.16923-1-huhb04@gmail.com/

Have you looked into using b4 for contributing code? I find it much
simpler to use.

On Thu, 17 Sept 2026 at 16:47, Hongbing Hu <huhb04@gmail.com> wrote:
>
> uvc_queue_cancel() returns queued buffers to videobuf2 directly via
> vb2_buffer_done(). This is unsafe because asynchronous memcpy workers may
> still hold a reference on the same uvc_buffer. Once vb2_buffer_done() has
> run, userspace can dequeue and requeue the buffer while the old worker is
> still running, leading to the same list_head being inserted into irqqueue
> twice and corrupting the list.
>
> The crash manifests in two ways depending on the drop-corrupted-frames
> module parameter:
>
> 1. With the default (drop corrupted frames enabled), the old worker's
>    final kref_put reaches uvc_queue_buffer_complete(), sees buf->error set,
>    and calls uvc_queue_buffer_requeue(). This performs list_add_tail() on
>    buf->queue even though userspace has already QBUF'd the same buffer and
>    added it to irqqueue.
>
> 2. With nodrop=1, uvc_queue_buffer_complete() calls vb2_buffer_done() a
>    second time. If userspace has already dequeued and requeued the buffer,
>    the second completion exposes it to userspace again, and the next
>    DQBUF/QBUF inserts the still-linked list_head into irqqueue a second
>    time.
>
> Both cases produce the kind of list_del corruption seen here:
>
>     list_del corruption. next->prev should be ..., but was ...
>     kernel BUG at lib/list_debug.c:64!
>
> Fix the cancel path to remove buffers from irqqueue and drop the queue's
> reference through uvc_queue_buffer_release() (i.e. kref_put()). The final
> VB2 completion then only happens from uvc_queue_buffer_complete() once the
> last async reference is released, so userspace cannot reuse the buffer
> while an old worker still accesses it.
>
> Introduce a dedicated bool cancelled flag in struct uvc_buffer,this keeps
> the cancellation semantics explicit regardless of any later state updates
> from the packet decoder.
> Cancelled buffers are forced to complete as errors; normal malformed frames
> continue to be requeued or dropped as before.

I prefer my commit message [2], but I am probably biased :)

[2] https://lore.kernel.org/linux-media/CANiDSCuxtH1wbXAwoWrbV6Y97dN3Ym-A2zSbkEpLVEGFToXLfg@mail.gmail.com/

>
> Fixes: 01e90464e42e ("media: uvcvideo: queue: Support asynchronous buffer handling")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongbing Hu <huhb04@gmail.com>
> ---
> v3:
> - invoke uvc_queue_buffer_release() while holding the spinlock to keep it simple
> - Set/clear the new cancelled flag in uvc_queue_cancel() and
>   uvc_buffer_prepare() instead of overloading buf->state.
> - Force cancelled buffers to complete as errors,
>   so the returned buff->state is UVC_BUF_STATE_ERROR.
>
>  drivers/media/usb/uvc/uvc_queue.c | 17 +++++++++++++++--
>  drivers/media/usb/uvc/uvcvideo.h  |  1 +
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
> index 3c002c8f44..1026d7def3 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -122,6 +122,7 @@ static int uvc_buffer_prepare(struct vb2_buffer *vb)
>
>         buf->state = UVC_BUF_STATE_QUEUED;
>         buf->error = 0;
> +       buf->cancelled = false;
>         buf->mem = vb2_plane_vaddr(vb, 0);
>         buf->length = vb2_plane_size(vb, 0);
>         if (vb->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> @@ -289,10 +290,17 @@ int uvc_queue_init(struct uvc_streaming *stream, struct uvc_video_queue *queue,
>   */
>  void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect)
>  {
> +       struct uvc_buffer *buf;
>         unsigned long flags;
>
>         spin_lock_irqsave(&queue->irqlock, flags);
> -       __uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
> +       while (!list_empty(&queue->irqqueue)) {
> +               buf = list_first_entry(&queue->irqqueue, struct uvc_buffer, queue);
> +               list_del(&buf->queue);
> +               buf->error = 1;
> +               buf->cancelled = true;
> +               uvc_queue_buffer_release(buf);
> +       }
>         /*
>          * This must be protected by the irqlock spinlock to avoid race
>          * conditions between uvc_buffer_queue and the disconnection event that
> @@ -356,7 +364,12 @@ static void uvc_queue_buffer_complete(struct kref *ref)

Please add a comment to the function header noting that if
buf->cancelled is true, this function may be called with
queue->irqlock held.

I am dreaming of a day where we can finally remove uvc_no_drop_param :)


>         struct vb2_buffer *vb = &buf->buf.vb2_buf;
>         struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);
>
> -       if (buf->error && !uvc_no_drop_param) {
> +       /*
> +        * Buffers cancelled from uvc_queue_cancel() are forced to complete as
> +        * errors. They must not be requeued by the corrupted-frame policy even
> +        * when buf->error is set.
> +        */
> +       if (!buf->cancelled && buf->error && !uvc_no_drop_param) {
>                 uvc_queue_buffer_requeue(queue, buf);
>                 return;
>         }
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index b6bcee4a22..1b611715fa 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -317,6 +317,7 @@ struct uvc_buffer {
>
>         enum uvc_buffer_state state;
>         unsigned int error;
> +       bool cancelled;
>
>         void *mem;
>         unsigned int length;
> --
> 2.34.1
>

With the comment (and maybe a new commit message, up to you), feel
free to add my:

Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>


-- 
Ricardo Ribalda

Re: [PATCH v3] media: uvcvideo: defer cancelled buffer completion until copies finish

[Laurent Pinchart]

On Thu, Sep 17, 2026 at 10:46:30PM +0800, Hongbing Hu wrote:
> uvc_queue_cancel() returns queued buffers to videobuf2 directly via
> vb2_buffer_done(). This is unsafe because asynchronous memcpy workers may
> still hold a reference on the same uvc_buffer. Once vb2_buffer_done() has
> run, userspace can dequeue and requeue the buffer while the old worker is
> still running, leading to the same list_head being inserted into irqqueue
> twice and corrupting the list.
> 
> The crash manifests in two ways depending on the drop-corrupted-frames
> module parameter:
> 
> 1. With the default (drop corrupted frames enabled), the old worker's
>    final kref_put reaches uvc_queue_buffer_complete(), sees buf->error set,

What sets buf->error in that case ?

>    and calls uvc_queue_buffer_requeue(). This performs list_add_tail() on
>    buf->queue even though userspace has already QBUF'd the same buffer and
>    added it to irqqueue.
> 
> 2. With nodrop=1, uvc_queue_buffer_complete() calls vb2_buffer_done() a
>    second time. If userspace has already dequeued and requeued the buffer,
>    the second completion exposes it to userspace again, and the next
>    DQBUF/QBUF inserts the still-linked list_head into irqqueue a second
>    time.
> 
> Both cases produce the kind of list_del corruption seen here:
> 
>     list_del corruption. next->prev should be ..., but was ...
>     kernel BUG at lib/list_debug.c:64!
> 
> Fix the cancel path to remove buffers from irqqueue and drop the queue's
> reference through uvc_queue_buffer_release() (i.e. kref_put()). The final
> VB2 completion then only happens from uvc_queue_buffer_complete() once the
> last async reference is released, so userspace cannot reuse the buffer
> while an old worker still accesses it.
> 
> Introduce a dedicated bool cancelled flag in struct uvc_buffer,this keeps
> the cancellation semantics explicit regardless of any later state updates
> from the packet decoder.
> Cancelled buffers are forced to complete as errors; normal malformed frames
> continue to be requeued or dropped as before.
> 
> Fixes: 01e90464e42e ("media: uvcvideo: queue: Support asynchronous buffer handling")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongbing Hu <huhb04@gmail.com>
> ---
> v3:
> - invoke uvc_queue_buffer_release() while holding the spinlock to keep it simple
> - Set/clear the new cancelled flag in uvc_queue_cancel() and
>   uvc_buffer_prepare() instead of overloading buf->state. 
> - Force cancelled buffers to complete as errors,
>   so the returned buff->state is UVC_BUF_STATE_ERROR.
> 
>  drivers/media/usb/uvc/uvc_queue.c | 17 +++++++++++++++--
>  drivers/media/usb/uvc/uvcvideo.h  |  1 +
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
> index 3c002c8f44..1026d7def3 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -122,6 +122,7 @@ static int uvc_buffer_prepare(struct vb2_buffer *vb)
>  
>  	buf->state = UVC_BUF_STATE_QUEUED;
>  	buf->error = 0;
> +	buf->cancelled = false;
>  	buf->mem = vb2_plane_vaddr(vb, 0);
>  	buf->length = vb2_plane_size(vb, 0);
>  	if (vb->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> @@ -289,10 +290,17 @@ int uvc_queue_init(struct uvc_streaming *stream, struct uvc_video_queue *queue,
>   */
>  void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect)
>  {
> +	struct uvc_buffer *buf;
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&queue->irqlock, flags);
> -	__uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
> +	while (!list_empty(&queue->irqqueue)) {
> +		buf = list_first_entry(&queue->irqqueue, struct uvc_buffer, queue);
> +		list_del(&buf->queue);
> +		buf->error = 1;
> +		buf->cancelled = true;
> +		uvc_queue_buffer_release(buf);
> +	}
>  	/*
>  	 * This must be protected by the irqlock spinlock to avoid race
>  	 * conditions between uvc_buffer_queue and the disconnection event that
> @@ -356,7 +364,12 @@ static void uvc_queue_buffer_complete(struct kref *ref)
>  	struct vb2_buffer *vb = &buf->buf.vb2_buf;
>  	struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);
>  
> -	if (buf->error && !uvc_no_drop_param) {
> +	/*
> +	 * Buffers cancelled from uvc_queue_cancel() are forced to complete as
> +	 * errors. They must not be requeued by the corrupted-frame policy even
> +	 * when buf->error is set.
> +	 */
> +	if (!buf->cancelled && buf->error && !uvc_no_drop_param) {
>  		uvc_queue_buffer_requeue(queue, buf);
>  		return;
>  	}
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index b6bcee4a22..1b611715fa 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -317,6 +317,7 @@ struct uvc_buffer {
>  
>  	enum uvc_buffer_state state;
>  	unsigned int error;
> +	bool cancelled;
>  
>  	void *mem;
>  	unsigned int length;

-- 
Regards,

Laurent Pinchart