diff mbox series

media: v4l2-core: hold videodev_lock until dev reg, finishes

Message ID 50f691d3-f49b-4279-9048-48319afd86f9@xs4all.nl
State Accepted
Commit 1ed4477f2ea4743e7c5e1f9f3722152d14e6eeb1
Headers show
Series media: v4l2-core: hold videodev_lock until dev reg, finishes | expand

Commit Message

Hans Verkuil Feb. 23, 2024, 8:45 a.m. UTC
After the new V4L2 device node was registered, some additional
initialization was done before the device node was marked as
'registered'. During the time between creating the device node
and marking it as 'registered' it was possible to open the
device node, which would return -ENODEV since the 'registered'
flag was not yet set.

Hold the videodev_lock mutex from just before the device node
is registered until the 'registered' flag is set. Since v4l2_open
will take the same lock, it will wait until this registration
process is finished. This resolves this race condition.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/v4l2-dev.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Sakari Ailus April 9, 2024, 8:47 a.m. UTC | #1
Hi Hans,

On Fri, Feb 23, 2024 at 09:45:36AM +0100, Hans Verkuil wrote:
> After the new V4L2 device node was registered, some additional
> initialization was done before the device node was marked as
> 'registered'. During the time between creating the device node
> and marking it as 'registered' it was possible to open the
> device node, which would return -ENODEV since the 'registered'
> flag was not yet set.
> 
> Hold the videodev_lock mutex from just before the device node
> is registered until the 'registered' flag is set. Since v4l2_open
> will take the same lock, it will wait until this registration
> process is finished. This resolves this race condition.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Thanks for the patch.

This seems worth a Fixes: tag if not cc'ing to stable.

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

> ---
>  drivers/media/v4l2-core/v4l2-dev.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index d13954bd31fd..bae73b8c52ff 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -1036,8 +1036,10 @@ int __video_register_device(struct video_device *vdev,
>  	vdev->dev.devt = MKDEV(VIDEO_MAJOR, vdev->minor);
>  	vdev->dev.parent = vdev->dev_parent;
>  	dev_set_name(&vdev->dev, "%s%d", name_base, vdev->num);
> +	mutex_lock(&videodev_lock);
>  	ret = device_register(&vdev->dev);
>  	if (ret < 0) {
> +		mutex_unlock(&videodev_lock);
>  		pr_err("%s: device_register failed\n", __func__);
>  		goto cleanup;
>  	}
> @@ -1057,6 +1059,7 @@ int __video_register_device(struct video_device *vdev,
> 
>  	/* Part 6: Activate this minor. The char device can now be used. */
>  	set_bit(V4L2_FL_REGISTERED, &vdev->flags);
> +	mutex_unlock(&videodev_lock);
> 
>  	return 0;
>
diff mbox series

Patch

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index d13954bd31fd..bae73b8c52ff 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -1036,8 +1036,10 @@  int __video_register_device(struct video_device *vdev,
 	vdev->dev.devt = MKDEV(VIDEO_MAJOR, vdev->minor);
 	vdev->dev.parent = vdev->dev_parent;
 	dev_set_name(&vdev->dev, "%s%d", name_base, vdev->num);
+	mutex_lock(&videodev_lock);
 	ret = device_register(&vdev->dev);
 	if (ret < 0) {
+		mutex_unlock(&videodev_lock);
 		pr_err("%s: device_register failed\n", __func__);
 		goto cleanup;
 	}
@@ -1057,6 +1059,7 @@  int __video_register_device(struct video_device *vdev,

 	/* Part 6: Activate this minor. The char device can now be used. */
 	set_bit(V4L2_FL_REGISTERED, &vdev->flags);
+	mutex_unlock(&videodev_lock);

 	return 0;