On Mon Mar 16 11:58:22 2026 -0400, Sean Anderson wrote:
> If a UVC camera has an asynchronous control, uvc_status_stop may be
> called from async_ctrl.work:
>
> uvc_ctrl_status_event_work()
> uvc_ctrl_status_event()
> uvc_ctrl_clear_handle()
> uvc_pm_put()
> uvc_status_put()
> uvc_status_stop()
> cancel_work_sync()
>
> This will cause a deadlock, since cancel_work_sync will wait for
> uvc_ctrl_status_event_work to complete before returning.
>
> Fix this by returning early from uvc_status_stop if we are currently in
> the work function. flush_status now remains false until uvc_status_start
> is called again, ensuring that uvc_ctrl_status_event_work won't resubmit
> the URB.
>
> Fixes: a32d9c41bdb8 ("media: uvcvideo: Make power management granular")
> Cc: [email protected]
> Closes:
> https://lore.kernel.org/all/[email protected]/
> Signed-off-by: Sean Anderson <[email protected]>
> Link:
> https://patch.msgid.link/[email protected]
> Reviewed-by: Ricardo Ribalda <[email protected]>
> Tested-by: Ricardo Ribalda <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Signed-off-by: Hans de Goede <[email protected]>
> Signed-off-by: Hans Verkuil <[email protected]>
Patch committed.
Thanks,
Hans Verkuil
drivers/media/usb/uvc/uvc_status.c | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)
---
diff --git a/drivers/media/usb/uvc/uvc_status.c
b/drivers/media/usb/uvc/uvc_status.c
index 65f5356bebb3..b632cf5e3fe9 100644
--- a/drivers/media/usb/uvc/uvc_status.c
+++ b/drivers/media/usb/uvc/uvc_status.c
@@ -316,6 +316,16 @@ static int uvc_status_start(struct uvc_device *dev, gfp_t
flags)
if (!dev->int_urb)
return 0;
+ /*
+ * If the previous uvc_status_stop() call was from the async work,
+ * the work may still be running. Wait for it to finish before we submit
+ * the urb.
+ */
+ flush_work(&dev->async_ctrl.work);
+
+ /* Clear the flush status if we were previously stopped. */
+ smp_store_release(&dev->flush_status, false);
+
return usb_submit_urb(dev->int_urb, flags);
}
@@ -336,6 +346,15 @@ static void uvc_status_stop(struct uvc_device *dev)
*/
smp_store_release(&dev->flush_status, true);
+ /*
+ * If we are called from the event work function, the URB is guaranteed
+ * to not be in flight as it has completed and has not been resubmitted.
+ * There's no need to cancel the work (which would deadlock), or to kill
+ * the URB.
+ */
+ if (current_work() == &w->work)
+ return;
+
/*
* Cancel any pending asynchronous work. If any status event was queued,
* process it synchronously.
@@ -354,15 +373,6 @@ static void uvc_status_stop(struct uvc_device *dev)
*/
if (cancel_work_sync(&w->work))
uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
-
- /*
- * From this point, there are no events on the queue and the status URB
- * is dead. No events will be queued until uvc_status_start() is called.
- * The barrier is needed to make sure that flush_status is visible to
- * uvc_ctrl_status_event_work() when uvc_status_start() will be called
- * again.
- */
- smp_store_release(&dev->flush_status, false);
}
int uvc_status_resume(struct uvc_device *dev)
_______________________________________________
linuxtv-commits mailing list -- [email protected]
To unsubscribe send an email to [email protected]
