usb: gadget: uvc: wait for req complete before free req

when uvc calls uvcg_video_enable() to disable the video
stream, it dequeues all requests from the usb endpoint,
and it expects the usb controller to giveback the requests
immediately, and then it can free the requests safely.
But for usb dwc3 controller, it doesn't giveback the
started requests in dequeue ops. Instead, it issues the
end transfer command and  wait for the command completion
IRQ, then giveback the requests to uvc. If the uvc driver
free req before the command completion IRQ, it will led
to null pointer dereference problem. So need to wait
for the req complete before free it.

Example call stack on RK3588 platform:

Thread#1:
uvcg_video_enable()
 -> usb_ep_dequeue()
  -> dwc3_gadget_ep_dequeue()
   -> dwc3_stop_active_transfer()
      issue end transfer command
    -> dwc3_gadget_move_cancelled_request()
     ...
      -> uvc_video_free_requests()
       -> Thread#2 executes

Thread#2:
dwc3 end transfer command completion IRQ occurs
dwc3_gadget_endpoint_command_complete()
 -> dwc3_gadget_ep_cleanup_cancelled_requests()
  -> dwc3_gadget_giveback()
   -> usb_gadget_giveback_request()
    -> uvc_video_complete()

Signed-off-by: William Wu <william.wu@rock-chips.com>
Change-Id: I0bcb8d18e851448fc973f901d74afa19ab1e2406
This commit is contained in:
William Wu
2022-11-08 17:26:00 +08:00
committed by Tao Huang
parent 7e31422aa4
commit b37ab4ccd0
2 changed files with 41 additions and 0 deletions

View File

@@ -77,6 +77,9 @@ struct uvc_request {
struct usb_request *req;
u8 *req_buffer;
struct uvc_video *video;
#if defined(CONFIG_ARCH_ROCKCHIP) && defined(CONFIG_NO_GKI)
struct completion req_done;
#endif
};
struct uvc_video {

View File

@@ -32,11 +32,42 @@ static bool uvc_using_zero_copy(struct uvc_video *video)
else
return false;
}
static void uvc_wait_req_complete(struct uvc_video *video, struct uvc_request *ureq)
{
unsigned long flags;
struct usb_request *req;
int ret;
spin_lock_irqsave(&video->req_lock, flags);
list_for_each_entry(req, &video->req_free, list) {
if (req == ureq->req)
break;
}
if (req != ureq->req) {
reinit_completion(&ureq->req_done);
spin_unlock_irqrestore(&video->req_lock, flags);
ret = wait_for_completion_timeout(&ureq->req_done,
msecs_to_jiffies(500));
if (ret == 0)
uvcg_warn(&video->uvc->func,
"timed out waiting for req done\n");
return;
}
spin_unlock_irqrestore(&video->req_lock, flags);
}
#else
static inline bool uvc_using_zero_copy(struct uvc_video *video)
{
return false;
}
static inline void uvc_wait_req_complete(struct uvc_video *video, struct uvc_request *ureq)
{ }
#endif
/* --------------------------------------------------------------------------
@@ -212,6 +243,9 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
spin_lock_irqsave(&video->req_lock, flags);
list_add_tail(&req->list, &video->req_free);
#if defined(CONFIG_ARCH_ROCKCHIP) && defined(CONFIG_NO_GKI)
complete(&ureq->req_done);
#endif
spin_unlock_irqrestore(&video->req_lock, flags);
if (uvc->state == UVC_STATE_STREAMING)
@@ -226,6 +260,7 @@ uvc_video_free_requests(struct uvc_video *video)
if (video->ureq) {
for (i = 0; i < video->uvc_num_requests; ++i) {
if (video->ureq[i].req) {
uvc_wait_req_complete(video, &video->ureq[i]);
usb_ep_free_request(video->ep, video->ureq[i].req);
video->ureq[i].req = NULL;
}
@@ -282,6 +317,9 @@ uvc_video_alloc_requests(struct uvc_video *video)
video->ureq[i].req->context = &video->ureq[i];
video->ureq[i].video = video;
#if defined(CONFIG_ARCH_ROCKCHIP) && defined(CONFIG_NO_GKI)
init_completion(&video->ureq[i].req_done);
#endif
list_add_tail(&video->ureq[i].req->list, &video->req_free);
}