[PATCH for v4.20 0/5] vb2 fixes (mostly request API related)

[PATCH for v4.20 0/5] vb2 fixes (mostly request API related)

[hverkuil-cisco]

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

While improving the v4l2-compliance tests I came across several vb2
problems.

After modifying v4l2-compliance I was now able to use the vivid error
injection feature to test what happens if VIDIOC_STREAMON fails and
a following STREAMON succeeds.

This generated patches 1/5 and 4/5+5/5.

Patch 1/5 fixes an issue that was never noticed before since it didn't
result in kernel oopses or warnings, but after the second STREAMON it
won't call start_streaming anymore until you call REQBUFS(0) or close
the device node.

Patches 4 and 5 are request specific fixes for the same corner case:
the code would unbind (in vb2) or complete (in vivid) a request if
start_streaming fails, but it should just leave things as-is. The
buffer is just put back in the queue, ready for the next attempt at
STREAMON.

Patch 2/5 was also discovered by v4l2-compliance: the request fd in
v4l2_buffer should be ignored by VIDIOC_PREPARE_BUF, but it wasn't.

Patch 3/5 fixes a nasty corner case: a buffer with associated request
is queued, and then the request fd is closed by v4l2-compliance.

When the driver calls vb2_buffer_done, the buffer request object is
unbound, the object is put, and indirectly the associated request
is put as well, and since that was the last references to the request
the whole request is released, which requires the ability to call
mutex_lock. But vb2_buffer_done is atomic (it can be called
from interrupt context), so this shouldn't happen.

vb2 now takes an extra refcount to the request on qbuf and releases
it on dqbuf and at two other places where an internal dqbuf is done.

Note that 'skip request checks for VIDIOC_PREPARE_BUF' is a duplicate
of https://patchwork.linuxtv.org/patch/53171/, which is now marked as
superseded.

I've marked all these patches for 4.20, but I think it is also possible
to apply them for 4.21 since the request API is only used by virtual
drivers and a staging driver.

Regards,

	Hans

Hans Verkuil (5):
  vb2: don't call __vb2_queue_cancel if vb2_start_streaming failed
  vb2: skip request checks for VIDIOC_PREPARE_BUF
  vb2: keep a reference to the request until dqbuf
  vb2: don't unbind/put the object when going to state QUEUED
  vivid: drop v4l2_ctrl_request_complete() from start_streaming

 .../media/common/videobuf2/videobuf2-core.c   | 44 +++++++++++++++----
 .../media/common/videobuf2/videobuf2-v4l2.c   | 11 +++--
 drivers/media/platform/vivid/vivid-sdr-cap.c  |  2 -
 drivers/media/platform/vivid/vivid-vbi-cap.c  |  2 -
 drivers/media/platform/vivid/vivid-vbi-out.c  |  2 -
 drivers/media/platform/vivid/vivid-vid-cap.c  |  2 -
 drivers/media/platform/vivid/vivid-vid-out.c  |  2 -
 include/media/videobuf2-core.h                |  2 +
 8 files changed, 44 insertions(+), 23 deletions(-)

-- 
2.19.1

[PATCH for v4.20 1/5] vb2: don't call __vb2_queue_cancel if vb2_start_streaming failed

[hverkuil-cisco]

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

vb2_start_streaming() already rolls back the buffers, so there is no
need to call __vb2_queue_cancel(). Especially since __vb2_queue_cancel()
does too much, such as zeroing the q->queued_count value, causing vb2
to think that no buffers have been queued.

It appears that this call to __vb2_queue_cancel() is a left-over from
before commit b3379c6201bb3.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Fixes: b3379c6201bb3 ('vb2: only call start_streaming if sufficient buffers are queued')
Cc: <stable@vger.kernel.org>      # for v4.16 and up
---
 drivers/media/common/videobuf2/videobuf2-core.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 0ca81d495bda..77e2bfe5e722 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -1941,10 +1941,8 @@ int vb2_core_streamon(struct vb2_queue *q, unsigned int type)
 		if (ret)
 			return ret;
 		ret = vb2_start_streaming(q);
-		if (ret) {
-			__vb2_queue_cancel(q);
+		if (ret)
 			return ret;
-		}
 	}
 
 	q->streaming = 1;
-- 
2.19.1

[PATCH for v4.20 2/5] vb2: skip request checks for VIDIOC_PREPARE_BUF

[hverkuil-cisco]

From: Hans Verkuil <hansverk@cisco.com>

