[PATCH] usb: hcd: Cancel BH giveback works on removal

[PATCH] usb: hcd: Cancel BH giveback works on removal

[Michal Pecio]

Turns out, we do actually need to flush them, because workers use the
'high_prio_bh' and 'low_prio_bh' members of 'usb_hcd' for a brief time
after all URBs are completed to track pending completions and possibly
reschedule themselves, see usb_giveback_urb_bh() implementation.

Flushing would suffice if the works don't reschedule themselves, but
cancel_work_sync() is more robust against stray completions.

Syzbot may have found the issue due to unlucky hard IRQ timing. It can
be reproduced by adding udelay(3000) in the work function, disabling RH
autosuspend to maintain the status URB and unbinding a real HC:

[10818.828029] ehci-pci 0000:00:12.0: USB bus 1 deregistered
[10818.828077] hcd_release freeing high_prio_bh ffff88814a950978
[10818.829211] usb_giveback_urb_bh still running on bh ffff88814a950978

Reported-by: syzbot+cade843a1e4af0651f5e@syzkaller.appspotmail.com
Link: https://lore.kernel.org/linux-usb/6a8a5047.dbb3a75c.13dd47.003e.GAE@google.com/
Fixes: 94dfd7edfd5c ("USB: HCD: support giveback of URB in tasklet context")
Cc: stable@vger.kernel.org
Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---
 drivers/usb/core/hcd.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index ee19628cd653..b17fd8a0a90a 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -3053,14 +3053,12 @@ void usb_remove_hcd(struct usb_hcd *hcd)
 	mutex_unlock(&usb_bus_idr_lock);
 
 	/*
-	 * flush_work() isn't needed here because:
-	 * - driver's disconnect() called from usb_disconnect() should
-	 *   make sure its URBs are completed during the disconnect()
-	 *   callback
-	 *
-	 * - it is too late to run complete() here since driver may have
-	 *   been removed already now
+	 * Hopefully no complete() callbacks are running anymore; disconnect()
+	 * methods should have waited for them to prevent UAF of driver data.
+	 * However, we still must kill these works so they don't UAF the HCD.
 	 */
+	cancel_work_sync(&hcd->high_prio_bh.bh);
+	cancel_work_sync(&hcd->low_prio_bh.bh);
 
 	/* Prevent any more root-hub status calls from the timer.
 	 * The HCD might still restart the timer (if a port status change
-- 
2.48.1

Re: [PATCH] usb: hcd: Cancel BH giveback works on removal

[Alan Stern]

On Sun, Aug 23, 2026 at 12:58:31PM +0200, Michal Pecio wrote:
> Turns out, we do actually need to flush them, because workers use the
> 'high_prio_bh' and 'low_prio_bh' members of 'usb_hcd' for a brief time
> after all URBs are completed to track pending completions and possibly
> reschedule themselves, see usb_giveback_urb_bh() implementation.
> 
> Flushing would suffice if the works don't reschedule themselves, but
> cancel_work_sync() is more robust against stray completions.
> 
> Syzbot may have found the issue due to unlucky hard IRQ timing. It can
> be reproduced by adding udelay(3000) in the work function, disabling RH
> autosuspend to maintain the status URB and unbinding a real HC:
> 
> [10818.828029] ehci-pci 0000:00:12.0: USB bus 1 deregistered
> [10818.828077] hcd_release freeing high_prio_bh ffff88814a950978
> [10818.829211] usb_giveback_urb_bh still running on bh ffff88814a950978
> 
> Reported-by: syzbot+cade843a1e4af0651f5e@syzkaller.appspotmail.com
> Link: https://lore.kernel.org/linux-usb/6a8a5047.dbb3a75c.13dd47.003e.GAE@google.com/
> Fixes: 94dfd7edfd5c ("USB: HCD: support giveback of URB in tasklet context")

While that is logically correct, in fact the patch won't apply as-is to 
any commit earlier than 8fea0c8fda30129b ("usb: core: hcd: Convert from 
tasklet to BH workqueue").

> Cc: stable@vger.kernel.org
> Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
> ---

Reviewed-by: Alan Stern <stern@rowland.harvard.edu>

>  drivers/usb/core/hcd.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index ee19628cd653..b17fd8a0a90a 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -3053,14 +3053,12 @@ void usb_remove_hcd(struct usb_hcd *hcd)
>  	mutex_unlock(&usb_bus_idr_lock);
>  
>  	/*
> -	 * flush_work() isn't needed here because:
> -	 * - driver's disconnect() called from usb_disconnect() should
> -	 *   make sure its URBs are completed during the disconnect()
> -	 *   callback
> -	 *
> -	 * - it is too late to run complete() here since driver may have
> -	 *   been removed already now
> +	 * Hopefully no complete() callbacks are running anymore; disconnect()
> +	 * methods should have waited for them to prevent UAF of driver data.
> +	 * However, we still must kill these works so they don't UAF the HCD.
>  	 */
> +	cancel_work_sync(&hcd->high_prio_bh.bh);
> +	cancel_work_sync(&hcd->low_prio_bh.bh);
>  
>  	/* Prevent any more root-hub status calls from the timer.
>  	 * The HCD might still restart the timer (if a port status change
> -- 
> 2.48.1

Re: [PATCH] usb: hcd: Cancel BH giveback works on removal

[Michal Pecio]

On Sun, 23 Aug 2026 10:31:28 -0400, Alan Stern wrote:
> On Sun, Aug 23, 2026 at 12:58:31PM +0200, Michal Pecio wrote:
> > Turns out, we do actually need to flush them, because workers use the
> > 'high_prio_bh' and 'low_prio_bh' members of 'usb_hcd' for a brief time
> > after all URBs are completed to track pending completions and possibly
> > reschedule themselves, see usb_giveback_urb_bh() implementation.
> > 
> > Flushing would suffice if the works don't reschedule themselves, but
> > cancel_work_sync() is more robust against stray completions.
> > 
> > Syzbot may have found the issue due to unlucky hard IRQ timing. It can
> > be reproduced by adding udelay(3000) in the work function, disabling RH
> > autosuspend to maintain the status URB and unbinding a real HC:
> > 
> > [10818.828029] ehci-pci 0000:00:12.0: USB bus 1 deregistered
> > [10818.828077] hcd_release freeing high_prio_bh ffff88814a950978
> > [10818.829211] usb_giveback_urb_bh still running on bh ffff88814a950978
> > 
> > Reported-by: syzbot+cade843a1e4af0651f5e@syzkaller.appspotmail.com
> > Link: https://lore.kernel.org/linux-usb/6a8a5047.dbb3a75c.13dd47.003e.GAE@google.com/
> > Fixes: 94dfd7edfd5c ("USB: HCD: support giveback of URB in tasklet context")  
> 
> While that is logically correct, in fact the patch won't apply as-is to 
> any commit earlier than 8fea0c8fda30129b ("usb: core: hcd: Convert from 
> tasklet to BH workqueue").

Hmm, it's a fairly recent one, so this will only work on v6.12+.

Maybe not a big deal considering Greg's recent opinion about unbind
issues, though TBH I'm not really convinced about unbind's irrelevance
in this era where everything (including USB HCs) is hotpluggable.

I'm OK with this patch being dropped (I will maintain it for myself),
stripped of Cc:stable or limited to branches where it applies. And
I probably won't bother backporting it all they way to stone age.

Regards,
Michal