[PATCH v2 1/4] ARM: stm32: Fix ECDSA authentication with Dcache enabled
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@denx.de --- Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@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; }
The STM32MP15xx platform currently comes with two incompatible implementations of save_boot_params() weak function override. Factor the save_boot_params() implementation into common cpu.c code and provide accessors to read out both ROM API table address and DT address from any place in the code instead.
Signed-off-by: Marek Vasut marex@denx.de --- Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com --- V2: Avoid #if CONFIG... , use if (CONFIG... instead --- arch/arm/mach-stm32mp/boot_params.c | 20 ++--------- arch/arm/mach-stm32mp/cpu.c | 35 +++++++++++++++++++ arch/arm/mach-stm32mp/ecdsa_romapi.c | 20 ++--------- .../arm/mach-stm32mp/include/mach/sys_proto.h | 3 ++ 4 files changed, 42 insertions(+), 36 deletions(-)
diff --git a/arch/arm/mach-stm32mp/boot_params.c b/arch/arm/mach-stm32mp/boot_params.c index e91ef1b2fc7..e40cca938ef 100644 --- a/arch/arm/mach-stm32mp/boot_params.c +++ b/arch/arm/mach-stm32mp/boot_params.c @@ -11,30 +11,14 @@ #include <asm/sections.h> #include <asm/system.h>
-/* - * Force data-section, as .bss will not be valid - * when save_boot_params is invoked. - */ -static unsigned long nt_fw_dtb __section(".data"); - -/* - * Save the FDT address provided by TF-A in r2 at boot time - * This function is called from start.S - */ -void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2, - unsigned long r3) -{ - nt_fw_dtb = r2; - - save_boot_params_ret(); -} - /* * Use the saved FDT address provided by TF-A at boot time (NT_FW_CONFIG = * Non Trusted Firmware configuration file) when the pointer is valid */ void *board_fdt_blob_setup(int *err) { + unsigned long nt_fw_dtb = get_stm32mp_bl2_dtb(); + log_debug("%s: nt_fw_dtb=%lx\n", __func__, nt_fw_dtb);
*err = 0; diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c index 855fc755fe0..ee59866bb73 100644 --- a/arch/arm/mach-stm32mp/cpu.c +++ b/arch/arm/mach-stm32mp/cpu.c @@ -378,3 +378,38 @@ int arch_misc_init(void)
return 0; } + +/* + * Without forcing the ".data" section, this would get saved in ".bss". BSS + * will be cleared soon after, so it's not suitable. + */ +static uintptr_t rom_api_table __section(".data"); +static uintptr_t nt_fw_dtb __section(".data"); + +/* + * The ROM gives us the API location in r0 when starting. This is only available + * during SPL, as there isn't (yet) a mechanism to pass this on to u-boot. Save + * the FDT address provided by TF-A in r2 at boot time. This function is called + * from start.S + */ +void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2, + unsigned long r3) +{ + if (IS_ENABLED(CONFIG_STM32_ECDSA_VERIFY)) + rom_api_table = r0; + + if (IS_ENABLED(CONFIG_TFABOOT)) + nt_fw_dtb = r2; + + save_boot_params_ret(); +} + +uintptr_t get_stm32mp_rom_api_table(void) +{ + return rom_api_table; +} + +uintptr_t get_stm32mp_bl2_dtb(void) +{ + return nt_fw_dtb; +} diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c b/arch/arm/mach-stm32mp/ecdsa_romapi.c index 082178ce83f..b1ab49165e7 100644 --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c @@ -24,26 +24,10 @@ struct ecdsa_rom_api { uint32_t ecc_algo); };
-/* - * Without forcing the ".data" section, this would get saved in ".bss". BSS - * will be cleared soon after, so it's not suitable. - */ -static uintptr_t rom_api_loc __section(".data"); - -/* - * The ROM gives us the API location in r0 when starting. This is only available - * during SPL, as there isn't (yet) a mechanism to pass this on to u-boot. - */ -void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2, - unsigned long r3) -{ - rom_api_loc = r0; - save_boot_params_ret(); -} - static void stm32mp_rom_get_ecdsa_functions(struct ecdsa_rom_api *rom) { - uintptr_t verify_ptr = rom_api_loc + ROM_API_OFFSET_ECDSA_VERIFY; + uintptr_t verify_ptr = get_stm32mp_rom_api_table() + + ROM_API_OFFSET_ECDSA_VERIFY;
rom->ecdsa_verify_signature = *(void **)verify_ptr; } diff --git a/arch/arm/mach-stm32mp/include/mach/sys_proto.h b/arch/arm/mach-stm32mp/include/mach/sys_proto.h index f19a70e53e0..0d39b67178e 100644 --- a/arch/arm/mach-stm32mp/include/mach/sys_proto.h +++ b/arch/arm/mach-stm32mp/include/mach/sys_proto.h @@ -77,3 +77,6 @@ void stm32mp_misc_init(void);
/* helper function: read data from OTP */ u32 get_otp(int index, int shift, int mask); + +uintptr_t get_stm32mp_rom_api_table(void); +uintptr_t get_stm32mp_bl2_dtb(void);
The ROM API table pointer is no longer accessible from U-Boot, fix this by passing the ROM API pointer through. This makes it possible for U-Boot to call ROM API functions to authenticate payload like signed fitImages.
Signed-off-by: Marek Vasut marex@denx.de --- Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com --- V2: - Rename image_entry_noargs_t to image_entry_stm32_t - Add missing __noreturn --- arch/arm/mach-stm32mp/cpu.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c index ee59866bb73..dc4112d5e6c 100644 --- a/arch/arm/mach-stm32mp/cpu.c +++ b/arch/arm/mach-stm32mp/cpu.c @@ -22,6 +22,7 @@ #include <dm/device.h> #include <dm/uclass.h> #include <linux/bitops.h> +#include <spl.h>
/* * early TLB into the .data section so that it not get cleared @@ -413,3 +414,17 @@ uintptr_t get_stm32mp_bl2_dtb(void) { return nt_fw_dtb; } + +#ifdef CONFIG_SPL_BUILD +void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) +{ + typedef void __noreturn (*image_entry_stm32_t)(u32 romapi); + uintptr_t romapi = get_stm32mp_rom_api_table(); + + image_entry_stm32_t image_entry = + (image_entry_stm32_t)spl_image->entry_point; + + printf("image entry point: 0x%lx\n", spl_image->entry_point); + image_entry(romapi); +} +#endif
With U-Boot having access to ROM API call table, it is possible to use the ROM API call it authenticate e.g. signed kernel fitImages using the BootROM ECDSA support. Make this available by pulling the ECDSA BootROM call support from SPL-only guard.
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com Signed-off-by: Marek Vasut marex@denx.de --- Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com --- V2: Add RB from Patrice and Patrick --- arch/arm/mach-stm32mp/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-stm32mp/Makefile b/arch/arm/mach-stm32mp/Makefile index 1db9057e049..a19b2797c8b 100644 --- a/arch/arm/mach-stm32mp/Makefile +++ b/arch/arm/mach-stm32mp/Makefile @@ -11,10 +11,10 @@ obj-y += bsec.o obj-$(CONFIG_STM32MP13x) += stm32mp13x.o obj-$(CONFIG_STM32MP15x) += stm32mp15x.o
+obj-$(CONFIG_STM32_ECDSA_VERIFY) += ecdsa_romapi.o ifdef CONFIG_SPL_BUILD obj-y += spl.o obj-y += tzc400.o -obj-$(CONFIG_STM32_ECDSA_VERIFY) += ecdsa_romapi.o else obj-y += cmd_stm32prog/ obj-$(CONFIG_CMD_STM32KEY) += cmd_stm32key.o
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@denx.de
Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@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; }
On 12/7/22 11:08, Patrick DELAUNAY wrote:
Hi Marek,
Hello Patrick,
Sorry for the delay.
No worries.
I cross-check with ROM code team to understood this API limitation.
Thank you!
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@denx.de
Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@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 )
Hmmm, what does the BootROM use CRYP for then ? It is necessary to have MP15xC/F for the authenticated boot to work, but it seems the only difference there is the presence of CRYP. Or is there some BootROM fuse too ?
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; }
This indeed works, tested and sent V3.
Sorry again for the first review, not complete...
Thank you for checking !
Hi,
On 12/7/22 20:32, Marek Vasut wrote:
On 12/7/22 11:08, Patrick DELAUNAY wrote:
Hi Marek,
Hello Patrick,
Sorry for the delay.
No worries.
I cross-check with ROM code team to understood this API limitation.
Thank you!
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@denx.de
Cc: Alexandru Gagniuc mr.nuke.me@gmail.com Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@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 )
Hmmm, what does the BootROM use CRYP for then ?
used for SSP = Secure Secret Provisioning
https://wiki.st.com/stm32mpu/wiki/Secure_Secret_Provisioning_(SSP)
It is necessary to have MP15xC/F for the authenticated boot to work, but it seems the only difference there is the presence of CRYP. Or is there some BootROM fuse too ?
Yes, the secure boot feature availability is indicated in the security field of the chip part number, for STM32MP13 and STM32MP15.
- SSP is not supported
- the associated authentication feature for secure boot is deactivated in ROM code
=> the key is burned/locked in OTP on these chips
and checked by ROM code before to authenticate the FSBL
...
This indeed works, tested and sent V3.
Sorry again for the first review, not complete...
Thank you for checking !
Regards
Patrick
On 12/12/22 10:40, Patrick DELAUNAY wrote:
Hi,
Hello Patrick
[...]
Hmmm, what does the BootROM use CRYP for then ?
used for SSP = Secure Secret Provisioning
https://wiki.st.com/stm32mpu/wiki/Secure_Secret_Provisioning_(SSP)
Oh, only this part, I see.
It is necessary to have MP15xC/F for the authenticated boot to work, but it seems the only difference there is the presence of CRYP. Or is there some BootROM fuse too ?
Yes, the secure boot feature availability is indicated in the security field of the chip part number, for STM32MP13 and STM32MP15.
SSP is not supported
the associated authentication feature for secure boot is deactivated
in ROM code
=> the key is burned/locked in OTP on these chips
and checked by ROM code before to authenticate the FSBL
Thank you for the clarification, this is really useful.
participants (2)
-
Marek Vasut -
Patrick DELAUNAY