usb_kill_urb warranties that all the handlers are finished when it returns, but does not protect against threads that might be handling asynchronously the urb.
For UVC, the function uvc_ctrl_status_event_async() takes care of control changes. If the code is executed in the following order:
CPU 0 CPU 1 ===== ===== uvc_status_complete() uvc_status_stop() uvc_ctrl_status_event_work() uvc_status_start() -> FAIL
Then uvc_status_start will keep failing and this error will be shown:
<4>[ 5.540139] URB 0000000000000000 submitted while active drivers/usb/core/urb.c:378 usb_submit_urb+0x4c3/0x528
Let's improve the current situation, by not re-submiting the urb if there are no users on the system.
Also add a flag that is clear during stop, that will capture this situation:
CPU 0 CPU 1 ===== ===== uvc_status_complete() uvc_status_stop() uvc_status_start() uvc_ctrl_status_event_work() -> FAIL
Hopefully, with the usb layer protection it should be enough to cover all the cases.
Cc: stable@vger.kernel.org Fixes: e5225c820c05 ("media: uvcvideo: Send a control event when a Control Change interrupt arrives") Signed-off-by: Ricardo Ribalda ribalda@chromium.org --- To: Laurent Pinchart laurent.pinchart@ideasonboard.com To: Mauro Carvalho Chehab mchehab@kernel.org Cc: linux-media@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- drivers/media/usb/uvc/uvc_ctrl.c | 9 +++++++++ drivers/media/usb/uvc/uvc_status.c | 1 + drivers/media/usb/uvc/uvcvideo.h | 2 ++ 3 files changed, 12 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index c95a2229f4fa..0634a4baa2e9 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -1442,12 +1442,20 @@ static void uvc_ctrl_status_event_work(struct work_struct *work)
uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
+ mutex_lock(&dev->lock); + if (!dev->users || !dev->resubmit_urb) { + mutex_unlock(&dev->lock); + return; + } + /* Resubmit the URB. */ w->urb->interval = dev->int_ep->desc.bInterval; ret = usb_submit_urb(w->urb, GFP_KERNEL); if (ret < 0) dev_err(&dev->udev->dev, "Failed to resubmit status URB (%d).\n", ret); + dev->resubmit_urb = false; + mutex_unlock(&dev->lock); }
bool uvc_ctrl_status_event_async(struct urb *urb, struct uvc_video_chain *chain, @@ -1466,6 +1474,7 @@ bool uvc_ctrl_status_event_async(struct urb *urb, struct uvc_video_chain *chain, w->chain = chain; w->ctrl = ctrl;
+ dev->resubmit_urb = true; schedule_work(&w->work);
return true; diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c index 7518ffce22ed..3cc6e1dfaf01 100644 --- a/drivers/media/usb/uvc/uvc_status.c +++ b/drivers/media/usb/uvc/uvc_status.c @@ -310,4 +310,5 @@ int uvc_status_start(struct uvc_device *dev, gfp_t flags) void uvc_status_stop(struct uvc_device *dev) { usb_kill_urb(dev->int_urb); + dev->resubmit_urb = false; } diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index df93db259312..9e6a52008ce5 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -539,6 +539,8 @@ struct uvc_device {
struct mutex lock; /* Protects users */ unsigned int users; + bool resubmit_urb; + atomic_t nmappings;
/* Video control interface */
--- base-commit: 0ec5a38bf8499f403f81cb81a0e3a60887d1993c change-id: 20221212-uvc-race-09276ea68bf8
Best regards,