From patchwork Thu Mar 25 00:17:11 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Helen Mae Koike Fornazier X-Patchwork-Id: 12162579 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A464CC433C1 for ; Thu, 25 Mar 2021 00:19:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5867861A1E for ; Thu, 25 Mar 2021 00:19:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239189AbhCYASv (ORCPT ); Wed, 24 Mar 2021 20:18:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36776 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239156AbhCYASY (ORCPT ); Wed, 24 Mar 2021 20:18:24 -0400 Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2DC2AC06174A; Wed, 24 Mar 2021 17:18:24 -0700 (PDT) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: koike) with ESMTPSA id 3C1971F40DCA From: Helen Koike To: linux-media@vger.kernel.org Cc: hverkuil@xs4all.nl, kernel@collabora.com, linux-kernel@vger.kernel.org, jc@kynesim.co.uk, laurent.pinchart@ideasonboard.com, dave.stevenson@raspberrypi.org, tfiga@chromium.org, Helen Koike Subject: [PATCH 1/2] media: videobuf2: use dmabuf size for length Date: Wed, 24 Mar 2021 21:17:11 -0300 Message-Id: <20210325001712.197837-1-helen.koike@collabora.com> X-Mailer: git-send-email 2.30.1 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Always use dmabuf size when considering the length of the buffer. Discard userspace provided length. Fix length check error in _verify_length(), which was handling single and multiplanar diferently, and also not catching the case where userspace provides a bigger length and bytesused then the underlying buffer. Suggested-by: Hans Verkuil Signed-off-by: Helen Koike --- Hello, As discussed on https://patchwork.linuxtv.org/project/linux-media/patch/gh5kef5bkeel3o6b2dkgc2dfagu9klj4c0@4ax.com/ This patch also helps the conversion layer of the Ext API patchset, where we are not exposing the length field. It was discussed that userspace might use a smaller length field to limit the usage of the underlying buffer, but I'm not sure if this is really usefull and just complicates things. If this is usefull, then we should also expose a length field in the Ext API, and document this feature properly. What do you think? --- .../media/common/videobuf2/videobuf2-core.c | 21 ++++++++++++++++--- .../media/common/videobuf2/videobuf2-v4l2.c | 8 +++---- include/uapi/linux/videodev2.h | 7 +++++-- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index 02281d13505f..2cbde14af051 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -1205,6 +1205,7 @@ static int __prepare_dmabuf(struct vb2_buffer *vb) for (plane = 0; plane < vb->num_planes; ++plane) { struct dma_buf *dbuf = dma_buf_get(planes[plane].m.fd); + unsigned int bytesused; if (IS_ERR_OR_NULL(dbuf)) { dprintk(q, 1, "invalid dmabuf fd for plane %d\n", @@ -1213,9 +1214,23 @@ static int __prepare_dmabuf(struct vb2_buffer *vb) goto err; } - /* use DMABUF size if length is not provided */ - if (planes[plane].length == 0) - planes[plane].length = dbuf->size; + planes[plane].length = dbuf->size; + bytesused = planes[plane].bytesused ? + planes[plane].bytesused : dbuf->size; + + if (planes[plane].bytesused > planes[plane].length) { + dprintk(q, 1, "bytesused is bigger then dmabuf length for plane %d\n", + plane); + ret = -EINVAL; + goto err; + } + + if (planes[plane].data_offset >= bytesused) { + dprintk(q, 1, "data_offset >= bytesused for plane %d\n", + plane); + ret = -EINVAL; + goto err; + } if (planes[plane].length < vb->planes[plane].min_length) { dprintk(q, 1, "invalid dmabuf length %u for plane %d, minimum length %u\n", diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index 7e96f67c60ba..ffc7ed46f74a 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -98,14 +98,14 @@ static int __verify_length(struct vb2_buffer *vb, const struct v4l2_buffer *b) unsigned int bytesused; unsigned int plane; - if (V4L2_TYPE_IS_CAPTURE(b->type)) + /* length check for dmabuf is performed in _prepare_dmabuf() */ + if (V4L2_TYPE_IS_CAPTURE(b->type) || b->memory == VB2_MEMORY_DMABUF) return 0; if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) { for (plane = 0; plane < vb->num_planes; ++plane) { - length = (b->memory == VB2_MEMORY_USERPTR || - b->memory == VB2_MEMORY_DMABUF) - ? b->m.planes[plane].length + length = b->memory == VB2_MEMORY_USERPTR + ? b->m.planes[plane].length : vb->planes[plane].length; bytesused = b->m.planes[plane].bytesused ? b->m.planes[plane].bytesused : length; diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 8d15f6ccc4b4..79b3b2893513 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -968,7 +968,9 @@ struct v4l2_requestbuffers { /** * struct v4l2_plane - plane info for multi-planar buffers * @bytesused: number of bytes occupied by data in the plane (payload) - * @length: size of this plane (NOT the payload) in bytes + * @length: size of this plane (NOT the payload) in bytes. Filled + * by userspace for USERPTR and by the driver for DMABUF + * and MMAP. * @mem_offset: when memory in the associated struct v4l2_buffer is * V4L2_MEMORY_MMAP, equals the offset from the start of * the device memory for this plane (or is a "cookie" that @@ -1025,7 +1027,8 @@ struct v4l2_plane { * @m: union of @offset, @userptr, @planes and @fd * @length: size in bytes of the buffer (NOT its payload) for single-plane * buffers (when type != *_MPLANE); number of elements in the - * planes array for multi-plane buffers + * planes array for multi-plane buffers. Filled by userspace for + * USERPTR and by the driver for DMABUF and MMAP. * @reserved2: drivers and applications must zero this field * @request_fd: fd of the request that this buffer should use * @reserved: for backwards compatibility with applications that do not know From patchwork Thu Mar 25 00:17:12 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Helen Mae Koike Fornazier X-Patchwork-Id: 12162577 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 85B76C433DB for ; Thu, 25 Mar 2021 00:19:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 39DB561A19 for ; Thu, 25 Mar 2021 00:19:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239193AbhCYASw (ORCPT ); Wed, 24 Mar 2021 20:18:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36814 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239163AbhCYASf (ORCPT ); Wed, 24 Mar 2021 20:18:35 -0400 Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C029CC06174A; Wed, 24 Mar 2021 17:18:34 -0700 (PDT) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: koike) with ESMTPSA id 93A621F45EF9 From: Helen Koike To: linux-media@vger.kernel.org Cc: hverkuil@xs4all.nl, kernel@collabora.com, linux-kernel@vger.kernel.org, jc@kynesim.co.uk, laurent.pinchart@ideasonboard.com, dave.stevenson@raspberrypi.org, tfiga@chromium.org, Helen Koike Subject: [PATCH 2/2] media: videobuf2: cleanup size argument from attach_dmabuf() Date: Wed, 24 Mar 2021 21:17:12 -0300 Message-Id: <20210325001712.197837-2-helen.koike@collabora.com> X-Mailer: git-send-email 2.30.1 In-Reply-To: <20210325001712.197837-1-helen.koike@collabora.com> References: <20210325001712.197837-1-helen.koike@collabora.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Since we always use the size of the underlying buffer for dmabuf, remove the size parameter from the attach_dmabuf() callback. Suggested-by: Hans Verkuil Signed-off-by: Helen Koike Reported-by: kernel test robot --- drivers/media/common/videobuf2/videobuf2-core.c | 2 +- drivers/media/common/videobuf2/videobuf2-dma-contig.c | 7 ++----- drivers/media/common/videobuf2/videobuf2-dma-sg.c | 7 ++----- drivers/media/common/videobuf2/videobuf2-vmalloc.c | 7 ++----- include/media/videobuf2-core.h | 1 - 5 files changed, 7 insertions(+), 17 deletions(-) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index 2cbde14af051..86af4f3c72eb 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -1266,7 +1266,7 @@ static int __prepare_dmabuf(struct vb2_buffer *vb) /* Acquire each plane's memory */ mem_priv = call_ptr_memop(vb, attach_dmabuf, q->alloc_devs[plane] ? : q->dev, - dbuf, planes[plane].length, q->dma_dir); + dbuf, q->dma_dir); if (IS_ERR(mem_priv)) { dprintk(q, 1, "failed to attach dmabuf\n"); ret = PTR_ERR(mem_priv); diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c index a7f61ba85440..a26aa52f954b 100644 --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c @@ -661,14 +661,11 @@ static void vb2_dc_detach_dmabuf(void *mem_priv) } static void *vb2_dc_attach_dmabuf(struct device *dev, struct dma_buf *dbuf, - unsigned long size, enum dma_data_direction dma_dir) + enum dma_data_direction dma_dir) { struct vb2_dc_buf *buf; struct dma_buf_attachment *dba; - if (dbuf->size < size) - return ERR_PTR(-EFAULT); - if (WARN_ON(!dev)) return ERR_PTR(-EINVAL); @@ -686,7 +683,7 @@ static void *vb2_dc_attach_dmabuf(struct device *dev, struct dma_buf *dbuf, } buf->dma_dir = dma_dir; - buf->size = size; + buf->size = dbuf->size; buf->db_attach = dba; return buf; diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c index c5b06a509566..8c006f79bed4 100644 --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c @@ -606,7 +606,7 @@ static void vb2_dma_sg_detach_dmabuf(void *mem_priv) } static void *vb2_dma_sg_attach_dmabuf(struct device *dev, struct dma_buf *dbuf, - unsigned long size, enum dma_data_direction dma_dir) + enum dma_data_direction dma_dir) { struct vb2_dma_sg_buf *buf; struct dma_buf_attachment *dba; @@ -614,9 +614,6 @@ static void *vb2_dma_sg_attach_dmabuf(struct device *dev, struct dma_buf *dbuf, if (WARN_ON(!dev)) return ERR_PTR(-EINVAL); - if (dbuf->size < size) - return ERR_PTR(-EFAULT); - buf = kzalloc(sizeof(*buf), GFP_KERNEL); if (!buf) return ERR_PTR(-ENOMEM); @@ -631,7 +628,7 @@ static void *vb2_dma_sg_attach_dmabuf(struct device *dev, struct dma_buf *dbuf, } buf->dma_dir = dma_dir; - buf->size = size; + buf->size = dmabuf->size; buf->db_attach = dba; return buf; diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c index 83f95258ec8c..c2d41b375c10 100644 --- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c +++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c @@ -404,20 +404,17 @@ static void vb2_vmalloc_detach_dmabuf(void *mem_priv) } static void *vb2_vmalloc_attach_dmabuf(struct device *dev, struct dma_buf *dbuf, - unsigned long size, enum dma_data_direction dma_dir) + enum dma_data_direction dma_dir) { struct vb2_vmalloc_buf *buf; - if (dbuf->size < size) - return ERR_PTR(-EFAULT); - buf = kzalloc(sizeof(*buf), GFP_KERNEL); if (!buf) return ERR_PTR(-ENOMEM); buf->dbuf = dbuf; buf->dma_dir = dma_dir; - buf->size = size; + buf->size = dbuf->size; return buf; } diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index 12955cb460d2..db07001cada8 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -134,7 +134,6 @@ struct vb2_mem_ops { void *(*attach_dmabuf)(struct device *dev, struct dma_buf *dbuf, - unsigned long size, enum dma_data_direction dma_dir); void (*detach_dmabuf)(void *buf_priv); int (*map_dmabuf)(void *buf_priv);