linux-kernel - Re: [PATCH] usb: dwc3: Ignore late xferNotReady event to prevent halt timeout

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKzKK0r7StRcA=dEZSzK2=yN-HdFyKDsDV=+yzTdE0_UJboORw@mail.gmail.com>
Date: Wed, 6 Aug 2025 18:17:52 +0800
From: Kuen-Han Tsai <khtsai@...gle.com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>, 
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, stable <stable@...nel.org>
Subject: Re: [PATCH] usb: dwc3: Ignore late xferNotReady event to prevent halt timeout

Hi Thinh,

Thank you for your review.

On Wed, Aug 6, 2025 at 7:28 AM Thinh Nguyen <Thinh.Nguyen@...opsys.com> wrote:
>
> On Tue, Aug 05, 2025, Kuen-Han Tsai wrote:
> > During a device-initiated disconnect, the End Transfer command resets
> > the event filter, allowing a new xferNotReady event to be generated
> > before the controller is fully halted. Processing this late event
> > incorrectly triggers a Start Transfer, which prevents the controller
> > from halting and results in a DSTS.DEVCTLHLT bit polling timeout.
> >
> > Ignore the late xferNotReady event if the controller is already in a
> > disconnected state.
> >
> > Fixes: 8f608e8ab628 ("usb: dwc3: gadget: remove unnecessary 'dwc' parameter")
>
> The Fixes should target since the beginning of the driver:
> 72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")

Commit 72246da40f37 doesn't have the dwc->connected member. Should we
change the Fixes tag to f09ddcfcb8c5 ("usb: dwc3: gadget: Prevent EP
queuing while stopping transfers")?
That's the commit where the "dwc->connected = false" logic was moved before
stopping active transfers within the pullup function.

>
> > Cc: stable <stable@...nel.org>
> > Signed-off-by: Kuen-Han Tsai <khtsai@...gle.com>
> > ---
> > Tracing:
> >
> > # Stop active transfers by sending End Transfer commands
> > dwc3_gadget_ep_cmd: ep1out: cmd 'End Transfer' [20d08] params 00000000 00000000 00000000 --> status: Successful
> > dwc3_gadget_ep_cmd: ep1in: cmd 'End Transfer' [40d08] params 00000000 00000000 00000000 --> status: Successful
> >  ...
> > # Recieve an xferNotReady event on an ISOC IN endpoint
> > dwc3_event: event (35d010c6): ep1in: Transfer Not Ready [000035d0] (Not Active)
> > dwc3_gadget_ep_cmd: ep1in: cmd 'Start Transfer' [35d60406] params 00000000 ffffb620 00000000 --> status: Successful
> > dwc3_gadget_ep_cmd: ep2in: cmd 'End Transfer' [30d08] params 00000000 00000000 00000000 --> status: Timed Out
> >  ...
> > # Start polling DSTS.DEVCTRLHLT
> > dwc3_gadget_run_stop: start polling DWC3_DSTS_DEVCTRLHLT
> >  ...
> > # HALT timeout and print out the endpoint status for debugging
> > dwc3_gadget_run_stop: finish polling DWC3_DSTS_DEVCTRLHLT, is_on=0, reg=0
> > dwc3_gadget_ep_status: ep1out: mps 1024/2765 streams 16 burst 5 ring 64/56 flags E:swbp:>
> > dwc3_gadget_ep_status: ep1in: mps 1024/1024 streams 16 burst 2 ring 21/64 flags E:swBp:<
> > dwc3_gadget_ep_status: ep2out: mps 1024/2765 streams 16 burst 5 ring 56/48 flags e:swbp:>
> > ---
> >  drivers/usb/dwc3/gadget.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index 25db36c63951..ad89cbeea761 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -3777,6 +3777,15 @@ static void dwc3_gadget_endpoint_transfer_complete(struct dwc3_ep *dep,
> >  static void dwc3_gadget_endpoint_transfer_not_ready(struct dwc3_ep *dep,
> >               const struct dwc3_event_depevt *event)
> >  {
> > +     /*
> > +      * During a device-initiated disconnect, a late xferNotReady event can
> > +      * be generated after the End Transfer command resets the event filter,
> > +      * but before the controller is halted. Ignore it to prevent a new
> > +      * transfer from starting.
> > +      */
> > +     if (dep->dwc->connected)
>
> Did you test this? This is supposed to be if (!dep->dwc->connected)
> right?
>

You're absolutely right, I'll update your suggested change in the next
patch. Sorry for the mistake.

The code in my Android testing environment was correct and we've
verified that it resolves the halt problem.

Regards,
Kuen-Han




> > +             return;
> > +
> >       dwc3_gadget_endpoint_frame_from_event(dep, event);
> >
> >       /*
> > --
> > 2.50.1.565.gc32cd1483b-goog
> >