From 7d051b35d5196ad6011a17e751dbd3d180abb046 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Thu, 20 Dec 2012 13:02:51 -0300 Subject: [PATCH] [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 Signed-off-by: Mauro Carvalho Chehab --- .../media/platform/soc_camera/soc_camera.c | 22 ++++++++++++++++--- include/media/soc_camera.h | 2 +- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c index a8ca956e7a40..1070a0c496d8 100644 --- 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 6442edc2a151..0370a9517282 100644 --- 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;