VIDIOC_PREPARE_BUF should ignore V4L2_BUF_FLAG_REQUEST_FD since it isn't
doing anything with requests. So inform vb2_queue_or_prepare_buf whether
it is called from vb2_prepare_buf or vb2_qbuf and just return 0 in the
first case.

This was found when adding new v4l2-compliance checks.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/common/videobuf2/videobuf2-v4l2.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 1244c246d0c4..1ac1b3f334f6 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -332,10 +332,10 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b
 }
 
 static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *mdev,
-				    struct v4l2_buffer *b,
-				    const char *opname,
+				    struct v4l2_buffer *b, bool is_prepare,
 				    struct media_request **p_req)
 {
+	const char *opname = is_prepare ? "prepare_buf" : "qbuf";
 	struct media_request *req;
 	struct vb2_v4l2_buffer *vbuf;
 	struct vb2_buffer *vb;
@@ -377,6 +377,9 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md
 			return ret;
 	}
 
+	if (is_prepare)
+		return 0;
+
 	if (!(b->flags & V4L2_BUF_FLAG_REQUEST_FD)) {
 		if (q->uses_requests) {
 			dprintk(1, "%s: queue uses requests\n", opname);
@@ -656,7 +659,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct media_device *mdev,
 	if (b->flags & V4L2_BUF_FLAG_REQUEST_FD)
 		return -EINVAL;
 
-	ret = vb2_queue_or_prepare_buf(q, mdev, b, "prepare_buf", NULL);
+	ret = vb2_queue_or_prepare_buf(q, mdev, b, true, NULL);
 
 	return ret ? ret : vb2_core_prepare_buf(q, b->index, b);
 }
@@ -728,7 +731,7 @@ int vb2_qbuf(struct vb2_queue *q, struct media_device *mdev,
 		return -EBUSY;
 	}
 
-	ret = vb2_queue_or_prepare_buf(q, mdev, b, "qbuf", &req);
+	ret = vb2_queue_or_prepare_buf(q, mdev, b, false, &req);
 	if (ret)
 		return ret;
 	ret = vb2_core_qbuf(q, b->index, b, req);
-- 
2.19.1

[PATCH for v4.20 3/5] vb2: keep a reference to the request until dqbuf

[hverkuil-cisco]

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

When vb2_buffer_done is called the buffer is unbound from the
request and put. The media_request_object_put also 'put's the
request reference. If the application has already closed the
request fd, then that means that the request reference at that
point goes to 0 and the whole request is released.

This means that the control handler associated with the request is
also freed and that causes this kernel oops:

[174705.995401] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:908
[174705.995411] in_atomic(): 1, irqs_disabled(): 1, pid: 28071, name: vivid-000-vid-o
[174705.995416] 2 locks held by vivid-000-vid-o/28071:
[174705.995420]  #0: 000000001ea3a232 (&dev->mutex#3){....}, at: vivid_thread_vid_out+0x3f5/0x550 [vivid]
[174705.995447]  #1: 00000000e30a0d1e (&(&q->done_lock)->rlock){....}, at: vb2_buffer_done+0x92/0x1d0 [videobuf2_common]
[174705.995460] Preemption disabled at:
[174705.995461] [<0000000000000000>]           (null)
[174705.995472] CPU: 11 PID: 28071 Comm: vivid-000-vid-o Tainted: G        W         4.20.0-rc1-test-no #88
[174705.995476] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/19/2017
[174705.995481] Call Trace:
[174705.995500]  dump_stack+0x46/0x60
[174705.995512]  ___might_sleep.cold.79+0xe1/0xf1
[174705.995523]  __mutex_lock+0x50/0x8f0
[174705.995531]  ? find_held_lock+0x2d/0x90
[174705.995536]  ? find_held_lock+0x2d/0x90
[174705.995542]  ? find_held_lock+0x2d/0x90
[174705.995564]  ? v4l2_ctrl_handler_free.part.13+0x44/0x1d0 [videodev]
[174705.995576]  v4l2_ctrl_handler_free.part.13+0x44/0x1d0 [videodev]
[174705.995590]  v4l2_ctrl_request_release+0x1c/0x30 [videodev]
[174705.995600]  media_request_clean+0x64/0xe0 [media]
[174705.995609]  media_request_release+0x19/0x40 [media]
[174705.995617]  vb2_buffer_done+0xef/0x1d0 [videobuf2_common]
[174705.995630]  vivid_thread_vid_out+0x2c1/0x550 [vivid]
[174705.995645]  ? vivid_stop_generating_vid_cap+0x1c0/0x1c0 [vivid]
[174705.995653]  kthread+0x113/0x130
[174705.995659]  ? kthread_park+0x80/0x80
[174705.995667]  ret_from_fork+0x35/0x40

The vb2_buffer_done function can be called from interrupt context, so
anything that sleeps is not allowed.

The solution is to increment the request refcount when the buffer is
queued and decrement it when the buffer is dequeued. Releasing the
request is fine if that happens from VIDIOC_DQBUF.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 .../media/common/videobuf2/videobuf2-core.c   | 38 ++++++++++++++++---
 include/media/videobuf2-core.h                |  2 +
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 77e2bfe5e722..86a12b335aac 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -1360,8 +1360,12 @@ static void vb2_req_release(struct media_request_object *obj)
 {
 	struct vb2_buffer *vb = container_of(obj, struct vb2_buffer, req_obj);
 
-	if (vb->state == VB2_BUF_STATE_IN_REQUEST)
+	if (vb->state == VB2_BUF_STATE_IN_REQUEST) {
 		vb->state = VB2_BUF_STATE_DEQUEUED;
+		if (vb->request)
+			media_request_put(vb->request);
+		vb->request = NULL;
+	}
 }
 
 static const struct media_request_object_ops vb2_core_req_ops = {
@@ -1529,6 +1533,18 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
 			return ret;
 
 		vb->state = VB2_BUF_STATE_IN_REQUEST;
+
+		/*
+		 * Increment the refcount and store the request.
+		 * The request refcount is decremented again when the
+		 * buffer is dequeued. This is to prevent vb2_buffer_done()
+		 * from freeing the request from interrupt context, which can
+		 * happen if the application closed the request fd after
+		 * queueing the request.
+		 */
+		media_request_get(req);
+		vb->request = req;
+
 		/* Fill buffer information for the userspace */
 		if (pb) {
 			call_void_bufop(q, copy_timestamp, vb, pb);
@@ -1750,10 +1766,6 @@ static void __vb2_dqbuf(struct vb2_buffer *vb)
 			call_void_memop(vb, unmap_dmabuf, vb->planes[i].mem_priv);
 			vb->planes[i].dbuf_mapped = 0;
 		}
-	if (vb->req_obj.req) {
-		media_request_object_unbind(&vb->req_obj);
-		media_request_object_put(&vb->req_obj);
-	}
 	call_void_bufop(q, init_buffer, vb);
 }
 
@@ -1798,6 +1810,14 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
 	/* go back to dequeued state */
 	__vb2_dqbuf(vb);
 
+	if (WARN_ON(vb->req_obj.req)) {
+		media_request_object_unbind(&vb->req_obj);
+		media_request_object_put(&vb->req_obj);
+	}
+	if (vb->request)
+		media_request_put(vb->request);
+	vb->request = NULL;
+
 	dprintk(2, "dqbuf of buffer %d, with state %d\n",
 			vb->index, vb->state);
 
@@ -1904,6 +1924,14 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
 			vb->prepared = false;
 		}
 		__vb2_dqbuf(vb);
