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

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

[Guangshuo Li]

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.

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")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 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:
 	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;
 err_free_host:
 	mmc_free_host(mmc);
 	/*
-- 
2.43.0

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@gmail.com>
> ---
>  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

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

[Guangshuo Li]

Hi Johan,

Thanks a lot for reviewing the patch.

On Thu, 4 Jun 2026 at 16:25, Johan Hovold <johan@kernel.org> wrote:
>
> 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@gmail.com>
> > ---
> >  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

This issue was found through manual audit of the vub300 kref, timer and
workqueue lifetime rules.

You are right, I was working on a local tree that had diverged from
current mainline. I will rebase the patch onto the latest tree and send a
v2 using the renamed error labels.

I will also update the Fixes tag to:

Fixes: 0613ad2401f8 ("mmc: vub300: fix return value check of mmc_add_host()")

and add the blank lines around the early return as suggested.

Thanks,
Guangshuo