[media] soc-camera: properly fix camera probing races

The recently introduced host_lock causes lockdep warnings, besides, list
enumeration in scan_add_host() must be protected by holdint the list_lock.
OTOH, holding .video_lock in soc_camera_open() isn't enough to protect
the host during its building of the pipeline, because .video_lock is per
soc-camera device. If, e.g. more than one sensor can be attached to a host
and the user tries to open both device nodes simultaneously, host's .add()
method can be called simultaneously for both sensors. Fix these problems
by holding list_lock instead of .host_lock in scan_add_host() and taking
it shortly at the beginning of soc_camera_open(), and using .host_lock to
protect host's .add() and .remove() operations only.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
index a8ca956e7a40e10aaf4aec13eb9c25938a5fe080..1070a0c496d82818ead00106ae261634880dea88 100644 (file)
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -517,7 +517,14 @@ static int soc_camera_open(struct file *file)
                /* No device driver attached */
                return -ENODEV;
 
+       /*
+        * Don't mess with the host during probe: wait until the loop in
+        * scan_add_host() completes
+        */
+       if (mutex_lock_interruptible(&list_lock))
+               return -ERESTARTSYS;
        ici = to_soc_camera_host(icd->parent);
+       mutex_unlock(&list_lock);
 
        if (mutex_lock_interruptible(&icd->video_lock))
                return -ERESTARTSYS;
@@ -548,7 +555,6 @@ static int soc_camera_open(struct file *file)
                if (icl->reset)
                        icl->reset(icd->pdev);
 
-               /* Don't mess with the host during probe */
                mutex_lock(&ici->host_lock);
                ret = ici->ops->add(icd);
                mutex_unlock(&ici->host_lock);
@@ -602,7 +608,9 @@ esfmt:
 eresume:
        __soc_camera_power_off(icd);
 epower:
+       mutex_lock(&ici->host_lock);
        ici->ops->remove(icd);
+       mutex_unlock(&ici->host_lock);
 eiciadd:
        icd->use_count--;
        module_put(ici->ops->owner);
@@ -625,7 +633,9 @@ static int soc_camera_close(struct file *file)
 
                if (ici->ops->init_videobuf2)
                        vb2_queue_release(&icd->vb2_vidq);
+               mutex_lock(&ici->host_lock);
                ici->ops->remove(icd);
+               mutex_unlock(&ici->host_lock);
 
                __soc_camera_power_off(icd);
        }
@@ -1052,7 +1062,7 @@ static void scan_add_host(struct soc_camera_host *ici)
 {
        struct soc_camera_device *icd;
 
-       mutex_lock(&ici->host_lock);
+       mutex_lock(&list_lock);
 
        list_for_each_entry(icd, &devices, list) {
                if (icd->iface == ici->nr) {
@@ -1061,7 +1071,7 @@ static void scan_add_host(struct soc_camera_host *ici)
                }
        }
 
-       mutex_unlock(&ici->host_lock);
+       mutex_unlock(&list_lock);
 }
 
 #ifdef CONFIG_I2C_BOARDINFO
@@ -1148,7 +1158,9 @@ static int soc_camera_probe(struct soc_camera_device *icd)
        if (icl->reset)
                icl->reset(icd->pdev);
 
+       mutex_lock(&ici->host_lock);
        ret = ici->ops->add(icd);
+       mutex_unlock(&ici->host_lock);
        if (ret < 0)
                goto eadd;
 
@@ -1220,7 +1232,9 @@ static int soc_camera_probe(struct soc_camera_device *icd)
                icd->field              = mf.field;
        }
 
+       mutex_lock(&ici->host_lock);
        ici->ops->remove(icd);
+       mutex_unlock(&ici->host_lock);
 
        mutex_unlock(&icd->video_lock);
 
@@ -1242,7 +1256,9 @@ eadddev:
        video_device_release(icd->vdev);
        icd->vdev = NULL;
 evdc:
+       mutex_lock(&ici->host_lock);
        ici->ops->remove(icd);
+       mutex_unlock(&ici->host_lock);
 eadd:
 ereg:
        v4l2_ctrl_handler_free(&icd->ctrl_handler);

diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index 6442edc2a151db0d703b6ebb6f98338499603818..0370a95172820d08d39b5ae9f62be9ad10a78098 100644 (file)
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -62,7 +62,7 @@ struct soc_camera_device {
 struct soc_camera_host {
        struct v4l2_device v4l2_dev;
        struct list_head list;
-       struct mutex host_lock;         /* Protect during probing */
+       struct mutex host_lock;         /* Protect pipeline modifications */
        unsigned char nr;               /* Host number */
        u32 capabilities;
        void *priv;