[PATCH v4 3/3] media: uvcvideo: Do not use usb_* functions after .disconnect
From: Ricardo Ribalda
Date: Wed Nov 22 2023 - 05:20:02 EST
usb drivers should not call to any I/O function after the
.disconnect() callback has been triggered.
https://www.kernel.org/doc/html/latest/driver-api/usb/callbacks.html#the-disconnect-callback
If an application is receiving frames form a camera and the device is
disconnected: the device will call close() after the usb .disconnect()
callback has been called. The streamoff path will call usb_set_interface
or usb_clear_halt, which is not allowed.
This patch only solves the calls to close() *after* .disconnect() is
being called.
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 | 4 +++-
drivers/media/usb/uvc/uvc_status.c | 8 +++----
drivers/media/usb/uvc/uvc_v4l2.c | 2 +-
drivers/media/usb/uvc/uvc_video.c | 45 ++++++++++++++++++++++++--------------
drivers/media/usb/uvc/uvcvideo.h | 4 +++-
5 files changed, 39 insertions(+), 24 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index ded2cb6ce14f..d78640d422f4 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2266,6 +2266,8 @@ static void uvc_disconnect(struct usb_interface *intf)
return;
uvc_unregister_video(dev);
+ /* Barrier needed to pair with uvc_video_stop_streaming(). */
+ smp_store_release(&dev->disconnected, true);
kref_put(&dev->ref, uvc_delete);
}
@@ -2282,7 +2284,7 @@ static int uvc_suspend(struct usb_interface *intf, pm_message_t message)
UVC_SC_VIDEOCONTROL) {
mutex_lock(&dev->lock);
if (dev->users)
- uvc_status_stop(dev);
+ uvc_status_stop(dev, true);
mutex_unlock(&dev->lock);
return 0;
}
diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c
index 0208612a9f12..9c5da1244999 100644
--- a/drivers/media/usb/uvc/uvc_status.c
+++ b/drivers/media/usb/uvc/uvc_status.c
@@ -292,7 +292,7 @@ int uvc_status_init(struct uvc_device *dev)
void uvc_status_unregister(struct uvc_device *dev)
{
- uvc_status_stop(dev);
+ uvc_status_stop(dev, false);
uvc_input_unregister(dev);
}
@@ -310,7 +310,7 @@ int uvc_status_start(struct uvc_device *dev, gfp_t flags)
return usb_submit_urb(dev->int_urb, flags);
}
-void uvc_status_stop(struct uvc_device *dev)
+void uvc_status_stop(struct uvc_device *dev, bool run_async_work)
{
struct uvc_ctrl_work *w = &dev->async_ctrl;
@@ -326,7 +326,7 @@ void uvc_status_stop(struct uvc_device *dev)
* Cancel any pending asynchronous work. If any status event was queued,
* process it synchronously.
*/
- if (cancel_work_sync(&w->work))
+ if (cancel_work_sync(&w->work) && run_async_work)
uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
/* Kill the urb. */
@@ -338,7 +338,7 @@ void uvc_status_stop(struct uvc_device *dev)
* cancelled before returning or it could then race with a future
* uvc_status_start() call.
*/
- if (cancel_work_sync(&w->work))
+ if (cancel_work_sync(&w->work) && run_async_work)
uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
/*
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index f4988f03640a..f90206263ff4 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -672,7 +672,7 @@ static int uvc_v4l2_release(struct file *file)
mutex_lock(&stream->dev->lock);
if (--stream->dev->users == 0)
- uvc_status_stop(stream->dev);
+ uvc_status_stop(stream->dev, false);
mutex_unlock(&stream->dev->lock);
usb_autopm_put_interface(stream->dev->intf);
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 28dde08ec6c5..f5ef375088de 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 pair 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..5b1a3643de05 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;
@@ -745,7 +747,7 @@ int uvc_status_init(struct uvc_device *dev);
void uvc_status_unregister(struct uvc_device *dev);
void uvc_status_cleanup(struct uvc_device *dev);
int uvc_status_start(struct uvc_device *dev, gfp_t flags);
-void uvc_status_stop(struct uvc_device *dev);
+void uvc_status_stop(struct uvc_device *dev, bool run_async_work);
/* Controls */
extern const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited;
--
2.43.0.rc1.413.gea7ed67945-goog