Messages in this thread | | | From | Felipe Balbi <> | Subject | Re: [PATCH v2 1/5] usb: gadget: u_audio: Free requests only after callback | Date | Thu, 07 Jan 2021 11:22:50 +0200 |
| |
Jerome Brunet <jbrunet@baylibre.com> writes:
> From: Jack Pham <jackp@codeaurora.org> > > As per the kernel doc for usb_ep_dequeue(), it states that "this > routine is asynchronous, that is, it may return before the completion > routine runs". And indeed since v5.0 the dwc3 gadget driver updated > its behavior to place dequeued requests on to a cancelled list to be > given back later after the endpoint is stopped. > > The free_ep() was incorrectly assuming that a request was ready to > be freed after calling dequeue which results in a use-after-free > in dwc3 when it traverses its cancelled list. Fix this by moving > the usb_ep_free_request() call to the callback itself in case the > ep is disabled. > > Fixes: eb9fecb9e69b0 ("usb: gadget: f_uac2: split out audio core") > Reported-and-tested-by: Ferry Toth <fntoth@gmail.com> > Reviewed-and-tested-by: Peter Chen <peter.chen@nxp.com> > Signed-off-by: Jack Pham <jackp@codeaurora.org> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Looks good to me, just one comment below:
> @@ -336,8 +341,9 @@ static inline void free_ep(struct uac_rtd_params *prm, struct usb_ep *ep) > > for (i = 0; i < params->req_number; i++) { > if (prm->ureq[i].req) { > - usb_ep_dequeue(ep, prm->ureq[i].req); > - usb_ep_free_request(ep, prm->ureq[i].req); > + if (usb_ep_dequeue(ep, prm->ureq[i].req)) > + usb_ep_free_request(ep, prm->ureq[i].req);
do you mind adding a comment here stating that this is coping with a possible error during usb_ep_dequeue()?
Other than that:
Acked-by: Felipe Balbi <balbi@kernel.org>
-- balbi [unhandled content-type:application/pgp-signature] | |