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@xxxxxxxxxx> 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@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

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