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 for Android: free password hash cracker in your pocket

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

Message-ID: <20250421232007.u2tmih4djakhttxq@synopsys.com>
Date: Mon, 21 Apr 2025 23:20:16 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Kuen-Han Tsai <khtsai@...gle.com>
CC: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        "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 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