Re: [PATCH v2 1/1] usb: dwc3: gadget: fix IRQ storm on invalid event buffer count

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

Hi,

Sorry for the delay response.

On Wed, Aug 12, 2026, Jiazi Liu wrote:
> From: Jiazi Liu <jiazi.liu1984@gmail.com>
> 
> When dwc3_check_event_buf() reads a GEVNTCOUNT value exceeding the
> event buffer length, commit 63ccd26cd1f6 ("usb: dwc3: gadget: check
> that event count does not exceed event buffer length") returns IRQ_NONE
> without writing back GEVNTCOUNT. Since the DWC3 interrupt is
> level-triggered, the uncleared IRQ source keeps the line asserted,
> causing a tight IRQ storm that accumulates 99,900 unhandled interrupts
> and triggers spurious.c:184 BUG -> kernel panic.
> 
> The resulting call stack:
>   __report_bad_irq+0xac/0xc8
>   note_interrupt+0x340/0x468
>   handle_irq_event+0xac/0xc0
>   handle_fasteoi_irq+0x120/0x228
>   gic_handle_irq+0x68/0x108
>   ...
>   kernel BUG at kernel/irq/spurious.c:184
> 
> To reproduce, write a bogus value exceeding the event buffer length
> directly to the GEVNTCOUNT register:
> 
>   devmem <DWC3_BASE + 0xc40c> 4 0x1004
> 
> Write the bogus count back to GEVNTCOUNT to clear the IRQ source,
> consistent with the stale event clearing pattern in
> dwc3_event_buffers_setup(), and schedule error recovery to
> reinitialize the controller.
> 
> Fixes: 63ccd26cd1f6 ("usb: dwc3: gadget: check that event count does not
> exceed event buffer length")

Fixes tag should be single line.

> Cc: stable@vger.kernel.org
> Signed-off-by: Jiazi Liu <liujiazi@amazon.com>

Your email from Signed-off-by is mismatching your From: line above.

> ---
> Changes in v2:
>   - Rename softcon_work to err_recovery_work
>   - Use dev_err instead of dev_err_ratelimited
>   - Update comment to document driver/controller out-of-sync fatal error
>     condition rather than describing IRQ storm behavior
>   - Remove dev_err from work handler; error message belongs at the call
>     site where the specific failure is known
>   - Follow dwc3_gadget_suspend/resume logic in recovery work handler
>   - Add err_dying state to reject gadget driver requests during recovery
>   - Add err_recovery_count counter and stop scheduling recovery after
>     DWC3_ERR_RECOVERY_MAX attempts to prevent infinite retry loops
>   - Add synchronize_irq() before soft_disconnect to ensure IRQ handler
>     has completed before reinitializing the controller
>   - Define DWC3_ERR_RECOVERY_MAX in core.h alongside other global constants
>   - Fix sparse warning: use dwc as first argument to dwc3_writel, not dwc->regs
> ---
>  drivers/usb/dwc3/core.h   |  7 +++++++
>  drivers/usb/dwc3/gadget.c | 41 +++++++++++++++++++++++++++++++++++++--
>  2 files changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 608daeb7ef10..b1cd888eca77 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -49,6 +49,7 @@
>  #define DWC3_ENDPOINTS_NUM	32
>  #define DWC3_XHCI_RESOURCES_NUM	2
>  #define DWC3_ISOC_MAX_RETRIES	5
> +#define DWC3_ERR_RECOVERY_MAX	3
>  
>  #define DWC3_SCRATCHBUF_SIZE	4096	/* each buffer is assumed to be 4KiB */
>  #define DWC3_EVENT_BUFFERS_SIZE	4096
> @@ -1004,6 +1005,7 @@ struct dwc3_glue_ops {
>  /**
>   * struct dwc3 - representation of our controller
>   * @drd_work: workqueue used for role swapping
> + * @err_recovery_work: workqueue used for controller error recovery
>   * @ep0_trb: trb which is used for the ctrl_req
>   * @bounce: address of bounce buffer
>   * @setup_buf: used while precessing STD USB requests
> @@ -1171,6 +1173,7 @@ struct dwc3_glue_ops {
>   * @wakeup_configured: set if the device is configured for remote wakeup.
>   * @suspended: set to track suspend event due to U3/L2.
>   * @susphy_state: state of DWC3_GUSB2PHYCFG_SUSPHY + DWC3_GUSB3PIPECTL_SUSPHY
> + * @err_dying: true when controller is in error recovery, reject all requests
>   *		  before PM suspend.
>   * @imod_interval: set the interrupt moderation interval in 250ns
>   *			increments or 0 to disable.
> @@ -1186,9 +1189,11 @@ struct dwc3_glue_ops {
>   * @wakeup_pending_funcs: Indicates whether any interface has requested for
>   *			 function wakeup in bitmap format where bit position
>   *			 represents interface_id.
> + * @err_recovery_count: number of consecutive error recovery attempts
>   */
>  struct dwc3 {
>  	struct work_struct	drd_work;
> +	struct work_struct	err_recovery_work;
>  	struct dwc3_trb		*ep0_trb;
>  	void			*bounce;
>  	u8			*setup_buf;
> @@ -1420,6 +1425,7 @@ struct dwc3 {
>  	unsigned		wakeup_configured:1;
>  	unsigned		suspended:1;
>  	unsigned		susphy_state:1;
> +	unsigned		err_dying:1;

Can we change this to enum dwc3_err_state err_state where

enum dwc3_err_state {
	DWC3_ERR_NONE = 0,
	DWC3_ERR_RECOVERY,
	DWC3_ERR_UNRECOVERABLE,
};

Then we can set err_state to DWC3_ERR_RECOVERY before scheduling the
recovery work, and set it to DWC3_ERR_NONE when the recovery work is
done. If we hit the max number of retries, we can set it to
DWC3_ERR_UNRECOVERABLE. This will make it clearer what state the
controller is in.

>  
>  	u16			imod_interval;
>  
> @@ -1429,6 +1435,7 @@ struct dwc3 {
>  	struct dentry		*debug_root;
>  	u32			gsbuscfg0_reqinfo;
>  	u32			wakeup_pending_funcs;
> +	u32			err_recovery_count;
>  };
>  
>  #define INCRX_BURST_MODE 0
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index fa0f16ffafef..05f3ffab6e47 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2054,6 +2054,9 @@ static int dwc3_gadget_ep_queue(struct usb_ep *ep, struct usb_request *request,
>  
>  	int				ret;
>  
> +	if (dwc->err_dying)
> +		return -ESHUTDOWN;
> +

As Krishna brought up, may need to double check, but I think all the
gadget and ep ops need to be guarded.

>  	spin_lock_irqsave(&dwc->lock, flags);
>  	ret = __dwc3_gadget_ep_queue(dep, req);
>  	spin_unlock_irqrestore(&dwc->lock, flags);
> @@ -4667,9 +4670,22 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
>  		return IRQ_NONE;
>  
>  	if (count > evt->length) {
> -		dev_err_ratelimited(dwc->dev, "invalid count(%u) > evt->length(%u)\n",
> +		dev_err(dwc->dev, "invalid count(%u) > evt->length(%u)\n",
>  			count, evt->length);
> -		return IRQ_NONE;
> +		/*
> +		 * This is a fatal error - the driver and controller are out of
> +		 * sync on which event has been consumed. Reinitializing the
> +		 * controller is required to recover. Write the bogus count back
> +		 * to GEVNTCOUNT to clear the IRQ source, consistent with the
> +		 * stale event clearing in dwc3_event_buffers_setup(), then
> +		 * schedule error recovery.
> +		 */
> +		dwc3_writel(dwc, DWC3_GEVNTCOUNT(0), count);
> +		dwc->err_dying = true;
> +		dwc->err_recovery_count++;

This increment should be done in dwc3_err_recovery_work(). Reset the
count to 0 after err_state transition to DWC3_ERR_NONE and on reset
event interrupt.

> +		if (dwc->err_recovery_count <= DWC3_ERR_RECOVERY_MAX)

The dwc->err_recovery_count check here should be checked in the
dwc3_err_recovery_work, to check whether to perform soft connect or not.
Set the err_state to unrecoverable and print a dev_err() if soft_connect
or soft_disconnect fails or if recovery count exceeds the max.

> +			schedule_work(&dwc->err_recovery_work);
> +		return IRQ_HANDLED;
>  	}
>  
>  	evt->count = count;
> @@ -4729,6 +4745,25 @@ static void dwc_gadget_release(struct device *dev)
>  	kfree(gadget);
>  }
>  
> +static void dwc3_err_recovery_work(struct work_struct *work)
> +{
> +	struct dwc3 *dwc = container_of(work, struct dwc3, err_recovery_work);
> +	int ret;
> +
> +	synchronize_irq(dwc->irq_gadget);

I think we should use disable_irq_nosync() before soft_disconnect and
re-enabling it after soft_connect succeeds. Leave it disabled if
DWC3_ERR_RECOVERY_MAX is reached or soft_connect fails or is skipped.

> +
> +	ret = dwc3_gadget_soft_disconnect(dwc);
> +	if (ret)
> +		return;
> +
> +	dwc3_disconnect_gadget_sleepable(dwc);
> +
> +	dwc->err_dying = false;
> +
> +	if (dwc->softconnect)
> +		dwc3_gadget_soft_connect(dwc);
> +}
> +
>  /**
>   * dwc3_gadget_init - initializes gadget related registers
>   * @dwc: pointer to our controller context structure
> @@ -4772,6 +4807,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>  	}
>  
>  	init_completion(&dwc->ep0_in_setup);
> +	INIT_WORK(&dwc->err_recovery_work, dwc3_err_recovery_work);
>  	dwc->gadget = kzalloc_obj(struct usb_gadget);
>  	if (!dwc->gadget) {
>  		ret = -ENOMEM;
> @@ -4868,6 +4904,7 @@ void dwc3_gadget_exit(struct dwc3 *dwc)
>  	if (!dwc->gadget)
>  		return;
>  
> +	cancel_work_sync(&dwc->err_recovery_work);
>  	dwc3_enable_susphy(dwc, true);
>  	usb_del_gadget(dwc->gadget);
>  	dwc3_gadget_free_endpoints(dwc);
> -- 
> 2.50.1 (Apple Git-155)
> 

Thanks,
Thinh