summary refs log tree commit diff
path: root/drivers/media/common/videobuf2/videobuf2-dma-sg.c
diff options
context:
space:
mode:
authorHans Verkuil <hverkuil@xs4all.nl>2019-04-04 09:15:00 -0400
committerMauro Carvalho Chehab <mchehab+samsung@kernel.org>2019-05-29 08:05:58 -0400
commit707947247e9517b94af5661b504467765edf16c3 (patch)
tree939a30cddd6d0fdaa9437c46ec1e0950abb6d9d0 /drivers/media/common/videobuf2/videobuf2-dma-sg.c
parent878344de61d0d5f351a1b84fce009a321be3eb45 (diff)
downloadlinux-707947247e9517b94af5661b504467765edf16c3.tar.gz
media: videobuf2-vmalloc: get_userptr: buffers are always writable
In vb2_vmalloc_get_userptr() the framevector is created with the
'write' argument set to false when vb2_create_framevec() is called
for OUTPUT buffers. So the pages are marked as read-only.

However, userspace will write to these buffers since it will fill
in the data to output. Since get_userptr is only called if the userptr
of the queued buffer has changed since the last time that same buffer
was queued, this will fail when the buffer contents is updated and the
buffer is queued again.

E.g., userspace fills buffer 1 with the output video and queues it.
The first time get_userptr is called and the pages are grabbed and
pinned in memory and marked read-only. The second time buffer 1 is
filled with different video data and queued again. Since the userptr
hasn't changed the get_userptr() callback isn't called again. Since
the pages were marked as read-only the new contents isn't updated.

Just always call vb2_create_framevec() with FOLL_WRITE to always
allow writing to the buffers.

Using USERPTR streaming with OUTPUT devices is almost never done. And
when it is done it is via v4l2-compliance and a driver like vim2m. But
since v4l2-compliance doesn't actually inspect the capture buffer and
compare it to the original output buffer, this issue was never noticed.

But the vicodec driver actually needs to parse the bitstream in the
OUTPUT buffers and any errors there will be immediately noticed. So
this time v4l2-compliance failed the USERPTR streaming test.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Diffstat (limited to 'drivers/media/common/videobuf2/videobuf2-dma-sg.c')
-rw-r--r--drivers/media/common/videobuf2/videobuf2-dma-sg.c3
1 files changed, 1 insertions, 2 deletions
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
index 0f06f08346ba..ed706b2a263c 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
@@ -239,8 +239,7 @@ static void *vb2_dma_sg_get_userptr(struct device *dev, unsigned long vaddr,
 	buf->offset = vaddr & ~PAGE_MASK;
 	buf->size = size;
 	buf->dma_sgt = &buf->sg_table;
-	vec = vb2_create_framevec(vaddr, size, dma_dir == DMA_FROM_DEVICE ||
-					       dma_dir == DMA_BIDIRECTIONAL);
+	vec = vb2_create_framevec(vaddr, size);
 	if (IS_ERR(vec))
 		goto userptr_fail_pfnvec;
 	buf->vec = vec;