Re: [PATCH] mmc: vub300: fix use-after-free on probe failure

[Johan Hovold]

On Thu, Jun 04, 2026 at 01:46:28PM +0800, Guangshuo Li wrote:
> The vub300 driver lifetime-manages its controller state using
> vub300->kref, with vub300_delete() freeing the mmc host when the last
> reference is dropped. The probe error path after the inactivity timer has
> been armed still bypasses that lifetime rule, however, and falls through
> to mmc_free_host() directly if mmc_add_host() fails.
>
> The race window is between arming the inactivity timer and reaching the
> probe error unwind after mmc_add_host() fails:
> 
>         probe thread                     timer/workqueue
>         ------------                     ---------------
>         kref_init(&vub300->kref)         ref = 1
>         kref_get(&vub300->kref)          ref = 2, timer ref
>         add_timer(inactivity_timer)
>         |
>         |   race window
>         |<---------------------------------------------------->
>         |
>         mmc_add_host(mmc)
>                                          inactivity timer fires
>                                          vub300_queue_dead_work()
>                                            kref_get()          ref = 3
>                                            queue_work(deadwork)
>         mmc_add_host() fails
>         timer_delete_sync()
>         mmc_free_host(mmc)
>           frees vub300
>                                          deadwork runs
>                                            use-after-free
> 
> timer_delete_sync() only waits for the timer callback itself. It does
> not flush deadwork that the callback may already have queued. As a
> result, queued deadwork can still hold a kref while the probe error path
> directly frees the backing mmc host and the vub300 storage.

How was this issue found? Are you using some kind of static checker or
LLM?

> Fix this by making the post-timer probe error path obey the kref lifetime
> rules. After deleting the inactivity timer, drop both the timer reference
> and the initial probe reference, and return without falling through to
> err_free_host. If deadwork was queued, its reference keeps the object
> alive until the work item drops it; otherwise the final kref_put() runs
> vub300_delete() and releases the mmc host and USB resources.
>
> Fixes: 8f4d20a71022 ("mmc: vub300: fix use-after-free on disconnect")

This is not the commit that introduced this issue.

The incomplete error handling for mmc_add_host() was added by commit
0613ad2401f8 ("mmc: vub300: fix return value check of mmc_add_host()"),
and before that you'd probably get a crash on disconnect instead.

> Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
> ---
>  drivers/mmc/host/vub300.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
> index 3c9df27f9fa7..3546473d3486 100644
> --- a/drivers/mmc/host/vub300.c
> +++ b/drivers/mmc/host/vub300.c
> @@ -2341,6 +2341,14 @@ static int vub300_probe(struct usb_interface *interface,
>  	return 0;
>  error6:

Which tree did you create the patch against? These error labels have
been renamed in mainline.

>  	timer_delete_sync(&vub300->inactivity_timer);
> +	/*
> +	 * vub300 may have async references via queued deadwork. Drop the
> +	 * timer's kref and let the last kref release free vub300 via the
> +	 * proper destructor.
> +	 */
> +	kref_put(&vub300->kref, vub300_delete);  /* timer's kref */
> +	kref_put(&vub300->kref, vub300_delete);  /* init's kref */
> +	return retval;

Add a newline before and after the return statement to make it more
clear that you have an early return here.

>  err_free_host:
>  	mmc_free_host(mmc);
>  	/*

Johan