media: vb2: add waiting_in_dqbuf flag

Calling VIDIOC_DQBUF can release the core serialization lock pointed to
by vb2_queue->lock if it has to wait for a new buffer to arrive.

However, if userspace dup()ped the video device filehandle, then it is
possible to read or call DQBUF from two filehandles at the same time.

It is also possible to call REQBUFS from one filehandle while the other
is waiting for a buffer. This will remove all the buffers and reallocate
new ones. Removing all the buffers isn't the problem here (that's already
handled correctly by DQBUF), but the reallocating part is: DQBUF isn't
aware that the buffers have changed.

This is fixed by setting a flag whenever the lock is released while waiting
for a buffer to arrive. And checking the flag where needed so we can return
-EBUSY.

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
Reported-by: Syzbot <syzbot+4180ff9ca6810b06c1e9@syzkaller.appspotmail.com>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index b98ec6e..7ebd58a 100644 (file)
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -672,6 +672,11 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
                return -EBUSY;
        }
 
+       if (q->waiting_in_dqbuf && *count) {
+               dprintk(1, "another dup()ped fd is waiting for a buffer\n");
+               return -EBUSY;
+       }
+
        if (*count == 0 || q->num_buffers != 0 ||
            (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) {
                /*
@@ -807,6 +812,10 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
        }
 
        if (!q->num_buffers) {
+               if (q->waiting_in_dqbuf && *count) {
+                       dprintk(1, "another dup()ped fd is waiting for a buffer\n");
+                       return -EBUSY;
+               }
                memset(q->alloc_devs, 0, sizeof(q->alloc_devs));
                q->memory = memory;
                q->waiting_for_buffers = !q->is_output;
@@ -1656,6 +1665,11 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
        for (;;) {
                int ret;
 
+               if (q->waiting_in_dqbuf) {
+                       dprintk(1, "another dup()ped fd is waiting for a buffer\n");
+                       return -EBUSY;
+               }
+
                if (!q->streaming) {
                        dprintk(1, "streaming off, will not wait for buffers\n");
                        return -EINVAL;
@@ -1683,6 +1697,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
                        return -EAGAIN;
                }
 
+               q->waiting_in_dqbuf = 1;
                /*
                 * We are streaming and blocking, wait for another buffer to
                 * become ready or for streamoff. Driver's lock is released to
@@ -1703,6 +1718,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
                 * the locks or return an error if one occurred.
                 */
                call_void_qop(q, wait_finish, q);
+               q->waiting_in_dqbuf = 0;
                if (ret) {
                        dprintk(1, "sleep was interrupted\n");
                        return ret;
@@ -2585,6 +2601,12 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
        if (!data)
                return -EINVAL;
 
+       if (q->waiting_in_dqbuf) {
+               dprintk(3, "another dup()ped fd is %s\n",
+                       read ? "reading" : "writing");
+               return -EBUSY;
+       }
+
        /*
         * Initialize emulator on first call.
         */

diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index fe010ad..22f3ff7 100644 (file)
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -596,6 +596,7 @@ struct vb2_queue {
        unsigned int                    start_streaming_called:1;
        unsigned int                    error:1;
        unsigned int                    waiting_for_buffers:1;
+       unsigned int                    waiting_in_dqbuf:1;
        unsigned int                    is_multiplanar:1;
        unsigned int                    is_output:1;
        unsigned int                    copy_timestamp:1;