[PATCH] usb: gadget: dummy_hcd: fix IRQs-disabled violation in dummy_timer()

[PATCH] usb: gadget: dummy_hcd: fix IRQs-disabled violation in dummy_timer()

[Jiangong.Han]

dummy_timer() acquires dum->lock using spin_lock_irqsave(), which disables
local IRQs. Before invoking URB/request completion callbacks it drops the
lock with plain spin_unlock(), which releases the spinlock but leaves IRQs
disabled. The completion handlers (and any code they call) then execute
with IRQs disabled.

This is illegal for any completion handler that uses spin_unlock_bh() or
any other operation that requires IRQs to be enabled.  In this case the
call chain is:

  dummy_timer      [spin_lock_irqsave]
    spin_unlock           <- IRQs still disabled
    usb_hcd_giveback_urb
      carl9170_usb_rx_irq_complete
        carl9170_tx_process_status
          carl9170_get_queued_skb
            spin_unlock_bh
              __local_bh_enable_ip
                lockdep_assert_irqs_enabled  <- WARN

Fix this by replacing spin_unlock() with spin_unlock_irqrestore()
before usb_hcd_giveback_urb(), so that IRQs are properly re-enabled
before invoking callbacks and re-disabled after they return.

Function usb_gadget_giveback_request() calls fifo_complete() which
does nothing, so just focus on this issue.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+381102a7292a374fe8a7@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=381102a7292a374fe8a7
Signed-off-by: Jiangong.Han <jiangong.han@windriver.com>
---
 drivers/usb/gadget/udc/dummy_hcd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index f47903461ed5..d11306202a5a 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -2001,9 +2001,9 @@ static enum hrtimer_restart dummy_timer(struct hrtimer *t)
 			ep->already_seen = ep->setup_stage = 0;
 
 		usb_hcd_unlink_urb_from_ep(dummy_hcd_to_hcd(dum_hcd), urb);
-		spin_unlock(&dum->lock);
+		spin_unlock_irqrestore(&dum->lock, flags);
 		usb_hcd_giveback_urb(dummy_hcd_to_hcd(dum_hcd), urb, status);
-		spin_lock(&dum->lock);
+		spin_lock_irqsave(&dum->lock, flags);
 
 		goto restart;
 	}
-- 
2.37.3

Re: [PATCH] usb: gadget: dummy_hcd: fix IRQs-disabled violation in dummy_timer()

[Alan Stern]

On Thu, Jul 16, 2026 at 10:07:13AM +0800, Jiangong.Han wrote:
> dummy_timer() acquires dum->lock using spin_lock_irqsave(), which disables
> local IRQs. Before invoking URB/request completion callbacks it drops the
> lock with plain spin_unlock(), which releases the spinlock but leaves IRQs
> disabled. The completion handlers (and any code they call) then execute
> with IRQs disabled.
> 
> This is illegal for any completion handler that uses spin_unlock_bh() or
> any other operation that requires IRQs to be enabled.  In this case the
> call chain is:
> 
>   dummy_timer      [spin_lock_irqsave]
>     spin_unlock           <- IRQs still disabled
>     usb_hcd_giveback_urb
>       carl9170_usb_rx_irq_complete
>         carl9170_tx_process_status
>           carl9170_get_queued_skb
>             spin_unlock_bh
>               __local_bh_enable_ip
>                 lockdep_assert_irqs_enabled  <- WARN
> 
> Fix this by replacing spin_unlock() with spin_unlock_irqrestore()
> before usb_hcd_giveback_urb(), so that IRQs are properly re-enabled
> before invoking callbacks and re-disabled after they return.
> 
> Function usb_gadget_giveback_request() calls fifo_complete() which
> does nothing, so just focus on this issue.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: syzbot+381102a7292a374fe8a7@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=381102a7292a374fe8a7
> Signed-off-by: Jiangong.Han <jiangong.han@windriver.com>
> ---

This is not the right way to solve the problem observed in carl9170.  It 
is documented that usb_hcd_giveback_urb() runs in atomic context, and 
drivers' completion handlers should not expect to be called with 
interrupts enabled.  The problem lies in carl9170_get_queued_skb(); 
that's where it needs to be fixed.

Alan Stern

Re: [PATCH] usb: gadget: dummy_hcd: fix IRQs-disabled violation in dummy_timer()

[Sebastian Andrzej Siewior]

On 2026-07-15 22:15:52 [-0400], Alan Stern wrote:
> This is not the right way to solve the problem observed in carl9170.  It 
> is documented that usb_hcd_giveback_urb() runs in atomic context, and 
> drivers' completion handlers should not expect to be called with 
> interrupts enabled.  The problem lies in carl9170_get_queued_skb(); 
> that's where it needs to be fixed.

I guess we don't see this that often because the widely used HCDs have
HCD_BH set.
I sort of think that we already did have this conversation about
having unlock+lock with irq and not skipping it. But yes, while this
would solve the problem _here_ we would still have the same warning on
OHCI.

Take a look at other drivers in drivers/net/usb/, they delay their doing
to napi/ workqueue/ tasklet.

> Alan Stern

Sebastian