linux-kernel - Re: [PATCH v1] 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] [day] [month] [year] [list]

Message-ID: <d0945da8-de9b-4906-b5fc-b7cd55c6bdb3@intel.com>
Date: Wed, 5 Mar 2025 19:01:52 +0200
From: Adrian Hunter <adrian.hunter@...el.com>
To: Seshu Madhavi Puppala <quic_spuppala@...cinc.com>, Ulf Hansson
	<ulf.hansson@...aro.org>
CC: <linux-mmc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-arm-msm@...r.kernel.org>, <quic_rampraka@...cinc.com>,
	<quic_nitirawa@...cinc.com>, <quic_sachgupt@...cinc.com>,
	<quic_bhaskarv@...cinc.com>, <kernel@...cinc.com>,
	<quic_neersoni@...cinc.com>, <quic_gaurkash@...cinc.com>
Subject: Re: [PATCH v1] mmc: Avoid reprogram all keys to Inline Crypto Engine
 for MMC runtime suspend resume

On 11/02/25 12:08, Seshu Madhavi Puppala wrote:
> 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_DONT_REPROGRAM as part of host->caps2 to control reprogramming
> keys to crypto engine for socs which dont require this feature.
> 
> This patch addresses the following:
> - Addresses the comments from:
>   https://lore.kernel.org/lkml/20241006135530.17363-3-
>   quic_spuppala@...cinc.com/T/#m69c9ab538bd9efd54515646952d0d7d1d7c17690
> - Avoids reprogram of keys for Qualcomm SOCs only.
> - Ensures reprogram of all keys on host controller reset.
> 
> 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>
> ---
>  drivers/mmc/core/crypto.c    | 2 +-
>  drivers/mmc/host/sdhci-msm.c | 4 ++++
>  include/linux/mmc/host.h     | 3 +++
>  3 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/crypto.c b/drivers/mmc/core/crypto.c
> index fec4fbf16a5b..d947cd1bbfe3 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_DONT_REPROGRAM))
>  		blk_crypto_reprogram_all_keys(&host->crypto_profile);
>  }
>  
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 4610f067faca..52207873e18d 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -1865,6 +1865,7 @@ static int sdhci_msm_ice_init(struct sdhci_msm_host *msm_host,
>  	}
>  
>  	mmc->caps2 |= MMC_CAP2_CRYPTO;
> +	mmc->caps2 |= MMC_CAP2_DONT_REPROGRAM;
>  	return 0;
>  }
>  
> @@ -2451,6 +2452,9 @@ static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
>  	usleep_range(200, 210);
>  	reset_control_put(reset);
>  
> +	if (host->mmc->caps2 & MMC_CAP2_CRYPTO)
> +		blk_crypto_reprogram_all_keys(&host->mmc->crypto_profile);
> +
>  	return ret;
>  }
>  
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 68f09a955a90..d1e6fafca21c 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -459,6 +459,9 @@ struct mmc_host {
>  #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 */
> +#ifdef CONFIG_MMC_CRYPTO
> +#define MMC_CAP2_DONT_REPROGRAM	(1 << 29)
> +#endif

The #ifdef is not needed since we probably want drivers
that reference MMC_CAP2_DONT_REPROGRAM to still compile.

Ulf suggested the name MMC_CAP2_CRYPTO_NO_PROG which seems
much better since it contains "CRYPTO"

>  
>  	bool			uhs2_sd_tran;	/* UHS-II flag for SD_TRAN state */
>  	bool			uhs2_app_cmd;	/* UHS-II flag for APP command */