linux-kernel - Re: [PATCH v2] 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: <6e7da6f3-3a2c-48be-8fb4-c2377d86efc3@quicinc.com>
Date: Thu, 10 Jul 2025 13:28:48 +0530
From: Debraj Mukhopadhyay <quic_dmukhopa@...cinc.com>
To: Eric Biggers <ebiggers@...nel.org>
CC: Ulf Hansson <ulf.hansson@...aro.org>,
        Adrian Hunter
	<adrian.hunter@...el.com>, <linux-mmc@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <kernel@...cinc.com>, Neeraj Soni <quic_neersoni@...cinc.com>,
        "Ram Prakash
 Gupta" <quic_rampraka@...cinc.com>,
        Nitin Rawat <quic_nitirawa@...cinc.com>,
        Sachin Gupta <quic_sachgupt@...cinc.com>,
        Bhaskar Valaboju
	<quic_bhaskarv@...cinc.com>,
        Gaurav Kashyap <quic_gaurkash@...cinc.com>,
        Sarthak Garg <quic_sartgarg@...cinc.com>
Subject: Re: [PATCH v2] mmc: Avoid reprogram all keys to Inline Crypto Engine
 for MMC runtime suspend resume

Hi Eric,

Thanks for the review.

Clarifications are given inline below.

On 6/6/2025 11:29 PM, Eric Biggers wrote:
> On Fri, Jun 06, 2025 at 04:17:14PM +0530, Debraj Mukhopadhyay wrote:
>> Crypto reprogram all keys is called for each MMC runtime
>> suspend/resume in current upstream design.
> It's called from mmc_set_initial_state(), which is documented as:
>
> /*
>   * Set initial state after a power cycle or a hw_reset.
>   */
>
> Please clarify how that corresponds to "MMC runtime suspend/resume".
As part of suspend/resume, mmc regulators are turned off/on - 
effectively performing a power cycle, and hence mmc_set_initial_state is 
called after runtime suspend/resume.
Call flow:
Suspend: mmc_runtime_suspend() → _mmc_suspend() → mmc_power_off() → 
mmc_set_initial_state()
Resume: mmc_runtime_resume() → _mmc_resume() → mmc_power_up() → 
mmc_set_initial_state()
>> 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.
> The flag has a different name in the code.
Ack, will be addressed in the v3 patch.
>> diff --git a/drivers/mmc/core/crypto.c b/drivers/mmc/core/crypto.c
>> index fec4fbf16a5b..d41672e2856e 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);
> Add parentheses around 'host->caps2 & MMC_CAP2_CRYPTO'
Ack, will be addressed in the v3 patch.
>> +#ifdef CONFIG_MMC_CRYPTO
>> +#define MMC_CAP2_CRYPTO_NO_REPROG	(1 << 29)	/* Host does not support inline crypto key reprogramming */
>> +#else
>> +#define MMC_CAP2_CRYPTO_NO_REPROG	0
>> +#endif
> Well, it does support inline crypto key reprogramming.  It just doesn't want the
> MMC core driver to handle it.  Please update the comment to something like:
> /* Host driver handles crypto key reprogramming */
>
> - Eric

Ack, will be addressed in the v3 patch.

Thanks,

Debraj