linux-kernel - [PATCH v6 1/4] media: uvcvideo: Stop stream during unregister

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240614-guenter-mini-v6-1-7b7fdc3b21b3@chromium.org>
Date: Fri, 14 Jun 2024 12:41:27 +0000
From: Ricardo Ribalda <ribalda@...omium.org>
To: Mauro Carvalho Chehab <mchehab@...nel.org>
Cc: Guenter Roeck <linux@...ck-us.net>, Tomasz Figa <tfiga@...omium.org>, 
 Laurent Pinchart <laurent.pinchart@...asonboard.com>, 
 Alan Stern <stern@...land.harvard.edu>, 
 Hans Verkuil <hverkuil-cisco@...all.nl>, linux-media@...r.kernel.org, 
 linux-kernel@...r.kernel.org, Sean Paul <seanpaul@...omium.org>, 
 Ricardo Ribalda <ribalda@...omium.org>, 
 Sakari Ailus <sakari.ailus@...ux.intel.com>
Subject: [PATCH v6 1/4] media: uvcvideo: Stop stream during unregister

uvc_unregister_video() can be called asynchronously from
uvc_disconnect(). If the device is still streaming when that happens, a
plethora of race conditions can occur.

Make sure that the device has stopped streaming before exiting this
function.

If the user still holds handles to the driver's file descriptors, any
ioctl will return -ENODEV from the v4l2 core.

This change makes uvc more consistent with the rest of the v4l2 drivers
using the vb2_fop_* and vb2_ioctl_* helpers.

Reviewed-by: Hans Verkuil <hverkuil-cisco@...all.nl>
Suggested-by: Hans Verkuil <hverkuil-cisco@...all.nl>
Signed-off-by: Ricardo Ribalda <ribalda@...omium.org>
---
 drivers/media/usb/uvc/uvc_driver.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index bbd90123a4e7..55132688e363 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1908,11 +1908,41 @@ static void uvc_unregister_video(struct uvc_device *dev)
 	struct uvc_streaming *stream;
 
 	list_for_each_entry(stream, &dev->streams, list) {
+		/* Nothing to do here, continue. */
 		if (!video_is_registered(&stream->vdev))
 			continue;
 
+		/*
+		 * For stream->vdev we follow the same logic as:
+		 * vb2_video_unregister_device().
+		 */
+
+		/* 1. Take a reference to vdev */
+		get_device(&stream->vdev.dev);
+
+		/* 2. Ensure that no new ioctls can be called. */
 		video_unregister_device(&stream->vdev);
-		video_unregister_device(&stream->meta.vdev);
+
+		/* 3. Wait for old ioctls to finish. */
+		mutex_lock(&stream->mutex);
+
+		/* 4. Stop streaming. */
+		uvc_queue_release(&stream->queue);
+
+		mutex_unlock(&stream->mutex);
+
+		put_device(&stream->vdev.dev);
+
+		/*
+		 * For stream->meta.vdev we can directly call:
+		 * vb2_video_unregister_device().
+		 */
+		vb2_video_unregister_device(&stream->meta.vdev);
+
+		/*
+		 * Now both vdevs are not streaming and all the ioctls will
+		 * return -ENODEV.
+		 */
 
 		uvc_debugfs_cleanup_stream(stream);
 	}

-- 
2.45.2.627.g7a2c4fd464-goog