[PATCH v2 1/4] ARM: stm32: Fix ECDSA authentication with Dcache enabled

[Patrick DELAUNAY]

Hi Marek,


Sorry for the delay.

I cross-check with ROM code team to understood this API limitation.


On 12/6/22 23:49, Marek Vasut wrote:
> In case Dcache is enabled while the ECDSA authentication function is
> called via BootROM ROM API, the CRYP DMA might pick stale version of
> data from DRAM. Disable Dcache around the BootROM call to avoid this
> issue.
>
> Signed-off-by: Marek Vasut <marex at denx.de>
> ---
> Cc: Alexandru Gagniuc <mr.nuke.me at gmail.com>
> Cc: Patrice Chotard <patrice.chotard at foss.st.com>
> Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>
> ---
> V2: - Initialize reenable_dcache variable
> ---
>   arch/arm/mach-stm32mp/ecdsa_romapi.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c b/arch/arm/mach-stm32mp/ecdsa_romapi.c
> index a2f63ff879f..082178ce83f 100644
> --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c
> +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c
> @@ -63,6 +63,7 @@ static int romapi_ecdsa_verify(struct udevice *dev,
>   			       const void *hash, size_t hash_len,
>   			       const void *signature, size_t sig_len)
>   {
> +	bool reenable_dcache = false;
>   	struct ecdsa_rom_api rom;
>   	uint8_t raw_key[64];
>   	uint32_t rom_ret;
> @@ -81,8 +82,21 @@ static int romapi_ecdsa_verify(struct udevice *dev,
>   	memcpy(raw_key + 32, pubkey->y, 32);
>   
>   	stm32mp_rom_get_ecdsa_functions(&rom);
> +
> +	/*
> +	 * Disable D-cache before calling into BootROM, else CRYP DMA
> +	 * may fail to pick up the correct data.
> +	 */
> +	if (dcache_status()) {
> +		dcache_disable();
> +		reenable_dcache = true;
> +	}
> +
>   	rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
>   
> +	if (reenable_dcache)
> +		dcache_enable();
> +
>   	return rom_ret == ROM_API_SUCCESS ? 0 : -EPERM;
>   }
>   


In fact, the ecdsa_verify_signature() don't use the HW (no DMA and no 
use of CRYP IP )

It is only a SW library, integrated in ROM code and exported to avoid 
the need

to include the same library in FSBL = TF-A, with size limitation (SYSRAM).


This library don't need to deactivate the data cache, the only impact of 
this deactivation it

is to reduce the execution performance....


After cross-check, I think the only problem today it the U-Boot MMU 
configuration of STM32MP15x

plaform: by default only the DDR is marked executable in U-Boot, all the 
other region are

defined as DEVICE memory/not executable (DCACHE_OFF in mmu_setup).


Deactivate the data cache only avoids the exception which occurs on jump 
to NotExecutable region

because in U-Boot "dcache OFF" imply  "MMU off"  (see cache_enable in 
./arch/arm/lib/cache-cp15.c)

and with MMU deactivated the check on executable MMU tag is also 
deactivated.


I think the next patch is enough:


#define STM32MP_ROM_BASE        U(0x00000000)


static int romapi_ecdsa_verify(struct udevice *dev,
  			       const void *hash, size_t hash_len,
  			       const void *signature, size_t sig_len)
  {
  	struct ecdsa_rom_api rom;
  	uint8_t raw_key[64];
  	uint32_t rom_ret;
@@ -81,8 +82,21 @@ static int romapi_ecdsa_verify(struct udevice *dev,
  	memcpy(raw_key + 32, pubkey->y, 32);
  
  	stm32mp_rom_get_ecdsa_functions(&rom);
+
+	/* mark executable the exported ROM code function: */
+	mmu_set_region_dcache_behaviour(STM32MP_ROM_BASE, MMU_SECTION_SIZE, DCACHE_DEFAULT_OPTION);
+
  	rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
  
  	return rom_ret == ROM_API_SUCCESS ? 0 : -EPERM;
  }


Sorry again for the first review, not complete...


Regards


Patrick



Reference in TF-A code: 
arm-trusted-firmware/plat/st/common/stm32mp_crypto_lib.c


uint32_t verify_signature(uint8_t *hash_in, uint8_t *pubkey_in,
               uint8_t *signature, uint32_t ecc_algo)
{
     int ret;

     ret = mmap_add_dynamic_region(STM32MP_ROM_BASE, STM32MP_ROM_BASE,
                       STM32MP_ROM_SIZE_2MB_ALIGNED, MT_CODE | MT_SECURE);

....
     ret = auth_ops.verify_signature(hash_in, pubkey_in, signature, 
ecc_algo);

....
     mmap_remove_dynamic_region(STM32MP_ROM_BASE, 
STM32MP_ROM_SIZE_2MB_ALIGNED);

     return ret;
}