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

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

[Runyu Xiao]

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@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
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

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@vger.kernel.org
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
> 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@synopsys.com>

BR,
Thinh