media: videobuf2: revert "get_userptr: buffers are always writable"

Commit 707947247e95 ("media: videobuf2-vmalloc: get_userptr: buffers are
always writable") caused problems in a corner case (passing read-only
shmem memory as a userptr). So revert this patch.

The original problem for which that commit was originally made is
something that I could not reproduce after reverting it. So just go
back to the way it was for many years, and if problems arise in
the future, then another approach should be taken to resolve it.

This patch is based on a patch from Hirokazu.

Fixes: 707947247e95 ("media: videobuf2-vmalloc: get_userptr: buffers are always writable")
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Acked-by: Tomasz Figa <tfiga@chromium.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

diff --git a/drivers/media/common/videobuf2/frame_vector.c b/drivers/media/common/videobuf2/frame_vector.c
index 542dde9d2609be07700f6cf63d46f7a9b4557444..aad72640f055dca8a75dbd9ca2062fee91e6dc12 100644 (file)
--- a/drivers/media/common/videobuf2/frame_vector.c
+++ b/drivers/media/common/videobuf2/frame_vector.c
@@ -14,6 +14,7 @@
  * get_vaddr_frames() - map virtual addresses to pfns
  * @start:     starting user address
  * @nr_frames: number of pages / pfns from start to map
+ * @write:     the mapped address has write permission
  * @vec:       structure which receives pages / pfns of the addresses mapped.
  *             It should have space for at least nr_frames entries.
  *
@@ -32,7 +33,7 @@
  *
  * This function takes care of grabbing mmap_lock as necessary.
  */
