linux-kernel - Re: [PATCH v4] usb: dwc3: Abort suspend on soft disconnect failure

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: <CAKzKK0pReSZeJ1-iRUbU=w+dK0O1fB7AgecfC7KJap6m5cQWnQ@mail.gmail.com>
Date: Tue, 22 Apr 2025 23:50:35 +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@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH v4] usb: dwc3: Abort suspend on soft disconnect failure

On Tue, Apr 22, 2025 at 7:20 AM Thinh Nguyen <Thinh.Nguyen@...opsys.com> wrote:
>
> On Mon, Apr 21, 2025, Kuen-Han Tsai wrote:
> > On Sat, Apr 19, 2025 at 9:24 AM Thinh Nguyen <Thinh.Nguyen@...opsys.com> wrote:
> > >
> > > On Wed, Apr 16, 2025, Kuen-Han Tsai wrote:
>
> <snip>
>
> > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > > > index 89a4dc8ebf94..630fd5f0ce97 100644
> > > > --- a/drivers/usb/dwc3/gadget.c
> > > > +++ b/drivers/usb/dwc3/gadget.c
> > > > @@ -4776,26 +4776,22 @@ int dwc3_gadget_suspend(struct dwc3 *dwc)
> > > >       int ret;
> > > >
> > > >       ret = dwc3_gadget_soft_disconnect(dwc);
> > > > -     if (ret)
> > > > -             goto err;
> > > > -
> > > > -     spin_lock_irqsave(&dwc->lock, flags);
> > > > -     if (dwc->gadget_driver)
> > > > -             dwc3_disconnect_gadget(dwc);
> > > > -     spin_unlock_irqrestore(&dwc->lock, flags);
> > > > -
> > > > -     return 0;
> > > > -
> > > > -err:
> > > >       /*
> > > >        * Attempt to reset the controller's state. Likely no
> > > >        * communication can be established until the host
> > > >        * performs a port reset.
> > > >        */
> > > > -     if (dwc->softconnect)
> > > > +     if (ret && dwc->softconnect) {
> > > >               dwc3_gadget_soft_connect(dwc);
> > > > +             return -EAGAIN;
> > >
> > > This may make sense to have -EAGAIN for runtime suspend. I supposed this
> > > should be fine for system suspend since it doesn't do anything special
> > > for this error code.
> > >
> > > When you tested runtime suspend, did you observe that the device
> > > successfully going into suspend on retry?
> >
> > Hi Thinh,
> >
> > Yes, the dwc3 device can be suspended using either
> > pm_runtime_suspend() or dwc3_gadget_pullup(), the latter of which
> > ultimately invokes pm_runtime_put().
> >
> > One question: Do you know how to naturally cause a run stop failure?
> > Based on the spec, the controller cannot halt until the event buffer
> > becomes empty. If the driver doesn't acknowledge the events, this
> > should lead to the run stop failure. However, since I cannot naturally
> > reproduce this problem, I am simulating this scenario by modifying
> > dwc3_gadget_run_stop() to return a timeout error directly.
> >
>
> I'm not clear what you meant by "naturally" here. The driver is
> implemented in such a way that this should not happen. If it does, we
> need to take look to see what we missed.
>
> However, to force the driver to hit the controller halt timeout, just
> wait/generate some events and don't clear the GEVNTCOUNT of event bytes
> before clearing the run_stop bit.
>
> BR,
> Thinh

Hi Thinh,

Thank you for getting back to me and the method to force the timeout!

By "naturally," I meant reproducing the issue without artificial steps
designed solely to trigger it. You're right; since the driver is
designed to prevent this state, reproducing it "naturally" is
difficult.

I really appreciate your patience, and thank you once more for the
helpful clarification.

Regards,
Kuen-Han