linux-kernel - Re: [PATCH v4] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPDyKFrOHqStZbsye9Quk71UGkzUxOkwG9yAGYFVvt+=nMJSyw@mail.gmail.com>
Date: Wed, 21 Jan 2026 15:12:43 +0100
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Neeraj Soni <neeraj.soni@....qualcomm.com>
Cc: ebiggers@...nel.org, adrian.hunter@...el.com, quic_dmukhopa@...cinc.com, 
	quic_rampraka@...cinc.com, quic_nitirawa@...cinc.com, 
	quic_sachgupt@...cinc.com, quic_bhaskarv@...cinc.com, 
	quic_gaurkash@...cinc.com, quic_sartgarg@...cinc.com, 
	linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v4] mmc: Avoid reprogram all keys to Inline Crypto Engine
 for MMC runtime suspend resume

On Fri, 16 Jan 2026 at 13:10, Neeraj Soni <neeraj.soni@....qualcomm.com> wrote:
>
> From: Debraj Mukhopadhyay <quic_dmukhopa@...cinc.com>
>
> Crypto reprogram all keys is called for each MMC runtime
> suspend/resume in current upstream design. If this is implemented
> as a non-interruptible call to TEE for security, the cpu core is
> blocked for execution while this call executes although the crypto
> engine already has the keys. For example, glitches in audio/video
> streaming applications have been observed due to this. Add the flag
> MMC_CAP2_CRYPTO_NO_REPROG as part of host->caps2 to control reprogramming
> keys to crypto engine for socs which dont require this feature.
>
> Signed-off-by: Seshu Madhavi Puppala <quic_spuppala@...cinc.com>
> Co-developed-by: Ram Prakash Gupta <quic_rampraka@...cinc.com>
> Signed-off-by: Ram Prakash Gupta <quic_rampraka@...cinc.com>
> Co-developed-by: Sarthak Garg <quic_sartgarg@...cinc.com>
> Signed-off-by: Sarthak Garg <quic_sartgarg@...cinc.com>
> Signed-off-by: Debraj Mukhopadhyay <quic_dmukhopa@...cinc.com>
> Signed-off-by: Neeraj Soni <neeraj.soni@....qualcomm.com>
>
> ---
> Changes in v4:
> - Removed the "CONFIG_MMC_CRYPTO" encapsulation for "MMC_CAP2_CRYPTO" and
>   "MMC_CAP2_CRYPTO_NO_REPROG".
>
> Changes in v3:
> - Renamed MMC_CAP2_DONT_REPROGRAM to MMC_CAP2_CRYPTO_NO_REPROG
>   in the commit message for clarity.
> - Added parentheses around the condition: (host->caps2 & MMC_CAP2_CRYPTO)
>   to improve readability and correctness.
> - Updated the comment associated with MMC_CAP2_CRYPTO_NO_REPROG
>   to better reflect its purpose.
>
> Changes in v2:
> - Renamed MMC_CAP2_DONT_REPROGRAM to MMC_CAP2_CRYPTO_NO_REPROG for
>   improved clarity.
> - Defined MMC_CAP2_CRYPTO_NO_REPROG for MMC targets that do not support
>   a Crypto Engine.
> - Restricted the usage of struct crypto_profile to MMC devices that
>   support a Crypto Engine.
>
> Changes in v1:
> - Addressed the comments from:
>   https://lore.kernel.org/lkml/20241006135530.17363-3-
>   quic_spuppala@...cinc.com/T/#m69c9ab538bd9efd54515646952d0d7d1d7c17690
> - Avoided reprogram of keys for Qualcomm SOCs only.
> - Ensured reprogram of all keys on host controller reset.
> ---
>  drivers/mmc/core/crypto.c    | 2 +-
>  drivers/mmc/host/sdhci-msm.c | 6 ++++++
>  include/linux/mmc/host.h     | 5 +----
>  3 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/core/crypto.c b/drivers/mmc/core/crypto.c
> index fec4fbf16a5b..a5a90bfc634e 100644
> --- a/drivers/mmc/core/crypto.c
> +++ b/drivers/mmc/core/crypto.c
> @@ -15,7 +15,7 @@
>  void mmc_crypto_set_initial_state(struct mmc_host *host)
>  {
>         /* Reset might clear all keys, so reprogram all the keys. */
> -       if (host->caps2 & MMC_CAP2_CRYPTO)
> +       if ((host->caps2 & MMC_CAP2_CRYPTO) && !(host->caps2 & MMC_CAP2_CRYPTO_NO_REPROG))
>                 blk_crypto_reprogram_all_keys(&host->crypto_profile);

As far as I understand, calling blk_crypto_reprogram_all_keys() would
only be needed for those mmc hosts that lose their corresponding ICE
context during runtime+system suspend, reset and possibly during
->probe().

In other words, calling mmc_crypto_set_initial_state() from
mmc_set_initial_state() looks like it's a mistake, as it has really
nothing to do with the card's initialization, unless I have understood
this wrong!?

That said, I would rather make the mtk-sd and sdhci-msm drivers to
handle this themselves, by explicitly calling
blk_crypto_reprogram_all_keys() when needed - and drop
mmc_crypto_set_initial_state() altogether.

For the sdhci-msm case, it seems like the only case we need to care
about is for the reset.

For mtk-sd I don't know what is needed, but possibly Eric can help out here?

>  }
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 4e5edbf2fc9b..2ccb63dde9c1 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -1949,6 +1949,7 @@ static int sdhci_msm_ice_init(struct sdhci_msm_host *msm_host,
>         }
>
>         mmc->caps2 |= MMC_CAP2_CRYPTO;
> +       mmc->caps2 |= MMC_CAP2_CRYPTO_NO_REPROG;
>         return 0;
>  }
>
> @@ -2526,6 +2527,11 @@ static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
>         usleep_range(200, 210);
>         reset_control_put(reset);
>
> +#ifdef CONFIG_MMC_CRYPTO
> +       if (host->mmc->caps2 & MMC_CAP2_CRYPTO)
> +               blk_crypto_reprogram_all_keys(&host->mmc->crypto_profile);
> +#endif
> +
>         return ret;
>  }
>
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index e0e2c265e5d1..2fd76f966e24 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -457,12 +457,9 @@ struct mmc_host {
>  #define MMC_CAP2_CQE_DCMD      (1 << 24)       /* CQE can issue a direct command */
>  #define MMC_CAP2_AVOID_3_3V    (1 << 25)       /* Host must negotiate down from 3.3V */
>  #define MMC_CAP2_MERGE_CAPABLE (1 << 26)       /* Host can merge a segment over the segment size */
> -#ifdef CONFIG_MMC_CRYPTO
>  #define MMC_CAP2_CRYPTO                (1 << 27)       /* Host supports inline encryption */
> -#else
> -#define MMC_CAP2_CRYPTO                0
> -#endif
>  #define MMC_CAP2_ALT_GPT_TEGRA (1 << 28)       /* Host with eMMC that has GPT entry at a non-standard location */
> +#define MMC_CAP2_CRYPTO_NO_REPROG (1 << 29)    /* Host handles inline crypto key reprogramming */
>
>         bool                    uhs2_sd_tran;   /* UHS-II flag for SD_TRAN state */
>         bool                    uhs2_app_cmd;   /* UHS-II flag for APP command */
> --
> 2.34.1
>

Kind regards
Uffe