The __prepare_userptr() function made the incorrect assumption that if the
same user pointer was used as the last one for which memory was acquired, then
there was no need to re-acquire the memory. This assumption was never properly
tested, and after doing that it became clear that this was in fact wrong.

Change the behavior to always reacquire memory.

Signed-off-by: Hans Verkuil <hverkuil-ci...@xs4all.nl>
Reported-by: Tomasz Figa <tf...@chromium.org>
Cc: <sta...@vger.kernel.org>      # for v5.1 and up
---
This should be backported to all stable kernels, unfortunately this patch only
applies cleanly to 5.1, so this has to be backported manually.
---
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c 
b/drivers/media/common/videobuf2/videobuf2-core.c
index 4489744fbbd9..a6400391117f 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -1013,7 +1013,7 @@ static int __prepare_userptr(struct vb2_buffer *vb)
        void *mem_priv;
        unsigned int plane;
        int ret = 0;
-       bool reacquired = vb->planes[0].mem_priv == NULL;
+       bool called_cleanup = false;

        memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
        /* Copy relevant information provided by the userspace */
@@ -1023,15 +1023,6 @@ static int __prepare_userptr(struct vb2_buffer *vb)
                return ret;

        for (plane = 0; plane < vb->num_planes; ++plane) {
-               /* Skip the plane if already verified */
-               if (vb->planes[plane].m.userptr &&
-                       vb->planes[plane].m.userptr == planes[plane].m.userptr
-                       && vb->planes[plane].length == planes[plane].length)
-                       continue;
-
-               dprintk(3, "userspace address for plane %d changed, reacquiring 
memory\n",
-                       plane);
-
                /* Check if the provided plane buffer is large enough */
                if (planes[plane].length < vb->planes[plane].min_length) {
                        dprintk(1, "provided buffer size %u is less than setup 
size %u for plane %d\n",
@@ -1044,8 +1035,8 @@ static int __prepare_userptr(struct vb2_buffer *vb)

                /* Release previously acquired memory if present */
                if (vb->planes[plane].mem_priv) {
-                       if (!reacquired) {
-                               reacquired = true;
+                       if (!called_cleanup) {
+                               called_cleanup = true;
                                vb->copied_timestamp = 0;
                                call_void_vb_qop(vb, buf_cleanup, vb);
                        }
@@ -1083,17 +1074,14 @@ static int __prepare_userptr(struct vb2_buffer *vb)
                vb->planes[plane].data_offset = planes[plane].data_offset;
        }

-       if (reacquired) {
-               /*
-                * One or more planes changed, so we must call buf_init to do
-                * the driver-specific initialization on the newly acquired
-                * buffer, if provided.
-                */
-               ret = call_vb_qop(vb, buf_init, vb);
-               if (ret) {
-                       dprintk(1, "buffer initialization failed\n");
-                       goto err;
-               }
+       /*
+        * Call buf_init to do the driver-specific initialization on
+        * the newly acquired buffer.
+        */
+       ret = call_vb_qop(vb, buf_init, vb);
+       if (ret) {
+               dprintk(1, "buffer initialization failed\n");
+               goto err;
        }

        ret = call_vb_qop(vb, buf_prepare, vb);

Reply via email to