-int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
+int get_vaddr_frames(unsigned long start, unsigned int nr_frames, bool write,
                     struct frame_vector *vec)
 {
        struct mm_struct *mm = current->mm;
@@ -40,6 +41,7 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
        int ret_pin_user_pages_fast = 0;
        int ret = 0;
        int err;
+       unsigned int gup_flags = FOLL_FORCE | FOLL_LONGTERM;
 
        if (nr_frames == 0)
                return 0;
@@ -49,8 +51,10 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
 
        start = untagged_addr(start);
 
-       ret = pin_user_pages_fast(start, nr_frames,
-                                 FOLL_FORCE | FOLL_WRITE | FOLL_LONGTERM,
+       if (write)
+               gup_flags |= FOLL_WRITE;
+
+       ret = pin_user_pages_fast(start, nr_frames, gup_flags,
                                  (struct page **)(vec->ptrs));
        if (ret > 0) {
                vec->got_ref = true;

diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
index 678b359717c4857d17d2bd8870fe9cda483f1a88..8e55468cb60d1443f04f436eb97d29e22eb6a6ae 100644 (file)
--- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
@@ -603,7 +603,8 @@ static void *vb2_dc_get_userptr(struct vb2_buffer *vb, struct device *dev,
        buf->vb = vb;
 
        offset = lower_32_bits(offset_in_page(vaddr));
-       vec = vb2_create_framevec(vaddr, size);
+       vec = vb2_create_framevec(vaddr, size, buf->dma_dir == DMA_FROM_DEVICE ||
+                                              buf->dma_dir == DMA_BIDIRECTIONAL);
        if (IS_ERR(vec)) {
                ret = PTR_ERR(vec);
                goto fail_buf;

diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
index fa69158a65b1fd2311946e7bec189e1bf4b6e35a..099693e42bc68fe1eb67ab623b982755ac86ed83 100644 (file)
--- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
@@ -241,7 +241,9 @@ static void *vb2_dma_sg_get_userptr(struct vb2_buffer *vb, struct device *dev,
        buf->size = size;
        buf->dma_sgt = &buf->sg_table;
        buf->vb = vb;
-       vec = vb2_create_framevec(vaddr, size);
+       vec = vb2_create_framevec(vaddr, size,
+                                 buf->dma_dir == DMA_FROM_DEVICE ||
+                                 buf->dma_dir == DMA_BIDIRECTIONAL);
        if (IS_ERR(vec))
                goto userptr_fail_pfnvec;
        buf->vec = vec;

diff --git a/drivers/media/common/videobuf2/videobuf2-memops.c b/drivers/media/common/videobuf2/videobuf2-memops.c
index 9dd6c27162f433e1a279893267f54cd72f497046..f9a4ec44422eb36ffe9c1a9758b7d71d8c9dcb9a 100644 (file)
--- a/drivers/media/common/videobuf2/videobuf2-memops.c
+++ b/drivers/media/common/videobuf2/videobuf2-memops.c
@@ -26,6 +26,7 @@
  * vb2_create_framevec() - map virtual addresses to pfns
  * @start:     Virtual user address where we start mapping
  * @length:    Length of a range to map
+ * @write:     Should we map for writing into the area
  *
  * This function allocates and fills in a vector with pfns corresponding to
  * virtual address range passed in arguments. If pfns have corresponding pages,
@@ -34,7 +35,8 @@
  * failure. Returned vector needs to be freed via vb2_destroy_pfnvec().
  */
 struct frame_vector *vb2_create_framevec(unsigned long start,
-                                        unsigned long length)
+                                        unsigned long length,
+                                        bool write)
 {
        int ret;
        unsigned long first, last;
@@ -47,7 +49,7 @@ struct frame_vector *vb2_create_framevec(unsigned long start,
        vec = frame_vector_create(nr);
        if (!vec)
                return ERR_PTR(-ENOMEM);
-       ret = get_vaddr_frames(start & PAGE_MASK, nr, vec);
+       ret = get_vaddr_frames(start & PAGE_MASK, nr, write, vec);
        if (ret < 0)
                goto out_destroy;
        /* We accept only complete set of PFNs */

diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
index 948152f1596b496d0286c9ccf02e7a8d02309be9..67d0b89e701b5efe5a71b9c30406d02b0bfa4c33 100644 (file)
--- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c
+++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
@@ -85,7 +85,9 @@ static void *vb2_vmalloc_get_userptr(struct vb2_buffer *vb, struct device *dev,
        buf->dma_dir = vb->vb2_queue->dma_dir;
        offset = vaddr & ~PAGE_MASK;
        buf->size = size;
-       vec = vb2_create_framevec(vaddr, size);
+       vec = vb2_create_framevec(vaddr, size,
+                                 buf->dma_dir == DMA_FROM_DEVICE ||
+                                 buf->dma_dir == DMA_BIDIRECTIONAL);
        if (IS_ERR(vec)) {
                ret = PTR_ERR(vec);
                goto fail_pfnvec_create;

diff --git a/include/media/frame_vector.h b/include/media/frame_vector.h
index bfed1710dc2485f0546f93c094d5b297ef73085c..541c71a2c7be1f85eae3764f95a0116018ac0d8a 100644 (file)
--- a/include/media/frame_vector.h
+++ b/include/media/frame_vector.h
@@ -16,7 +16,7 @@ struct frame_vector {
 struct frame_vector *frame_vector_create(unsigned int nr_frames);
 void frame_vector_destroy(struct frame_vector *vec);
 int get_vaddr_frames(unsigned long start, unsigned int nr_pfns,
-                    struct frame_vector *vec);
+                    bool write, struct frame_vector *vec);
 void put_vaddr_frames(struct frame_vector *vec);
 int frame_vector_to_pages(struct frame_vector *vec);
 void frame_vector_to_pfns(struct frame_vector *vec);

diff --git a/include/media/videobuf2-memops.h b/include/media/videobuf2-memops.h
index cd4a463315313410a75b98b2a567d5f75925824e..4b5b84f93538ed4cc808dbc6cfdf18a448a5abab 100644 (file)
--- a/include/media/videobuf2-memops.h
+++ b/include/media/videobuf2-memops.h
@@ -34,7 +34,8 @@ struct vb2_vmarea_handler {
 extern const struct vm_operations_struct vb2_common_vm_ops;
 
 struct frame_vector *vb2_create_framevec(unsigned long start,
-                                        unsigned long length);
+                                        unsigned long length,
+                                        bool write);
 void vb2_destroy_framevec(struct frame_vector *vec);
 
 #endif