+
+		if (vb->req_obj.req) {
+			media_request_object_unbind(&vb->req_obj);
+			media_request_object_put(&vb->req_obj);
+		}
+		if (vb->request)
+			media_request_put(vb->request);
+		vb->request = NULL;
 	}
 }
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index e86981d615ae..4a737b2c610b 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -239,6 +239,7 @@ struct vb2_queue;
  * @num_planes:		number of planes in the buffer
  *			on an internal driver queue.
  * @timestamp:		frame timestamp in ns.
+ * @request:		the request this buffer is associated with.
  * @req_obj:		used to bind this buffer to a request. This
  *			request object has a refcount.
  */
@@ -249,6 +250,7 @@ struct vb2_buffer {
 	unsigned int		memory;
 	unsigned int		num_planes;
 	u64			timestamp;
+	struct media_request	*request;
 	struct media_request_object	req_obj;
 
 	/* private: internal use only
-- 
2.19.1

[PATCH for v4.20 4/5] vb2: don't unbind/put the object when going to state QUEUED

[hverkuil-cisco]

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

When a buffer is returned to state QUEUED (that happens when
start_streaming fails), then do not unbind and put the object
from the request. Nothing has changed yet, so just keep it as
is.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 86a12b335aac..70e8c3366f9c 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -948,7 +948,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 	}
 	atomic_dec(&q->owned_by_drv_count);
 
-	if (vb->req_obj.req) {
+	if (state != VB2_BUF_STATE_QUEUED && vb->req_obj.req) {
 		/* This is not supported at the moment */
 		WARN_ON(state == VB2_BUF_STATE_REQUEUEING);
 		media_request_object_unbind(&vb->req_obj);
-- 
2.19.1

[PATCH for v4.20 5/5] vivid: drop v4l2_ctrl_request_complete() from start_streaming

[hverkuil-cisco]

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

If start_streaming() fails and all queued buffers are returned to
vb2, then do not call v4l2_ctrl_request_complete(). Nothing happened
to the request and the state should remain as it was before
start_streaming was called.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/platform/vivid/vivid-sdr-cap.c | 2 --
 drivers/media/platform/vivid/vivid-vbi-cap.c | 2 --
 drivers/media/platform/vivid/vivid-vbi-out.c | 2 --
 drivers/media/platform/vivid/vivid-vid-cap.c | 2 --
 drivers/media/platform/vivid/vivid-vid-out.c | 2 --
 5 files changed, 10 deletions(-)

diff --git a/drivers/media/platform/vivid/vivid-sdr-cap.c b/drivers/media/platform/vivid/vivid-sdr-cap.c
index dcdc80e272c2..9acc709b0740 100644
--- a/drivers/media/platform/vivid/vivid-sdr-cap.c
+++ b/drivers/media/platform/vivid/vivid-sdr-cap.c
@@ -276,8 +276,6 @@ static int sdr_cap_start_streaming(struct vb2_queue *vq, unsigned count)
 
 		list_for_each_entry_safe(buf, tmp, &dev->sdr_cap_active, list) {
 			list_del(&buf->list);
-			v4l2_ctrl_request_complete(buf->vb.vb2_buf.req_obj.req,
-						   &dev->ctrl_hdl_sdr_cap);
 			vb2_buffer_done(&buf->vb.vb2_buf,
 					VB2_BUF_STATE_QUEUED);
 		}
diff --git a/drivers/media/platform/vivid/vivid-vbi-cap.c b/drivers/media/platform/vivid/vivid-vbi-cap.c
index 903cebeb5ce5..d666271bdaed 100644
--- a/drivers/media/platform/vivid/vivid-vbi-cap.c
+++ b/drivers/media/platform/vivid/vivid-vbi-cap.c
@@ -204,8 +204,6 @@ static int vbi_cap_start_streaming(struct vb2_queue *vq, unsigned count)
 
 		list_for_each_entry_safe(buf, tmp, &dev->vbi_cap_active, list) {
 			list_del(&buf->list);
-			v4l2_ctrl_request_complete(buf->vb.vb2_buf.req_obj.req,
-						   &dev->ctrl_hdl_vbi_cap);
 			vb2_buffer_done(&buf->vb.vb2_buf,
 					VB2_BUF_STATE_QUEUED);
 		}
diff --git a/drivers/media/platform/vivid/vivid-vbi-out.c b/drivers/media/platform/vivid/vivid-vbi-out.c
index 9357c07e30d6..cd56476902a2 100644
--- a/drivers/media/platform/vivid/vivid-vbi-out.c
+++ b/drivers/media/platform/vivid/vivid-vbi-out.c
@@ -96,8 +96,6 @@ static int vbi_out_start_streaming(struct vb2_queue *vq, unsigned count)
 
 		list_for_each_entry_safe(buf, tmp, &dev->vbi_out_active, list) {
 			list_del(&buf->list);
-			v4l2_ctrl_request_complete(buf->vb.vb2_buf.req_obj.req,
-						   &dev->ctrl_hdl_vbi_out);
 			vb2_buffer_done(&buf->vb.vb2_buf,
 					VB2_BUF_STATE_QUEUED);
 		}
diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c b/drivers/media/platform/vivid/vivid-vid-cap.c
index a1ed5fdabc75..c059fc12668a 100644
--- a/drivers/media/platform/vivid/vivid-vid-cap.c
+++ b/drivers/media/platform/vivid/vivid-vid-cap.c
@@ -243,8 +243,6 @@ static int vid_cap_start_streaming(struct vb2_queue *vq, unsigned count)
 
 		list_for_each_entry_safe(buf, tmp, &dev->vid_cap_active, list) {
 			list_del(&buf->list);
-			v4l2_ctrl_request_complete(buf->vb.vb2_buf.req_obj.req,
-						   &dev->ctrl_hdl_vid_cap);
 			vb2_buffer_done(&buf->vb.vb2_buf,
 					VB2_BUF_STATE_QUEUED);
 		}
diff --git a/drivers/media/platform/vivid/vivid-vid-out.c b/drivers/media/platform/vivid/vivid-vid-out.c
index 7642cbdb0e14..ea250aee2b2e 100644
--- a/drivers/media/platform/vivid/vivid-vid-out.c
+++ b/drivers/media/platform/vivid/vivid-vid-out.c
@@ -162,8 +162,6 @@ static int vid_out_start_streaming(struct vb2_queue *vq, unsigned count)
 
 		list_for_each_entry_safe(buf, tmp, &dev->vid_out_active, list) {
 			list_del(&buf->list);
-			v4l2_ctrl_request_complete(buf->vb.vb2_buf.req_obj.req,
-						   &dev->ctrl_hdl_vid_out);
 			vb2_buffer_done(&buf->vb.vb2_buf,
 					VB2_BUF_STATE_QUEUED);
 		}
-- 
2.19.1

Re: [PATCH for v4.20 1/5] vb2: don't call __vb2_queue_cancel if vb2_start_streaming failed

[Sakari Ailus]

On Wed, Nov 28, 2018 at 09:37:43AM +0100, hverkuil-cisco@xs4all.nl wrote:
> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> vb2_start_streaming() already rolls back the buffers, so there is no
> need to call __vb2_queue_cancel(). Especially since __vb2_queue_cancel()
> does too much, such as zeroing the q->queued_count value, causing vb2
> to think that no buffers have been queued.
> 
> It appears that this call to __vb2_queue_cancel() is a left-over from
> before commit b3379c6201bb3.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Fixes: b3379c6201bb3 ('vb2: only call start_streaming if sufficient buffers are queued')
> Cc: <stable@vger.kernel.org>      # for v4.16 and up

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

> ---
>  drivers/media/common/videobuf2/videobuf2-core.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index 0ca81d495bda..77e2bfe5e722 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -1941,10 +1941,8 @@ int vb2_core_streamon(struct vb2_queue *q, unsigned int type)
>  		if (ret)
>  			return ret;
>  		ret = vb2_start_streaming(q);
> -		if (ret) {
> -			__vb2_queue_cancel(q);
> +		if (ret)
>  			return ret;
> -		}
>  	}
>  
>  	q->streaming = 1;

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

