Linux MultiMedia Card development
 help / color / mirror / Atom feed
From: Johan Hovold <johan@kernel.org>
To: Guangshuo Li <lgs201920130244@gmail.com>
Cc: Ulf Hansson <ulfh@kernel.org>,
	Huacai Chen <chenhuacai@kernel.org>,
	Binbin Zhou <zhoubinbin@loongson.cn>,
	linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] mmc: vub300: fix use-after-free on probe failure
Date: Wed, 10 Jun 2026 09:57:06 +0200	[thread overview]
Message-ID: <aikY0nFweVi4fn6v@hovoldconsulting.com> (raw)
In-Reply-To: <20260610034904.1596521-1-lgs201920130244@gmail.com>

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

      reply	other threads:[~2026-06-10  7:57 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-10  3:49 [PATCH v3] mmc: vub300: fix use-after-free on probe failure Guangshuo Li
2026-06-10  7:57 ` Johan Hovold [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aikY0nFweVi4fn6v@hovoldconsulting.com \
    --to=johan@kernel.org \
    --cc=chenhuacai@kernel.org \
    --cc=lgs201920130244@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=ulfh@kernel.org \
    --cc=zhoubinbin@loongson.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox