From b37ab4ccd03095614b2395573f4dcbd07958be9d Mon Sep 17 00:00:00 2001 From: William Wu Date: Tue, 8 Nov 2022 17:26:00 +0800 Subject: [PATCH] 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 Change-Id: I0bcb8d18e851448fc973f901d74afa19ab1e2406 --- drivers/usb/gadget/function/uvc.h | 3 ++ drivers/usb/gadget/function/uvc_video.c | 38 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index 7e9af56d060a..9a5507d49545 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -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 { diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index 4962b5562ba7..bec9e6155d61 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -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); }