On Thu, Oct 29, 2020 at 08:24:52AM +0000, Peter Chen wrote:
> On 20-10-28 23:21:44, Jack Pham wrote:
> > 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@xxxxxxxxx>
> > Signed-off-by: Jack Pham <jackp@xxxxxxxxxxxxxx>
> > ---
> > v2: call free_request() in case of ep_dequeue() failure
> >
> > drivers/usb/gadget/function/u_audio.c | 12 +++++++++---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
> > index e6d32c536781..6e69ccf02c95 100644
> > --- a/drivers/usb/gadget/function/u_audio.c
> > +++ b/drivers/usb/gadget/function/u_audio.c
> > @@ -89,7 +89,12 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
> > struct snd_uac_chip *uac = prm->uac;
> >
> > /* i/f shutting down */
> > - if (!prm->ep_enabled || req->status == -ESHUTDOWN)
> > + if (!prm->ep_enabled) {
> > + usb_ep_free_request(ep, req);
> > + return;
> > + }
> > +
> > + if (req->status == -ESHUTDOWN)
> > return;
> >
> > /*
> > @@ -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, req);
>
> There is a build error for "req", otherwise:
>
> Reviewed-and-tested-by: Peter Chen <peter.chen@xxxxxxx>
Sorry, careless mistake!! Thanks for testing, V3 on its way.
Jack
> > + /* else will be freed in u_audio_iso_complete() */
> > prm->ureq[i].req = NULL;
> > }
> > }
> > --
> > 2.24.0
> >
