[PATCH v3 2/3] media: uvcvideo: Do not halt the device after disconnect

From: Ricardo Ribalda
Date: Tue Nov 21 2023 - 14:53:58 EST


usb drivers should not call to any usb_() function after the
.disconnect() callback has been triggered.

If the camera is streaming, the uvc driver will call usb_set_interface or
usb_clear_halt once the device is being released. Let's fix this issue.

This is probably not the only driver affected with this kind of bug, but
until there is a better way to do it in the core this is the way to
solve this issue.

When/if a different mechanism is implemented in the core to solve the
lifetime of devices we will adopt it in uvc.

Trace:
[ 1065.389723] drivers/media/usb/uvc/uvc_driver.c:2248 uvc_disconnect enter
[ 1065.390160] drivers/media/usb/uvc/uvc_driver.c:2264 uvc_disconnect exit
[ 1065.433956] drivers/media/usb/uvc/uvc_v4l2.c:659 uvc_v4l2_release enter
[ 1065.433973] drivers/media/usb/uvc/uvc_video.c:2274 uvc_video_stop_streaming enter
[ 1065.434560] drivers/media/usb/uvc/uvc_video.c:2285 uvc_video_stop_streaming exit
[ 1065.435154] drivers/media/usb/uvc/uvc_v4l2.c:680 uvc_v4l2_release exit
[ 1065.435188] drivers/media/usb/uvc/uvc_driver.c:2248 uvc_disconnect enter

Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
---
drivers/media/usb/uvc/uvc_driver.c | 2 ++
drivers/media/usb/uvc/uvc_video.c | 45 ++++++++++++++++++++++++--------------
drivers/media/usb/uvc/uvcvideo.h | 2 ++
3 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 08fcd2ffa727..413c32867617 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2257,6 +2257,8 @@ static void uvc_disconnect(struct usb_interface *intf)
return;

uvc_unregister_video(dev);
+ /* Barrier needed to synchronize with uvc_video_stop_streaming(). */
+ smp_store_release(&dev->disconnected, true);
kref_put(&dev->ref, uvc_delete);
}

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 28dde08ec6c5..032b44e45b22 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -2243,28 +2243,39 @@ int uvc_video_start_streaming(struct uvc_streaming *stream)
return ret;
}

-void uvc_video_stop_streaming(struct uvc_streaming *stream)
+static void uvc_video_halt(struct uvc_streaming *stream)
{
- uvc_video_stop_transfer(stream, 1);
+ unsigned int epnum;
+ unsigned int pipe;
+ unsigned int dir;

if (stream->intf->num_altsetting > 1) {
usb_set_interface(stream->dev->udev, stream->intfnum, 0);
- } else {
- /*
- * UVC doesn't specify how to inform a bulk-based device
- * when the video stream is stopped. Windows sends a
- * CLEAR_FEATURE(HALT) request to the video streaming
- * bulk endpoint, mimic the same behaviour.
- */
- unsigned int epnum = stream->header.bEndpointAddress
- & USB_ENDPOINT_NUMBER_MASK;
- unsigned int dir = stream->header.bEndpointAddress
- & USB_ENDPOINT_DIR_MASK;
- unsigned int pipe;
-
- pipe = usb_sndbulkpipe(stream->dev->udev, epnum) | dir;
- usb_clear_halt(stream->dev->udev, pipe);
+ return;
}

+ /*
+ * UVC doesn't specify how to inform a bulk-based device
+ * when the video stream is stopped. Windows sends a
+ * CLEAR_FEATURE(HALT) request to the video streaming
+ * bulk endpoint, mimic the same behaviour.
+ */
+ epnum = stream->header.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK;
+ dir = stream->header.bEndpointAddress & USB_ENDPOINT_DIR_MASK;
+ pipe = usb_sndbulkpipe(stream->dev->udev, epnum) | dir;
+ usb_clear_halt(stream->dev->udev, pipe);
+}
+
+void uvc_video_stop_streaming(struct uvc_streaming *stream)
+{
+ uvc_video_stop_transfer(stream, 1);
+
+ /*
+ * Barrier needed to synchronize with uvc_disconnect().
+ * We cannot call usb_* functions on a disconnected USB device.
+ */
+ if (!smp_load_acquire(&stream->dev->disconnected))
+ uvc_video_halt(stream);
+
uvc_video_clock_cleanup(stream);
}
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 6fb0a78b1b00..4318ce8e31db 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -559,6 +559,8 @@ struct uvc_device {
unsigned int users;
atomic_t nmappings;

+ bool disconnected;
+
/* Video control interface */
#ifdef CONFIG_MEDIA_CONTROLLER
struct media_device mdev;

--
2.43.0.rc1.413.gea7ed67945-goog