[PATCH] mmc: race condition between "sdcard hot plug out" and "system reboot"

[Ulf Hansson]

On Mon, 8 Apr 2024 at 12:38, Joe.Zhou <Joe.Zhou at mediatek.com> wrote:
>
> In mmc driver, a race condition may occur between "sdcard hot plug out" and "system reboot".
> How it happen?
>
> sdcard hot pulg out:                SyS_reboot:
> CPU0                               CPU1
> mmc_sd_detect()                    _mmc_sd_suspend
> {                                  {
>
> ......
> #Step1: detect SD card removed
> if (err) {                          ......
>     #Step2: host->card is NULL
>     mmc_sd_remove(host);
>                                     #Step3:_mmc_sd_suspend claimed host
>                                     mmc_claim_host(host);
>                                     #Step4: use host->card(NULL pointer)
>                                     if (mmc_card_suspended(host->card))
>                                     ......
>                                     }
>     mmc_claim_host(host);
>     mmc_detach_bus(host);
>  }
>  ......
>  }
> we can prevent it occuring by add claim for "host->card = NULL" and add "host->card" validity check in mmc_sd_suspend.
>
> Signed-off-by: Joe.Zhou <Joe.Zhou at mediatek.com>

Thanks for your patch!

Doesn't commit 66c915d09b94 ("mmc: core: Disable card detect during
shutdown") take care of this problem?

Kind regards
Uffe

> ---
>  drivers/mmc/core/sd.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 1c8148cdda50..38c0b271283a 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -1593,7 +1593,9 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
>  static void mmc_sd_remove(struct mmc_host *host)
>  {
>         mmc_remove_card(host->card);
> +       mmc_claim_host(host);
>         host->card = NULL;
> +       mmc_release_host(host);
>  }
>
>  /*
> @@ -1702,18 +1704,19 @@ static int _mmc_sd_suspend(struct mmc_host *host)
>         int err = 0;
>
>         mmc_claim_host(host);
> +       if (host->card) {
> +               if (mmc_card_suspended(card))
> +                       goto out;
>
> -       if (mmc_card_suspended(card))
> -               goto out;
> -
> -       if (sd_can_poweroff_notify(card))
> -               err = sd_poweroff_notify(card);
> -       else if (!mmc_host_is_spi(host))
> -               err = mmc_deselect_cards(host);
> +               if (sd_can_poweroff_notify(card))
> +                       err = sd_poweroff_notify(card);
> +               else if (!mmc_host_is_spi(host))
> +                       err = mmc_deselect_cards(host);
>
> -       if (!err) {
> -               mmc_power_off(host);
> -               mmc_card_set_suspended(card);
> +               if (!err) {
> +                       mmc_power_off(host);
> +                       mmc_card_set_suspended(card);
> +               }
>         }
>
>  out:
> @@ -1729,7 +1732,7 @@ static int mmc_sd_suspend(struct mmc_host *host)
>         int err;
>
>         err = _mmc_sd_suspend(host);
> -       if (!err) {
> +       if (!err && host->card) {
>                 pm_runtime_disable(&host->card->dev);
>                 pm_runtime_set_suspended(&host->card->dev);
>         }
> --
> 2.18.0
>