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

[PATCH v3] 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)      fires after one second
        |
        |   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

The inactivity timeout is one second, so this would require
mmc_add_host() to both fail and take more than one second to do so. This
is unlikely to happen in practice, but the error path is still wrong.

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, including the vub300 storage.

Fix this by using the same lifetime mechanism as disconnect. Clear
vub300->interface so that the timer callback and any queued deadwork
return early and drop their references, then drop the initial probe
reference and return without falling through to err_free_host.

Fixes: 8f4d20a71022 ("mmc: vub300: fix use-after-free on disconnect")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
v3:
  - Use the disconnect-style teardown by clearing vub300->interface.
  - Drop only the initial probe reference and let timer/deadwork drop their
    own references.
  - Mention the one-second inactivity timeout in the commit message.

v2:
  - Rebase on current mainline.
  - Correct the Fixes tag.
  - Add blank lines around the early return.
  - Reword the code comment.

 drivers/mmc/host/vub300.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
index 6c3cb2f1c9d3..c1c21e95f5bf 100644
--- a/drivers/mmc/host/vub300.c
+++ b/drivers/mmc/host/vub300.c
@@ -2336,12 +2336,16 @@ static int vub300_probe(struct usb_interface *interface,
 			 interface_to_InterfaceNumber(interface));
 	retval = mmc_add_host(mmc);
 	if (retval)
-		goto err_delete_timer;
+		goto err_stop_io;
 
 	return 0;
 
-err_delete_timer:
-	timer_delete_sync(&vub300->inactivity_timer);
+err_stop_io:
+	vub300->interface = NULL;
+	kref_put(&vub300->kref, vub300_delete);
+
+	return retval;
+
 err_free_host:
 	mmc_free_host(mmc);
 	/*
-- 
2.43.0

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

[Johan Hovold]

On Wed, Jun 10, 2026 at 11:49:03AM +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)      fires after one second
>         |
>         |   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
> 
> The inactivity timeout is one second, so this would require
> mmc_add_host() to both fail and take more than one second to do so. This
> is unlikely to happen in practice, but the error path is still wrong.
> 
> 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, including the vub300 storage.
> 
> Fix this by using the same lifetime mechanism as disconnect. Clear
> vub300->interface so that the timer callback and any queued deadwork
> return early and drop their references, then drop the initial probe
> reference and return without falling through to err_free_host.
> 
> Fixes: 8f4d20a71022 ("mmc: vub300: fix use-after-free on disconnect")

You are using the wrong Fixes tag here again. This should be

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

unless you consider this bug to have been there since the driver was
added.

> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> v3:
>   - Use the disconnect-style teardown by clearing vub300->interface.
>   - Drop only the initial probe reference and let timer/deadwork drop their
>     own references.
>   - Mention the one-second inactivity timeout in the commit message.
> 
> v2:
>   - Rebase on current mainline.
>   - Correct the Fixes tag.
>   - Add blank lines around the early return.
>   - Reword the code comment.

This looks good to me now otherwise, so with the above fixed:

Reviewed-by: Johan Hovold <johan@kernel.org>

Johan