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

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-Pl5Pbv+GP7P466ipTTIvnc23WoclnBCfAL8bYrjMMd8@public.gmane.org
> Link: https://lore.kernel.org/linux-usb/6a8a5047.dbb3a75c.13dd47.003e.GAE-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org/
> 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: [email protected]
> Signed-off-by: Michal Pecio <[email protected]>
> ---

Reviewed-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/[email protected]>

>  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