[media] gspca_main: Locking fixes 2

Before this patch vidioc_dqbuf is using its own read_lock, where as
other queue related functions use queue_lock. This means that dqbuf is
accessing several variables in a racy manor. The most important one
being fr_o, which may be changed from underneath dqbuf by vidioc_reqbufs
or vidioc_streamoff. Other variables which it accesses unprotected
are gspca_dev->memory, gspca_dev->streaming and gspca_dev->capt_file.

This patch fixes this by changing vidioc_dqbuf to also use the queue_lock.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Jean-Francois Moine <moinejf@free.fr>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

2 files changed, 58 insertions, 55 deletions

diff --git a/drivers/media/video/gspca/gspca.c b/drivers/media/video/gspca/gspca.c
index 74626b6a9eb4..64ecf669d655 100644
--- a/drivers/media/video/gspca/gspca.c
+++ b/drivers/media/video/gspca/gspca.c
@@ -1831,33 +1831,77 @@ out:
         return ret;
 }
 
+static int frame_ready_nolock(struct gspca_dev *gspca_dev, struct file *file,
+                                enum v4l2_memory memory)
+{
+        if (!gspca_dev->present)
+                return -ENODEV;
+        if (gspca_dev->capt_file != file || gspca_dev->memory != memory ||
+                        !gspca_dev->streaming)
+                return -EINVAL;
+
+        /* check if a frame is ready */
+        return gspca_dev->fr_o != atomic_read(&gspca_dev->fr_i);
+}
+
+static int frame_ready(struct gspca_dev *gspca_dev, struct file *file,
+                        enum v4l2_memory memory)
+{
+        int ret;
+
+        if (mutex_lock_interruptible(&gspca_dev->queue_lock))
+                return -ERESTARTSYS;
+        ret = frame_ready_nolock(gspca_dev, file, memory);
+        mutex_unlock(&gspca_dev->queue_lock);
+        return ret;
+}
+
 /*
- * wait for a video frame
+ * dequeue a video buffer
  *
- * If a frame is ready, its index is returned.
+ * If nonblock_ing is false, block until a buffer is available.
  */
