[PATCH v4 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 MMU tables are set up and the BootROM region is not marked as executable, so an attempt to run code from it results in a hang. Mark the BootROM region as executable as suggested by Patrick to prevent the hang.
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: - Initialize reenable_dcache variable V3: - Mark BootROM as executable instead V4: - Add RB from Patrick --- arch/arm/mach-stm32mp/ecdsa_romapi.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c b/arch/arm/mach-stm32mp/ecdsa_romapi.c index a2f63ff879f..6156526253c 100644 --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c @@ -81,6 +81,10 @@ static int romapi_ecdsa_verify(struct udevice *dev, memcpy(raw_key + 32, pubkey->y, 32);
stm32mp_rom_get_ecdsa_functions(&rom); + + /* Mark BootROM region as executable. */ + mmu_set_region_dcache_behaviour(0, SZ_2M, DCACHE_DEFAULT_OPTION); + rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
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.
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: Avoid #if CONFIG... , use if (CONFIG... instead V3: No change V4: - Add RB from Patrick - Include sys_proto.h to avoid undefined symbol warning --- arch/arm/mach-stm32mp/boot_params.c | 21 ++--------- 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, 43 insertions(+), 36 deletions(-)
diff --git a/arch/arm/mach-stm32mp/boot_params.c b/arch/arm/mach-stm32mp/boot_params.c index e91ef1b2fc7..24d04dcf0f9 100644 --- a/arch/arm/mach-stm32mp/boot_params.c +++ b/arch/arm/mach-stm32mp/boot_params.c @@ -8,33 +8,18 @@ #include <common.h> #include <log.h> #include <linux/libfdt.h> +#include <asm/arch/sys_proto.h> #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 6156526253c..12b42b9d59c 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);

On 1/12/23 18:58, Marek Vasut wrote:
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.
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: Avoid #if CONFIG... , use if (CONFIG... instead V3: No change V4: - Add RB from Patrick - Include sys_proto.h to avoid undefined symbol warning
arch/arm/mach-stm32mp/boot_params.c | 21 ++--------- 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, 43 insertions(+), 36 deletions(-)
diff --git a/arch/arm/mach-stm32mp/boot_params.c b/arch/arm/mach-stm32mp/boot_params.c index e91ef1b2fc7..24d04dcf0f9 100644 --- a/arch/arm/mach-stm32mp/boot_params.c +++ b/arch/arm/mach-stm32mp/boot_params.c @@ -8,33 +8,18 @@ #include <common.h> #include <log.h> #include <linux/libfdt.h> +#include <asm/arch/sys_proto.h> #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 6156526253c..12b42b9d59c 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);
Applied to u-boot-stm/master
Thanks Patrice

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.
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: - Rename image_entry_noargs_t to image_entry_stm32_t - Add missing __noreturn V3: No change V4: Add RB from Patrick --- 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

On 1/12/23 18:58, Marek Vasut wrote:
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.
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: - Rename image_entry_noargs_t to image_entry_stm32_t - Add missing __noreturn V3: No change V4: Add RB from Patrick
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
Applied to u-boot-stm/master
Thanks Patrice

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 V3: No change V4: No change --- 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

On 1/12/23 18:58, Marek Vasut wrote:
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 V3: No change V4: No change
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
Applied to u-boot-stm/master
Thanks Patrice

Hi Marek
On 1/12/23 18:58, Marek Vasut wrote:
In case Dcache is enabled while the ECDSA authentication function is called via BootROM ROM API, the MMU tables are set up and the BootROM region is not marked as executable, so an attempt to run code from it results in a hang. Mark the BootROM region as executable as suggested by Patrick to prevent the hang.
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: - Initialize reenable_dcache variable V3: - Mark BootROM as executable instead V4: - Add RB from Patrick
arch/arm/mach-stm32mp/ecdsa_romapi.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c b/arch/arm/mach-stm32mp/ecdsa_romapi.c index a2f63ff879f..6156526253c 100644 --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c @@ -81,6 +81,10 @@ static int romapi_ecdsa_verify(struct udevice *dev, memcpy(raw_key + 32, pubkey->y, 32);
stm32mp_rom_get_ecdsa_functions(&rom);
/* Mark BootROM region as executable. */
mmu_set_region_dcache_behaviour(0, SZ_2M, DCACHE_DEFAULT_OPTION);
rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
return rom_ret == ROM_API_SUCCESS ? 0 : -EPERM;
Applied to u-boot-stm/master
Thanks Patrice
participants (2)
-
Marek Vasut
-
Patrice CHOTARD