linux-kernel - Re: [PATCH] v4l-compliance: re-introduce NON

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <YTtouz4IG/UPq84K@google.com>
Date:   Fri, 10 Sep 2021 23:16:27 +0900
From:   Sergey Senozhatsky <senozhatsky@...omium.org>
To:     Hans Verkuil <hverkuil-cisco@...all.nl>
Cc:     Tomasz Figa <tfiga@...omium.org>,
        Ricardo Ribalda <ribalda@...omium.org>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <senozhatsky@...omium.org>
Subject: Re: [PATCH] v4l-compliance: re-introduce NON_COHERENT and cache
 hints tests

On (21/09/10 22:31), Sergey Senozhatsky wrote:
> > Looking into it now. I ran v4l2-compliance, but not "contrib/test/test-media"
> 
> AFAICT the problem is in v4l2-compliance patch.
> 
> We clear request flags if queue does not support user-space cache hints:
> 
> 	 q->allow_cache_hints && q->memory == VB2_MEMORY_MMAP
> 
> But for DMABUF buffers (only) we set cache hints internally in
> set_buffer_cache_hints() and always skip cache sync/flush on
> prepare/finish regardless of what is passed from the user-space:
> 
>        if (q->memory == VB2_MEMORY_DMABUF) {
>                vb->skip_cache_sync_on_finish = 1;
>                vb->skip_cache_sync_on_prepare = 1;
>                return;
>        }
> 
> Technically we don't support user-space cache hints for DMABUF, so we
> clear passed user-space cache hint flags.
> 
> I think the fix should look like this (tested with "test-media vivid"):
> 
> ---
> 
> diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp
> index 9b87c90f..baa306f1 100644
> --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
> @@ -1865,9 +1865,10 @@ static int setupDmaBuf(struct node *expbuf_node, struct node *node,
>  				fail_on_test(!buf.g_bytesused(p));
>  		}
>  		flags = buf.g_flags();
> -		/* We always skip cache sync/flush for DMABUF memory type */
> -		fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE));
> -		fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN));
> +
> +		/* Make sure that flags are cleared */
> +		fail_on_test((flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE));
> +		fail_on_test((flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN));
>  		fail_on_test(flags & V4L2_BUF_FLAG_DONE);
>  		fail_on_test(buf.querybuf(node, i));
>  		fail_on_test(buf.check(q, Queued, i));


Alternatively, we can do something like below on the kernel side instead:
do nothing in v4l2 for DMABUF and preserve b->flags (if user-space has
passed cache management flags).

// But I think it'll be better to clear b->flags cache hints for DMABUF. To
// indicate that we don't accept cache-hints for DMABUF.


---

diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 6edf4508c636..fc8e31b7dced 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -345,6 +345,13 @@ static void set_buffer_cache_hints(struct vb2_queue *q,
 				   struct vb2_buffer *vb,
 				   struct v4l2_buffer *b)
 {
+	/*
+	 * No user-space cache hints for DMABUF, but we preserve b->flags
+	 * cache hints (if user-space has passed any).
+	 */
+	if (q->memory == VB2_MEMORY_DMABUF)
+		return;
+
 	if (!vb2_queue_allows_cache_hints(q)) {
 		/*
 		 * Clear buffer cache flags if queue does not support user