From: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
Adding the request API uncovered a pre-existing problem with
validating output buffers.
The problem is that for output buffers the driver has to validate
the 'field' field of struct v4l2_buffer. This is critical when
encoding or deinterlacing interlaced video.
Drivers always did this in the buf_prepare callback, but that is
not called from VIDIOC_QBUF in two situations: when queueing a
buffer to a request and if VIDIOC_PREPARE_BUF has been called
earlier for that buffer.
As a result of this the 'field' value is not validated.
While the first case (queueing a buffer to a request) is request
API specific, the second case (VIDIOC_PREPARE_BUF has been called
earlier for that buffer) was always wrong.
This patch adds a new buf_out_validate callback to validate the
output buffer at QBUF time.
Note that PREPARE_BUF doesn't need to validate this: it just
locks the buffer memory and doesn't need nor want to know about
how this buffer is actually going to be used. It's the QBUF ioctl
that determines this.
This issue was found by v4l2-compliance since it failed to replace
V4L2_FIELD_ANY by a proper field value when testing the vivid video
output in combination with requests.
There never was a test before for the PREPARE_BUF/QBUF case, so even
though this bug has been present for quite some time, it was never
noticed.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
---
drivers/media/common/videobuf2/videobuf2-core.c | 14 +++++++++++---
include/media/videobuf2-core.h | 5 +++++
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 70e8c3366f9c..653e933db854 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -499,9 +499,9 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
pr_info(" buf_init: %u buf_cleanup: %u buf_prepare: %u buf_finish: %u\n",
vb->cnt_buf_init, vb->cnt_buf_cleanup,
vb->cnt_buf_prepare, vb->cnt_buf_finish);
- pr_info(" buf_queue: %u buf_done: %u buf_request_complete: %u\n",
- vb->cnt_buf_queue, vb->cnt_buf_done,
- vb->cnt_buf_request_complete);
+ pr_info(" buf_out_validate: %u buf_queue: %u buf_done: %u buf_request_complete: %u\n",
+ vb->cnt_buf_out_validate, vb->cnt_buf_queue,
+ vb->cnt_buf_done, vb->cnt_buf_request_complete);
pr_info(" alloc: %u put: %u prepare: %u finish: %u mmap: %u\n",
vb->cnt_mem_alloc, vb->cnt_mem_put,
vb->cnt_mem_prepare, vb->cnt_mem_finish,
@@ -1510,6 +1510,14 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
return -EBUSY;
}
+ if (q->is_output) {
+ ret = call_vb_qop(vb, buf_out_validate, vb);
+ if (ret) {
+ dprintk(1, "buffer validation failed\n");
+ return ret;
+ }
+ }
+
if (req) {
int ret;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 4a737b2c610b..91f1e66aacc6 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -296,6 +296,7 @@ struct vb2_buffer {
u32 cnt_mem_num_users;
u32 cnt_mem_mmap;
+ u32 cnt_buf_out_validate;
u32 cnt_buf_init;
u32 cnt_buf_prepare;
u32 cnt_buf_finish;
@@ -342,6 +343,9 @@ struct vb2_buffer {
* @wait_finish: reacquire all locks released in the previous callback;
* required to continue operation after sleeping while
* waiting for a new buffer to arrive.
+ * @buf_out_validate: called every time the output buffer is queued from
+ * userspace; drivers can use this to validate
+ * userspace-provided information; optional.
* @buf_init: called once after allocating a buffer (in MMAP case)
* or after acquiring a new USERPTR buffer; drivers may
* perform additional buffer-related initialization;
@@ -409,6 +413,7 @@ struct vb2_ops {
void (*wait_prepare)(struct vb2_queue *q);
void (*wait_finish)(struct vb2_queue *q);
+ int (*buf_out_validate)(struct vb2_buffer *vb);
int (*buf_init)(struct vb2_buffer *vb);
int (*buf_prepare)(struct vb2_buffer *vb);
void (*buf_finish)(struct vb2_buffer *vb);
--
2.19.2
