Commit 7d051b35 authored by Guennadi Liakhovetski's avatar Guennadi Liakhovetski Committed by Mauro Carvalho Chehab
Browse files

[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: default avatarGuennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@redhat.com>
parent daf16bab

drivers/media/platform/soc_camera/soc_camera.c

+19 −3
Original line number Diff line number Diff line
@@ -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);

include/media/soc_camera.h

+1 −1
Original line number Diff line number Diff line
@@ -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;

CRA Git | Maintained and supported by SUSTech CRA and CCSE