Re: [PATCH for v4.20 0/5] vb2 fixes (mostly request API related)

[sakari.ailus]

On Wed, Nov 28, 2018 at 09:37:42AM +0100, hverkuil-cisco@xs4all.nl wrote:
> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> While improving the v4l2-compliance tests I came across several vb2
> problems.
> 
> After modifying v4l2-compliance I was now able to use the vivid error
> injection feature to test what happens if VIDIOC_STREAMON fails and
> a following STREAMON succeeds.
> 
> This generated patches 1/5 and 4/5+5/5.
> 
> Patch 1/5 fixes an issue that was never noticed before since it didn't
> result in kernel oopses or warnings, but after the second STREAMON it
> won't call start_streaming anymore until you call REQBUFS(0) or close
> the device node.
> 
> Patches 4 and 5 are request specific fixes for the same corner case:
> the code would unbind (in vb2) or complete (in vivid) a request if
> start_streaming fails, but it should just leave things as-is. The
> buffer is just put back in the queue, ready for the next attempt at
> STREAMON.
> 
> Patch 2/5 was also discovered by v4l2-compliance: the request fd in
> v4l2_buffer should be ignored by VIDIOC_PREPARE_BUF, but it wasn't.
> 
> Patch 3/5 fixes a nasty corner case: a buffer with associated request
> is queued, and then the request fd is closed by v4l2-compliance.
> 
> When the driver calls vb2_buffer_done, the buffer request object is
> unbound, the object is put, and indirectly the associated request
> is put as well, and since that was the last references to the request
> the whole request is released, which requires the ability to call
> mutex_lock. But vb2_buffer_done is atomic (it can be called
> from interrupt context), so this shouldn't happen.
> 
> vb2 now takes an extra refcount to the request on qbuf and releases
> it on dqbuf and at two other places where an internal dqbuf is done.
> 
> Note that 'skip request checks for VIDIOC_PREPARE_BUF' is a duplicate
> of https://patchwork.linuxtv.org/patch/53171/, which is now marked as
> superseded.
> 
> I've marked all these patches for 4.20, but I think it is also possible
> to apply them for 4.21 since the request API is only used by virtual
> drivers and a staging driver.

For patches 2--5:

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi