From patchwork Mon Dec 3 12:46:01 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans Verkuil X-Patchwork-Id: 10709475 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id EC9B014BD for ; Mon, 3 Dec 2018 12:46:20 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DDB962B324 for ; Mon, 3 Dec 2018 12:46:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D25E92B33F; Mon, 3 Dec 2018 12:46:20 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 650DC2B324 for ; Mon, 3 Dec 2018 12:46:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726495AbeLCMrD (ORCPT ); Mon, 3 Dec 2018 07:47:03 -0500 Received: from lb1-smtp-cloud7.xs4all.net ([194.109.24.24]:49737 "EHLO lb1-smtp-cloud7.xs4all.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726298AbeLCMrD (ORCPT ); Mon, 3 Dec 2018 07:47:03 -0500 Received: from tschai.fritz.box ([212.251.195.8]) by smtp-cloud7.xs4all.net with ESMTPA id TnbvgsFXOfGaRTnbygjDVJ; Mon, 03 Dec 2018 13:46:07 +0100 From: hverkuil-cisco@xs4all.nl To: linux-media@vger.kernel.org Cc: Sakari Ailus , Hans Verkuil Subject: [PATCH 1/3] vb2: add buf_validate callback Date: Mon, 3 Dec 2018 13:46:01 +0100 Message-Id: <20181203124603.17932-1-hverkuil-cisco@xs4all.nl> X-Mailer: git-send-email 2.19.1 MIME-Version: 1.0 X-CMAE-Envelope: MS4wfO+cokn8FA6LlsPECxJWLH1nBRThU2KEeyyDkzxQUDI0uw0Dj808+HmtEGQOSzRcTbPTLZ5CnMH9SMygusY71bN03HGCP/txCoYVRK4Vi1QA9x9XQOuU XO4VFWXxxyAkmxkp61TqR97+yrNJLj96Q/K0UBIJ/rBfP9oYc+lGYWnfN6Jhct/u67Uv4ZbJVDtMDKQhuyszUZEKZyx62Hg8fm/rkAkbV2/9HTdQXbEeamY3 6s5SVsGZGq0phX7QjE8NO50D8wvYr3QOFCup9gV3Egc= Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Hans Verkuil 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. This patch adds a new buf_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 --- drivers/media/common/videobuf2/videobuf2-core.c | 12 +++++++++--- include/media/videobuf2-core.h | 5 +++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index 0ca81d495bda..42eb7716f8a9 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_validate: %u buf_queue: %u buf_done: %u buf_request_complete: %u\n", + vb->cnt_buf_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, @@ -1506,6 +1506,12 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb, return -EBUSY; } + ret = call_vb_qop(vb, buf_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 e86981d615ae..c9f0f3f4ef9a 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -294,6 +294,7 @@ struct vb2_buffer { u32 cnt_mem_num_users; u32 cnt_mem_mmap; + u32 cnt_buf_validate; u32 cnt_buf_init; u32 cnt_buf_prepare; u32 cnt_buf_finish; @@ -340,6 +341,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_validate: called every time the 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; @@ -407,6 +411,7 @@ struct vb2_ops { void (*wait_prepare)(struct vb2_queue *q); void (*wait_finish)(struct vb2_queue *q); + int (*buf_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);