Re: [PATCH] usb: dwc3: run gadget disconnect from sleepable suspend context

[Thinh Nguyen]

On Fri, Jun 12, 2026, Runyu Xiao wrote:
> dwc3_gadget_suspend() takes dwc->lock with IRQs disabled and then calls
> dwc3_disconnect_gadget().  For async callbacks that helper only uses
> plain spin_unlock()/spin_lock(), so the gadget ->disconnect() callback
> still runs with IRQs disabled and any sleepable callback trips Lockdep.
> 
> This issue was found by our static analysis tool and then manually
> reviewed against the current tree.
> 
> The grounded PoC kept the dwc3_gadget_suspend() ->
> dwc3_disconnect_gadget() -> gadget_driver->disconnect() chain, and
> Lockdep reported:
> 
>   BUG: sleeping function called from invalid context
>   gadget_disconnect+0x21/0x39 [vuln_msv]
>   dwc3_gadget_suspend.constprop.0+0x2b/0x42 [vuln_msv]
> 
> Keep the disconnect callback selection in one common helper, but add a
> sleepable suspend-side wrapper which snapshots the callback under
> dwc->lock and then runs it after spin_unlock_irqrestore().  The regular
> event path still uses the existing spin_unlock()/spin_lock() window.
> 
> Fixes: c8540870af4c ("usb: dwc3: gadget: Improve dwc3_gadget_suspend() and dwc3_gadget_resume()")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Runyu Xiao <runyu.xiao@xxxxxxxxxx>
> ---
> Notes:
>   - Validated with a grounded Lockdep PoC that preserves the
>     dwc3_gadget_suspend() -> dwc3_disconnect_gadget() ->
>     gadget_driver->disconnect() chain.
>   - Not tested on dwc3 hardware.
> 
>  drivers/usb/dwc3/gadget.c | 43 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index db5e5b77b1ea..63faa2d3811b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -3934,15 +3934,48 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
>  	}
>  }
>  
> +static bool dwc3_prepare_disconnect_gadget(struct dwc3 *dwc,
> +					   struct usb_gadget_driver **driver,
> +					   struct usb_gadget **gadget)
> +{
> +	if (!dwc->async_callbacks || !dwc->gadget_driver ||
> +	    !dwc->gadget_driver->disconnect)
> +		return false;
> +
> +	*driver = dwc->gadget_driver;
> +	*gadget = dwc->gadget;
> +
> +	return true;
> +}
> +
>  static void dwc3_disconnect_gadget(struct dwc3 *dwc)
>  {
> -	if (dwc->async_callbacks && dwc->gadget_driver->disconnect) {
> +	struct usb_gadget_driver *driver;
> +	struct usb_gadget *gadget;
> +
> +	if (dwc3_prepare_disconnect_gadget(dwc, &driver, &gadget)) {
>  		spin_unlock(&dwc->lock);
> -		dwc->gadget_driver->disconnect(dwc->gadget);
> +		driver->disconnect(gadget);
>  		spin_lock(&dwc->lock);
>  	}
>  }
>  
> +static void dwc3_disconnect_gadget_sleepable(struct dwc3 *dwc)
> +{
> +	struct usb_gadget_driver *driver;
> +	struct usb_gadget *gadget;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&dwc->lock, flags);
> +	if (!dwc3_prepare_disconnect_gadget(dwc, &driver, &gadget)) {
> +		spin_unlock_irqrestore(&dwc->lock, flags);
> +		return;
> +	}
> +
> +	spin_unlock_irqrestore(&dwc->lock, flags);
> +	driver->disconnect(gadget);
> +}
> +
>  static void dwc3_suspend_gadget(struct dwc3 *dwc)
>  {
>  	if (dwc->async_callbacks && dwc->gadget_driver->suspend) {
> @@ -4836,7 +4869,6 @@ void dwc3_gadget_exit(struct dwc3 *dwc)
>  
>  int dwc3_gadget_suspend(struct dwc3 *dwc)
>  {
> -	unsigned long flags;
>  	int ret;
>  
>  	ret = dwc3_gadget_soft_disconnect(dwc);
> @@ -4850,10 +4882,7 @@ int dwc3_gadget_suspend(struct dwc3 *dwc)
>  		return -EAGAIN;
>  	}
>  
> -	spin_lock_irqsave(&dwc->lock, flags);
> -	if (dwc->gadget_driver)
> -		dwc3_disconnect_gadget(dwc);
> -	spin_unlock_irqrestore(&dwc->lock, flags);
> +	dwc3_disconnect_gadget_sleepable(dwc);
>  
>  	return 0;
>  }
> -- 
> 2.34.1

Thanks for the catch. I don't see any issue with this logic.

Acked-by: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx>

BR,
Thinh