[PATCH] V4L: Drop meaningless video_is_registered() call in v4l2_open()

[Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>]

As it currently stands this code doesn't protect against any races
between video device open() and its unregistration. Races could be
avoided by doing the video_is_registered() check protected by the
core mutex, while the video device unregistration is also done with
this mutex held.

The history of this code is that the second video_is_registered()
call has been added in commit ee6869afc922a9849979e49bb3bbcad7948
"V4L/DVB: v4l2: add core serialization lock" together with addition
of the core mutex support in fops:

        mutex_unlock(&videodev_lock);
-       if (vdev->fops->open)
-               ret = vdev->fops->open(filp);
+       if (vdev->fops->open) {
+               if (vdev->lock)
+                       mutex_lock(vdev->lock);
+               if (video_is_registered(vdev))
+                       ret = vdev->fops->open(filp);
+               else
+                       ret = -ENODEV;
+               if (vdev->lock)
+                       mutex_unlock(vdev->lock);
+       }

While commit cf5337358548b813479b58478539fc20ee86556c
"[media] v4l2-dev: remove V4L2_FL_LOCK_ALL_FOPS"
removed only code touching the mutex:

        mutex_unlock(&videodev_lock);
        if (vdev->fops->open) {
-               if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags) &&
-                   mutex_lock_interruptible(vdev->lock)) {
-                       ret = -ERESTARTSYS;
-                       goto err;
-               }
                if (video_is_registered(vdev))
                        ret = vdev->fops->open(filp);
                else
                        ret = -ENODEV;
-               if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
-                       mutex_unlock(vdev->lock);
        }

Remove the remaining video_is_registered() call as it doesn't provide
any real protection and just adds unnecessary overhead.

The drivers need to perform the unregistration check themselves inside
their file operation handlers, while holding respective mutex.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
---
 drivers/media/v4l2-core/v4l2-dev.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index c8859d6..1743119 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -444,13 +444,9 @@ static int v4l2_open(struct inode *inode, struct file *filp)
 	/* and increase the device refcount */
 	video_get(vdev);
 	mutex_unlock(&videodev_lock);
-	if (vdev->fops->open) {
-		if (video_is_registered(vdev))
-			ret = vdev->fops->open(filp);
-		else
-			ret = -ENODEV;
-	}

+	if (vdev->fops->open)
+		ret = vdev->fops->open(filp);
 	if (vdev->debug)
 		printk(KERN_DEBUG "%s: open (%d)\n",
 			video_device_node_name(vdev), ret);
--
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html