| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAPDyKFqYgOOJdT-0oxCyrfCjW8SuORX3+tXq1D09Qj1jYDOKpw@mail.gmail.com>
Date: Thu, 22 Jan 2026 18:32:55 +0100
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Penghe Geng <pgeng@...dia.com>, Adrian Hunter <adrian.hunter@...el.com>
Cc: linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org,
stable@...r.kernel.org
Subject: Re: [PATCH mmc v1] mmc: core: Fix bitfield race between retune and
host claiming
+ Adrian
On Thu, 15 Jan 2026 at 22:46, Penghe Geng <pgeng@...dia.com> wrote:
>
> The host->claimed flag shares a bitfield storage word with several
> retune flags (retune_now, retune_paused, can_retune, doing_retune,
> doing_init_tune). Updating those flags without host->lock can RMW the
> shared word and clear claimed, triggering spurious
> WARN_ON(!host->claimed).
>
> Serialize all retune bitfield updates with host->lock. Provide lockless
> __mmc_retune_* helpers so callers that already hold host->lock can
> avoid deadlocks while public wrappers serialize updates. Also protect
> doing_init_tune and the CQE retune_now assignment with host->lock.
>
> Fixes: dfa13ebbe334 ("mmc: host: Add facility to support re-tuning")
> Cc: stable@...r.kernel.org
> Signed-off-by: Penghe Geng <pgeng@...dia.com>
Thanks for the patch! I have looped in Adrian to get his opinion on this.
Kind regards
Uffe
> ---
> drivers/mmc/core/host.c | 60 +++++++++++++++++++++++++++++++---------
> drivers/mmc/core/host.h | 35 ++++++++++++++++++++++-
> drivers/mmc/core/mmc.c | 6 ++++
> drivers/mmc/core/queue.c | 3 ++
> include/linux/mmc/host.h | 4 +++
> 5 files changed, 94 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index 88c95dbfd9cf..0b6b4a31f629 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -109,7 +109,11 @@ void mmc_unregister_host_class(void)
> */
> void mmc_retune_enable(struct mmc_host *host)
> {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> host->can_retune = 1;
> + spin_unlock_irqrestore(&host->lock, flags);
> if (host->retune_period)
> mod_timer(&host->retune_timer,
> jiffies + host->retune_period * HZ);
> @@ -121,18 +125,31 @@ void mmc_retune_enable(struct mmc_host *host)
> */
> void mmc_retune_pause(struct mmc_host *host)
> {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> if (!host->retune_paused) {
> host->retune_paused = 1;
> - mmc_retune_hold(host);
> + __mmc_retune_hold(host);
> }
> + spin_unlock_irqrestore(&host->lock, flags);
> }
> EXPORT_SYMBOL(mmc_retune_pause);
>
> void mmc_retune_unpause(struct mmc_host *host)
> {
> + unsigned long flags;
> + bool released;
> +
> + spin_lock_irqsave(&host->lock, flags);
> if (host->retune_paused) {
> host->retune_paused = 0;
> - mmc_retune_release(host);
> + released = __mmc_retune_release(host);
> + spin_unlock_irqrestore(&host->lock, flags);
> + if (!released)
> + WARN_ON(1);
> + } else {
> + spin_unlock_irqrestore(&host->lock, flags);
> }
> }
> EXPORT_SYMBOL(mmc_retune_unpause);
> @@ -145,8 +162,12 @@ EXPORT_SYMBOL(mmc_retune_unpause);
> */
> void mmc_retune_disable(struct mmc_host *host)
> {
> + unsigned long flags;
> +
> mmc_retune_unpause(host);
> + spin_lock_irqsave(&host->lock, flags);
> host->can_retune = 0;
> + spin_unlock_irqrestore(&host->lock, flags);
> timer_delete_sync(&host->retune_timer);
> mmc_retune_clear(host);
> }
> @@ -159,16 +180,22 @@ EXPORT_SYMBOL(mmc_retune_timer_stop);
>
> void mmc_retune_hold(struct mmc_host *host)
> {
> - if (!host->hold_retune)
> - host->retune_now = 1;
> - host->hold_retune += 1;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> + __mmc_retune_hold(host);
> + spin_unlock_irqrestore(&host->lock, flags);
> }
>
> void mmc_retune_release(struct mmc_host *host)
> {
> - if (host->hold_retune)
> - host->hold_retune -= 1;
> - else
> + unsigned long flags;
> + bool released;
> +
> + spin_lock_irqsave(&host->lock, flags);
> + released = __mmc_retune_release(host);
> + spin_unlock_irqrestore(&host->lock, flags);
> + if (!released)
> WARN_ON(1);
> }
> EXPORT_SYMBOL(mmc_retune_release);
> @@ -177,18 +204,23 @@ int mmc_retune(struct mmc_host *host)
> {
> bool return_to_hs400 = false;
> int err;
> + unsigned long flags;
>
> - if (host->retune_now)
> - host->retune_now = 0;
> - else
> + spin_lock_irqsave(&host->lock, flags);
> + if (!host->retune_now) {
> + spin_unlock_irqrestore(&host->lock, flags);
> return 0;
> + }
> + host->retune_now = 0;
>
> - if (!host->need_retune || host->doing_retune || !host->card)
> + if (!host->need_retune || host->doing_retune || !host->card) {
> + spin_unlock_irqrestore(&host->lock, flags);
> return 0;
> + }
>
> host->need_retune = 0;
> -
> host->doing_retune = 1;
> + spin_unlock_irqrestore(&host->lock, flags);
>
> if (host->ios.timing == MMC_TIMING_MMC_HS400) {
> err = mmc_hs400_to_hs200(host->card);
> @@ -205,7 +237,9 @@ int mmc_retune(struct mmc_host *host)
> if (return_to_hs400)
> err = mmc_hs200_to_hs400(host->card);
> out:
> + spin_lock_irqsave(&host->lock, flags);
> host->doing_retune = 0;
> + spin_unlock_irqrestore(&host->lock, flags);
>
> return err;
> }
> diff --git a/drivers/mmc/core/host.h b/drivers/mmc/core/host.h
> index 5941d68ff989..07e4f427fe15 100644
> --- a/drivers/mmc/core/host.h
> +++ b/drivers/mmc/core/host.h
> @@ -21,22 +21,55 @@ int mmc_retune(struct mmc_host *host);
> void mmc_retune_pause(struct mmc_host *host);
> void mmc_retune_unpause(struct mmc_host *host);
>
> -static inline void mmc_retune_clear(struct mmc_host *host)
> +static inline void __mmc_retune_clear(struct mmc_host *host)
> {
> host->retune_now = 0;
> host->need_retune = 0;
> }
>
> +static inline void mmc_retune_clear(struct mmc_host *host)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> + __mmc_retune_clear(host);
> + spin_unlock_irqrestore(&host->lock, flags);
> +}
> +
> +static inline void __mmc_retune_hold(struct mmc_host *host)
> +{
> + if (!host->hold_retune)
> + host->retune_now = 1;
> + host->hold_retune += 1;
> +}
> +
> +static inline bool __mmc_retune_release(struct mmc_host *host)
> +{
> + if (host->hold_retune) {
> + host->hold_retune -= 1;
> + return true;
> + }
> + return false;
> +}
> +
> static inline void mmc_retune_hold_now(struct mmc_host *host)
> {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> host->retune_now = 0;
> host->hold_retune += 1;
> + spin_unlock_irqrestore(&host->lock, flags);
> }
>
> static inline void mmc_retune_recheck(struct mmc_host *host)
> {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> if (host->hold_retune <= 1)
> host->retune_now = 1;
> + spin_unlock_irqrestore(&host->lock, flags);
> }
>
> static inline int mmc_host_can_cmd23(struct mmc_host *host)
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 7c86efb1044a..114febd15f08 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1820,13 +1820,19 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> goto free_card;
>
> if (mmc_card_hs200(card)) {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> host->doing_init_tune = 1;
> + spin_unlock_irqrestore(&host->lock, flags);
>
> err = mmc_hs200_tuning(card);
> if (!err)
> err = mmc_select_hs400(card);
>
> + spin_lock_irqsave(&host->lock, flags);
> host->doing_init_tune = 0;
> + spin_unlock_irqrestore(&host->lock, flags);
>
> if (err)
> goto free_card;
> diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
> index 284856c8f655..5e38759c87f5 100644
> --- a/drivers/mmc/core/queue.c
> +++ b/drivers/mmc/core/queue.c
> @@ -237,6 +237,7 @@ static blk_status_t mmc_mq_queue_rq(struct blk_mq_hw_ctx *hctx,
> enum mmc_issue_type issue_type;
> enum mmc_issued issued;
> bool get_card, cqe_retune_ok;
> + unsigned long flags;
> blk_status_t ret;
>
> if (mmc_card_removed(mq->card)) {
> @@ -297,8 +298,10 @@ static blk_status_t mmc_mq_queue_rq(struct blk_mq_hw_ctx *hctx,
> mmc_get_card(card, &mq->ctx);
>
> if (host->cqe_enabled) {
> + spin_lock_irqsave(&host->lock, flags);
> host->retune_now = host->need_retune && cqe_retune_ok &&
> !host->hold_retune;
> + spin_unlock_irqrestore(&host->lock, flags);
> }
>
> blk_mq_start_request(req);
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index e0e2c265e5d1..e7bddbafd1da 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -713,8 +713,12 @@ void mmc_retune_timer_stop(struct mmc_host *host);
>
> static inline void mmc_retune_needed(struct mmc_host *host)
> {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> if (host->can_retune)
> host->need_retune = 1;
> + spin_unlock_irqrestore(&host->lock, flags);
> }
>
> static inline bool mmc_can_retune(struct mmc_host *host)
> --
> 2.43.0
>
Powered by blists - more mailing lists