-static int frame_wait(struct gspca_dev *gspca_dev,
-                        int nonblock_ing)
+static int vidioc_dqbuf(struct file *file, void *priv,
+                        struct v4l2_buffer *v4l2_buf)
 {
-        int i, ret;
+        struct gspca_dev *gspca_dev = priv;
+        struct gspca_frame *frame;
+        int i, j, ret;
 
-        /* check if a frame is ready */
-        i = gspca_dev->fr_o;
-        if (i == atomic_read(&gspca_dev->fr_i)) {
-                if (nonblock_ing)
+        PDEBUG(D_FRAM, "dqbuf");
+
+        if (mutex_lock_interruptible(&gspca_dev->queue_lock))
+                return -ERESTARTSYS;
+
+        for (;;) {
+                ret = frame_ready_nolock(gspca_dev, file, v4l2_buf->memory);
+                if (ret < 0)
+                        goto out;
+                if (ret > 0)
+                        break;
+
+                mutex_unlock(&gspca_dev->queue_lock);
+
+                if (file->f_flags & O_NONBLOCK)
                         return -EAGAIN;
 
                 /* wait till a frame is ready */
                 ret = wait_event_interruptible_timeout(gspca_dev->wq,
-                        i != atomic_read(&gspca_dev->fr_i) ||
-                        !gspca_dev->streaming || !gspca_dev->present,
+                        frame_ready(gspca_dev, file, v4l2_buf->memory),
                         msecs_to_jiffies(3000));
                 if (ret < 0)
                         return ret;
-                if (ret == 0 || !gspca_dev->streaming || !gspca_dev->present)
+                if (ret == 0)
                         return -EIO;
+
+                if (mutex_lock_interruptible(&gspca_dev->queue_lock))
+                        return -ERESTARTSYS;
         }
 
+        i = gspca_dev->fr_o;
+        j = gspca_dev->fr_queue[i];
+        frame = &gspca_dev->frame[j];
+
         gspca_dev->fr_o = (i + 1) % GSPCA_MAX_FRAMES;
 
         if (gspca_dev->sd_desc->dq_callback) {
@@ -1867,46 +1911,7 @@ static int frame_wait(struct gspca_dev *gspca_dev,
                         gspca_dev->sd_desc->dq_callback(gspca_dev);
                 mutex_unlock(&gspca_dev->usb_lock);
         }
-        return gspca_dev->fr_queue[i];
-}
-
-/*
- * dequeue a video buffer
- *
- * If nonblock_ing is false, block until a buffer is available.
- */
-static int vidioc_dqbuf(struct file *file, void *priv,
-                        struct v4l2_buffer *v4l2_buf)
-{
-        struct gspca_dev *gspca_dev = priv;
-        struct gspca_frame *frame;
-        int i, ret;
-
-        PDEBUG(D_FRAM, "dqbuf");
-        if (v4l2_buf->memory != gspca_dev->memory)
-                return -EINVAL;
 
-        if (!gspca_dev->present)
-                return -ENODEV;
-
-        /* if not streaming, be sure the application will not loop forever */
-        if (!(file->f_flags & O_NONBLOCK)
-            && !gspca_dev->streaming && gspca_dev->users == 1)
-                return -EINVAL;
-
-        /* only the capturing file may dequeue */
-        if (gspca_dev->capt_file != file)
-                return -EINVAL;
-
-        /* only one dequeue / read at a time */
-        if (mutex_lock_interruptible(&gspca_dev->read_lock))
-                return -ERESTARTSYS;
-
-        ret = frame_wait(gspca_dev, file->f_flags & O_NONBLOCK);
-        if (ret < 0)
-                goto out;
-        i = ret;                                /* frame index */
-        frame = &gspca_dev->frame[i];
         if (gspca_dev->memory == V4L2_MEMORY_USERPTR) {
                 if (copy_to_user((__u8 __user *) frame->v4l2_buf.m.userptr,
                                  frame->data,
@@ -1919,10 +1924,10 @@ static int vidioc_dqbuf(struct file *file, void *priv,
         }
         frame->v4l2_buf.flags &= ~V4L2_BUF_FLAG_DONE;
         memcpy(v4l2_buf, &frame->v4l2_buf, sizeof *v4l2_buf);
-        PDEBUG(D_FRAM, "dqbuf %d", i);
+        PDEBUG(D_FRAM, "dqbuf %d", j);
         ret = 0;
 out:
-        mutex_unlock(&gspca_dev->read_lock);
+        mutex_unlock(&gspca_dev->queue_lock);
         return ret;
 }
 
@@ -2270,7 +2275,6 @@ int gspca_dev_probe2(struct usb_interface *intf,
                 goto out;
 
         mutex_init(&gspca_dev->usb_lock);
-        mutex_init(&gspca_dev->read_lock);
         mutex_init(&gspca_dev->queue_lock);
         init_waitqueue_head(&gspca_dev->wq);
 
diff --git a/drivers/media/video/gspca/gspca.h b/drivers/media/video/gspca/gspca.h
index 97b77a26a2eb..a2a1a6aa0606 100644
--- a/drivers/media/video/gspca/gspca.h
+++ b/drivers/media/video/gspca/gspca.h
@@ -205,7 +205,6 @@ struct gspca_dev {
 
         wait_queue_head_t wq;           /* wait queue */
         struct mutex usb_lock;          /* usb exchange protection */
-        struct mutex read_lock;         /* read protection */
         struct mutex queue_lock;        /* ISOC queue protection */
         int usb_err;                    /* USB error - protected by usb_lock */
         u16 pkt_size;                   /* ISOC packet size */