Skip to content

Commit 3659dea

Browse files
Sean Andersongregkh
authored andcommitted
media: uvcvideo: Fix deadlock if uvc_status_stop is called from async_ctrl.work
[ Upstream commit 6d27f92 ] 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: a32d9c4 ("media: uvcvideo: Make power management granular") Cc: stable@vger.kernel.org Closes: https://lore.kernel.org/all/6733bdfb-3e88-479f-8956-ab09c04c433e@linux.dev/ Signed-off-by: Sean Anderson <sean.anderson@linux.dev> Link: https://patch.msgid.link/20260316155823.1855434-1-sean.anderson@linux.dev Reviewed-by: Ricardo Ribalda <ribalda@chromium.org> Tested-by: Ricardo Ribalda <ribalda@chromium.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com> Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent c401492 commit 3659dea

1 file changed

Lines changed: 19 additions & 9 deletions

File tree

drivers/media/usb/uvc/uvc_status.c

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,16 @@ static int uvc_status_start(struct uvc_device *dev, gfp_t flags)
316316
if (!dev->int_urb)
317317
return 0;
318318

319+
/*
320+
* If the previous uvc_status_stop() call was from the async work,
321+
* the work may still be running. Wait for it to finish before we submit
322+
* the urb.
323+
*/
324+
flush_work(&dev->async_ctrl.work);
325+
326+
/* Clear the flush status if we were previously stopped. */
327+
smp_store_release(&dev->flush_status, false);
328+
319329
return usb_submit_urb(dev->int_urb, flags);
320330
}
321331

@@ -336,6 +346,15 @@ static void uvc_status_stop(struct uvc_device *dev)
336346
*/
337347
smp_store_release(&dev->flush_status, true);
338348

349+
/*
350+
* If we are called from the event work function, the URB is guaranteed
351+
* to not be in flight as it has completed and has not been resubmitted.
352+
* There's no need to cancel the work (which would deadlock), or to kill
353+
* the URB.
354+
*/
355+
if (current_work() == &w->work)
356+
return;
357+
339358
/*
340359
* Cancel any pending asynchronous work. If any status event was queued,
341360
* process it synchronously.
@@ -354,15 +373,6 @@ static void uvc_status_stop(struct uvc_device *dev)
354373
*/
355374
if (cancel_work_sync(&w->work))
356375
uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
357-
358-
/*
359-
* From this point, there are no events on the queue and the status URB
360-
* is dead. No events will be queued until uvc_status_start() is called.
361-
* The barrier is needed to make sure that flush_status is visible to
362-
* uvc_ctrl_status_event_work() when uvc_status_start() will be called
363-
* again.
364-
*/
365-
smp_store_release(&dev->flush_status, false);
366376
}
367377

368378
int uvc_status_resume(struct uvc_device *dev)

0 commit comments

Comments
 (0)