| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 10 Jan 2020 10:55:53 +0100
From: Hans Verkuil <hverkuil@...all.nl>
To: Sergey Senozhatsky <senozhatsky@...omium.org>,
Hans Verkuil <hans.verkuil@...co.com>,
Tomasz Figa <tfiga@...omium.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Kyungmin Park <kyungmin.park@...sung.com>,
Marek Szyprowski <m.szyprowski@...sung.com>
Cc: Sakari Ailus <sakari.ailus@....fi>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Pawel Osciak <posciak@...omium.org>,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC][PATCH 05/15] videobuf2: handle
V4L2_FLAG_MEMORY_NON_CONSISTENT in REQBUFS
On 12/17/19 4:20 AM, Sergey Senozhatsky wrote:
> This patch lets user-space to request a non-consistent memory
> allocation during REQBUFS ioctl call. We use one bit of a
> ->reserved[1] member of struct v4l2_requestbuffers, which is
> now renamed to ->flags.
>
> There is just 1 four-byte reserved area in v4l2_requestbuffers
> struct, therefore for backward compatibility ->reserved and
> ->flags were put into anonymous union.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@...omium.org>
> ---
> Documentation/media/uapi/v4l/vidioc-reqbufs.rst | 14 ++++++++++++--
> drivers/media/common/videobuf2/videobuf2-v4l2.c | 14 ++++++++++++--
> drivers/media/v4l2-core/v4l2-ioctl.c | 3 ---
> include/uapi/linux/videodev2.h | 5 ++++-
> 4 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> index d0c643db477a..9b69a61d9fd4 100644
> --- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> @@ -112,10 +112,20 @@ aborting or finishing any DMA in progress, an implicit
> ``V4L2_MEMORY_MMAP`` and ``type`` set to the buffer type. This will
> free any previously allocated buffers, so this is typically something
> that will be done at the start of the application.
> - * - __u32
> + * - union
> + - (anonymous)
> + * -
> + - __u32
> + - ``flags``\ [1]
> + - Specifies additional buffer management attributes. E.g. when
> + ``V4L2_FLAG_MEMORY_NON_CONSISTENT`` set vb2 backends may be allocated
> + in non-consistent memory.
This should link to the table with these memory flags, rather than
effectively documenting V4L2_FLAG_MEMORY_NON_CONSISTENT again.
You also probably meant "vb2 buffers" rather than "vb2 backends".
> + * -
> + - __u32
> - ``reserved``\ [1]
> - A place holder for future extensions. Drivers and applications
> - must set the array to zero.
> + must set the array to zero, unless application wants to specify
> + buffer management ``flags``.
I think support for this flag should be signaled as a V4L2_BUF_CAP capability.
If the capability is not set, then vb2 should set 'flags' to 0 to preserve the
old 'Drivers and applications must set the array to zero' behavior.
The documentation for 'reserved[1]' should be changed to something like:
Kept for backwards compatibility. Use ``flags`` instead.
Regards,
Hans
>
> .. tabularcolumns:: |p{6.1cm}|p{2.2cm}|p{8.7cm}|
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index f1e88c9398c7..0eabb589684f 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -693,9 +693,15 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
> int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
> {
> int ret = vb2_verify_memory_type(q, req->memory, req->type);
> + bool consistent = true;
> +
> + if (req->flags & V4L2_FLAG_MEMORY_NON_CONSISTENT)
> + consistent = false;
>
> fill_buf_caps(q, &req->capabilities);
> - return ret ? ret : vb2_core_reqbufs(q, req->memory, true, &req->count);
> + if (ret)
> + return ret;
> + return vb2_core_reqbufs(q, req->memory, consistent, &req->count);
> }
> EXPORT_SYMBOL_GPL(vb2_reqbufs);
>
> @@ -939,13 +945,17 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv,
> {
> struct video_device *vdev = video_devdata(file);
> int res = vb2_verify_memory_type(vdev->queue, p->memory, p->type);
> + bool consistent = true;
>
> fill_buf_caps(vdev->queue, &p->capabilities);
> if (res)
> return res;
> if (vb2_queue_is_busy(vdev, file))
> return -EBUSY;
> - res = vb2_core_reqbufs(vdev->queue, p->memory, true, &p->count);
> + if (p->flags & V4L2_FLAG_MEMORY_NON_CONSISTENT)
> + consistent = false;
> +
> + res = vb2_core_reqbufs(vdev->queue, p->memory, consistent, &p->count);
> /* If count == 0, then the owner has released all buffers and he
> is no longer owner of the queue. Otherwise we have a new owner. */
> if (res == 0)
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 003b7422aeef..225d06819bce 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1973,9 +1973,6 @@ static int v4l_reqbufs(const struct v4l2_ioctl_ops *ops,
>
> if (ret)
> return ret;
> -
> - CLEAR_AFTER_FIELD(p, capabilities);
> -
> return ops->vidioc_reqbufs(file, fh, p);
> }
>
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index d352997f2b62..73a4854f71bd 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -919,7 +919,10 @@ struct v4l2_requestbuffers {
> __u32 type; /* enum v4l2_buf_type */
> __u32 memory; /* enum v4l2_memory */
> __u32 capabilities;
> - __u32 reserved[1];
> + union {
> + __u32 flags;
> + __u32 reserved[1];
> + };
> };
>
> /* capabilities for struct v4l2_requestbuffers and v4l2_create_buffers */
>
Powered by blists - more mailing lists