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

On Fri, Jun 12, 2026 at 5:28 AM Guangshuo Li <[email protected]> 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: 0613ad2401f8 ("mmc: vub300: fix return value check of mmc_add_host()")
> Signed-off-by: Guangshuo Li <[email protected]>
> Reviewed-by: Johan Hovold <[email protected]>

Applied for fixes and by adding a stable tag, thanks!

Kind regards
Uffe


> ---
> v5:
>   - Correct the Fixes tag.
>   - Add Johan's Reviewed-by tag.
>
> v4:
>   - Fix the Fixes tag to point to the commit which added the
>     mmc_add_host() failure unwind.
>   - Add Johan's Reviewed-by tag.
>
> 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
>