[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 */