From 6719eeb376a187fa45c651cc8ad2d560c1420aa1 Mon Sep 17 00:00:00 2001 From: cepetr Date: Wed, 29 Jan 2025 21:34:20 +0100 Subject: [PATCH] refactor(core): simplify shutdown/handover code [no changelog] --- .../io/display/ltdc_dsi/display_driver.c | 9 -- core/embed/projects/boardloader/main.c | 1 - core/embed/projects/bootloader/main.c | 1 - core/embed/projects/kernel/main.c | 12 +- core/embed/sys/startup/inc/sys/bootutils.h | 22 +-- core/embed/sys/startup/inc/sys/sysutils.h | 14 ++ core/embed/sys/startup/stm32/bootutils.c | 145 +++++++++++------- core/embed/sys/startup/stm32/sysutils.c | 56 ++++++- core/embed/sys/startup/stm32f4/limited_util.S | 105 ------------- core/embed/sys/startup/stm32f4/startup_init.c | 20 --- core/embed/sys/startup/stm32f4/util.S | 120 --------------- core/embed/sys/startup/stm32u5/limited_util.S | 116 -------------- core/embed/sys/startup/stm32u5/util.S | 120 --------------- core/embed/sys/startup/unix/bootutils.c | 18 ++- core/embed/sys/task/stm32/systask.c | 5 +- core/embed/sys/task/stm32/system.c | 4 +- core/embed/sys/task/unix/system.c | 14 +- core/embed/util/rsod/rsod.c | 4 +- core/site_scons/models/stm32f4_common.py | 12 -- core/site_scons/models/stm32u5_common.py | 12 -- 20 files changed, 196 insertions(+), 614 deletions(-) delete mode 100644 core/embed/sys/startup/stm32f4/limited_util.S delete mode 100644 core/embed/sys/startup/stm32f4/util.S delete mode 100644 core/embed/sys/startup/stm32u5/limited_util.S delete mode 100644 core/embed/sys/startup/stm32u5/util.S diff --git a/core/embed/io/display/ltdc_dsi/display_driver.c b/core/embed/io/display/ltdc_dsi/display_driver.c index f1439ccbcf..717ecd0fdb 100644 --- a/core/embed/io/display/ltdc_dsi/display_driver.c +++ b/core/embed/io/display/ltdc_dsi/display_driver.c @@ -427,15 +427,6 @@ cleanup: void display_deinit(display_content_mode_t mode) { display_driver_t *drv = &g_display_driver; - if (mode == DISPLAY_RETAIN_CONTENT) { - // This is a temporary workaround for T3W1 to avoid clearing - // the display after drawing RSOD screen in `secure_shutdown()` - // function. The workaround should be removed once we have - // proper replacement for `secure_shutdown()` that resets the - // device instead of waiting for manual power off. - return; - } - GPIO_InitTypeDef GPIO_InitStructure = {0}; gfx_bitblt_deinit(); diff --git a/core/embed/projects/boardloader/main.c b/core/embed/projects/boardloader/main.c index 38c8750950..bcfa3446cb 100644 --- a/core/embed/projects/boardloader/main.c +++ b/core/embed/projects/boardloader/main.c @@ -106,7 +106,6 @@ static void drivers_deinit(void) { #ifdef USE_POWERCTL powerctl_deinit(); #endif - ensure_compatible_settings(); } static uint8_t get_bootloader_min_version(void) { diff --git a/core/embed/projects/bootloader/main.c b/core/embed/projects/bootloader/main.c index 9c6eb465b5..1a65b6796f 100644 --- a/core/embed/projects/bootloader/main.c +++ b/core/embed/projects/bootloader/main.c @@ -141,7 +141,6 @@ static void drivers_deinit(void) { #endif #endif display_deinit(DISPLAY_JUMP_BEHAVIOR); - ensure_compatible_settings(); } static void usb_init_all(secbool usb21_landing) { diff --git a/core/embed/projects/kernel/main.c b/core/embed/projects/kernel/main.c index cf241746b6..146652b240 100644 --- a/core/embed/projects/kernel/main.c +++ b/core/embed/projects/kernel/main.c @@ -220,8 +220,8 @@ static void show_rsod(const systask_postmortem_t *pminfo) { applet_run(&coreapp); if (coreapp.task.pminfo.reason == TASK_TERM_REASON_EXIT) { - // If the RSOD was shown successfully, proceed to shutdown - secure_shutdown(); + // RSOD was shown successfully + return; } } #endif @@ -243,8 +243,8 @@ static void init_and_show_rsod(const systask_postmortem_t *pminfo) { // Show RSOD show_rsod(pminfo); - // Wait for the user to manually power off the device - secure_shutdown(); + // Reboots or halts (if RSOD_INFINITE_LOOP is defined) + reboot_or_halt_after_rsod(); } // Kernel panic handler @@ -283,8 +283,8 @@ int main(void) { // Coreapp crashed, show RSOD show_rsod(&coreapp.task.pminfo); - // Wait for the user to manually power off the device - secure_shutdown(); + // Reboots or halts (if RSOD_INFINITE_LOOP is defined) + reboot_or_halt_after_rsod(); return 0; } diff --git a/core/embed/sys/startup/inc/sys/bootutils.h b/core/embed/sys/startup/inc/sys/bootutils.h index 53409dfc87..a32abe1ad4 100644 --- a/core/embed/sys/startup/inc/sys/bootutils.h +++ b/core/embed/sys/startup/inc/sys/bootutils.h @@ -38,26 +38,20 @@ void __attribute__((noreturn)) reboot_to_bootloader(void); // with the firmware installation. void __attribute__((noreturn)) reboot_and_upgrade(const uint8_t hash[32]); -// Allows the user to see the displayed error message and then -// safely shuts down the device (clears secrets, memory, etc.). +// Allows the user to read the displayed error message and then +// reboots the device or waits for power-off. // -// This function is called when the device enters an -// unrecoverable error state. -void __attribute__((noreturn)) secure_shutdown(void); +// The function's behavior depends on the `RSOD_INFINITE_LOOP` macro: +// 1) If `RSOD_INFINITE_LOOP` is defined, the function enters an infinite loop. +// 2) If `RSOD_INFINITE_LOOP` is not defined, the function waits for a +// specified duration and then resets the device. +void __attribute__((noreturn)) reboot_or_halt_after_rsod(void); // Jumps to the next booting stage (e.g. bootloader to firmware). // `vectbl_address` points to the flash at the vector table of the next stage. // // Before jumping, the function disables all interrupts and clears the // memory and registers that could contain sensitive information. -void jump_to_next_stage(uint32_t vectbl_address); - -// Ensure compatible hardware settings before jumping to -// the different booting stage. This function is used to -// ensure backward compatibility with older versions of -// released bootloaders and firmware. -// -// Does nothing on almost all platforms. -void ensure_compatible_settings(void); +void __attribute__((noreturn)) jump_to_next_stage(uint32_t vectbl_address); #endif // TREZORHAL_BOOTUTILS_H diff --git a/core/embed/sys/startup/inc/sys/sysutils.h b/core/embed/sys/startup/inc/sys/sysutils.h index c38615b71e..c3bba5f6bd 100644 --- a/core/embed/sys/startup/inc/sys/sysutils.h +++ b/core/embed/sys/startup/inc/sys/sysutils.h @@ -40,7 +40,21 @@ __attribute((noreturn)) void call_with_new_stack(uint32_t arg1, uint32_t arg2, // if so, it switches to privileged thread mode. void ensure_thread_mode(void); +// Ensure compatible hardware settings before jumping to +// the different booting stage. This function is used to +// ensure backward compatibility with older versions of +// released bootloaders and firmware. +// +// Does nothing on almost all platforms. +void ensure_compatible_settings(void); + // Clears USB peripheral fifo memory void clear_otg_hs_memory(void); +// Jumps to the binary using its vector table. +// +// The target binary is called with interrupts disabled, and all registers +// are cleared except R11, which is set to the specified value. +__attribute((noreturn)) void jump_to_vectbl(uint32_t vectbl_addr, uint32_t r11); + #endif // KERNEL_MODE diff --git a/core/embed/sys/startup/stm32/bootutils.c b/core/embed/sys/startup/stm32/bootutils.c index 3cde2ced9c..5d79ff7fce 100644 --- a/core/embed/sys/startup/stm32/bootutils.c +++ b/core/embed/sys/startup/stm32/bootutils.c @@ -25,16 +25,20 @@ #include #include #include +#include #include +#include #include #include -#ifdef TREZOR_MODEL_T2T1 -#include "../stm32f4/startup_init.h" -#endif - #ifdef KERNEL_MODE +// Battery powered devices (USE_POWERCTL) should not stall +// after showing RSOD, as it would drain the battery. +#ifndef USE_POWERCTL +#define RSOD_INFINITE_LOOP +#endif + #ifdef STM32U5 // Persistent variable that holds the 'command' for the next reboot. boot_command_t __attribute__((section(".boot_command"))) g_boot_command; @@ -95,81 +99,106 @@ void bootargs_init(uint32_t r11_register) { } #endif -// Deletes all secrets and SRAM2 where stack is located -// to prevent stack smashing error, do not return from function calling this -#ifdef STM32U5 -static inline void __attribute__((always_inline)) delete_secrets(void) { - __disable_irq(); +static void reboot_with_args_phase_2(uint32_t arg1, uint32_t arg2) { + // We are now running on a new stack. We cannot be sure about + // any variables in the .bss and .data sections, so we must + // be careful and avoid using them altogether. - // Disable SAES peripheral clock, so that we don't get tamper events - __HAL_RCC_SAES_CLK_DISABLE(); + // Clear unused part of stack + clear_unused_stack(); - TAMP->CR2 |= TAMP_CR2_BKERASE; -} -#endif // STM32U5 + // Clear all memory except stack and bootargs + memregion_t region = MEMREGION_ALL_ACCESSIBLE_RAM; + MEMREGION_DEL_SECTION(®ion, _stack_section); + MEMREGION_DEL_SECTION(®ion, _bootargs_ram); + memregion_fill(®ion, 0); -// Reboots the device with the given boot command and arguments -static void __attribute__((noreturn)) -reboot_with_args(boot_command_t command, const void* args, size_t args_size) { - bootargs_set(command, args, args_size); - -#ifdef STM32U5 - delete_secrets(); +#if defined STM32U5 NVIC_SystemReset(); +#elif defined STM32F4 + clear_otg_hs_memory(); + jump_to_vectbl(BOOTLOADER_START + IMAGE_HEADER_SIZE, arg1); #else - display_deinit(DISPLAY_RESET_CONTENT); - ensure_compatible_settings(); - - mpu_reconfig(MPU_MODE_DISABLED); - - ensure_thread_mode(); - - // from util.s - extern void jump_to_with_flag(uint32_t address, uint32_t reset_flag); - jump_to_with_flag(BOOTLOADER_START + IMAGE_HEADER_SIZE, g_boot_command); - for (;;) - ; +#error Unsupported platform #endif } -void reboot_to_bootloader(void) { +// Reboots the device with the given boot command and arguments +__attribute__((noreturn)) static void reboot_with_args(boot_command_t command, + const void* args, + size_t args_size) { + // Set bootargs area to the new command and arguments + bootargs_set(command, args, args_size); + +#ifdef STM32F4 + // We are going to jump directly to the bootloader, so we need to + // ensure that the device is in a compatible state. Following lines + // ensure the display is properly deinitialized, CPU frequency is + // properly set and we are running in privileged thread mode. + display_deinit(DISPLAY_RESET_CONTENT); + ensure_compatible_settings(); + ensure_thread_mode(); +#endif + + // Disable interrupts, MPU, clear all registers and set up a new stack + // (on STM32U5 it also clear all CPU secrets and SRAM2). + call_with_new_stack(command, 0, reboot_with_args_phase_2); +} + +__attribute__((noreturn)) void reboot_to_bootloader(void) { reboot_with_args(BOOT_COMMAND_STOP_AND_WAIT, NULL, 0); } -void reboot_and_upgrade(const uint8_t hash[32]) { +__attribute__((noreturn)) void reboot_and_upgrade(const uint8_t hash[32]) { reboot_with_args(BOOT_COMMAND_INSTALL_UPGRADE, hash, 32); } -void reboot_device(void) { - bootargs_set(BOOT_COMMAND_NONE, NULL, 0); - -#ifdef STM32U5 - delete_secrets(); -#endif - - NVIC_SystemReset(); +__attribute__((noreturn)) void reboot_device(void) { + reboot_with_args(BOOT_COMMAND_NONE, NULL, 0); } -void __attribute__((noreturn)) secure_shutdown(void) { - display_deinit(DISPLAY_RETAIN_CONTENT); - -#ifdef STM32U5 - delete_secrets(); +__attribute__((noreturn)) void reboot_or_halt_after_rsod(void) { +#ifndef RSOD_INFINITE_LOOP + systick_delay_ms(10 * 1000); #endif - // from util.s - extern void shutdown_privileged(void); - shutdown_privileged(); - - for (;;) +#ifdef RSOD_INFINITE_LOOP + while (true) ; +#else + reboot_device(); +#endif } -void ensure_compatible_settings(void) { -#ifdef TREZOR_MODEL_T2T1 - // Early version of bootloader on T2T1 expects 168 MHz core clock. - // So we need to set it here before handover to the bootloader. - set_core_clock(CLOCK_168_MHZ); +static void jump_to_next_stage_phase_2(uint32_t arg1, uint32_t arg2) { + // We are now running on a new stack. We cannot be sure about + // any variables in the .bss and .data sections, so we must + // be careful and avoid using them altogether. + + // Clear unused part of stack + clear_unused_stack(); + + // Clear all memory except stack and bootargs + memregion_t region = MEMREGION_ALL_ACCESSIBLE_RAM; + MEMREGION_DEL_SECTION(®ion, _stack_section); + MEMREGION_DEL_SECTION(®ion, _bootargs_ram); + memregion_fill(®ion, 0); + + // Jump to reset vector of the next stage + jump_to_vectbl(arg1, 0); +} + +void __attribute__((noreturn)) jump_to_next_stage(uint32_t vectbl_address) { +#ifdef STM32F4 + // Ensure the display is properly deinitialized, CPU frequency is + // properly set. It's needed for backward compatibility with the older + // firmware. + display_deinit(DISPLAY_JUMP_BEHAVIOR); + ensure_compatible_settings(); #endif + + // Disable interrupts, MPU, clear all registers and set up a new stack + // (on STM32U5 it also clear all CPU secrets and SRAM2). + call_with_new_stack(vectbl_address, 0, jump_to_next_stage_phase_2); } #endif // KERNEL_MODE diff --git a/core/embed/sys/startup/stm32/sysutils.c b/core/embed/sys/startup/stm32/sysutils.c index 334a43a462..2e493ceddb 100644 --- a/core/embed/sys/startup/stm32/sysutils.c +++ b/core/embed/sys/startup/stm32/sysutils.c @@ -24,6 +24,10 @@ #ifdef KERNEL_MODE +#ifdef TREZOR_MODEL_T2T1 +#include "../stm32f4/startup_init.h" +#endif + __attribute((naked, noreturn, no_stack_protector)) void call_with_new_stack( uint32_t arg1, uint32_t arg2, new_stack_callback_t callback) { __asm__ volatile( @@ -187,14 +191,13 @@ __attribute((naked, no_stack_protector)) void ensure_thread_mode(void) { "BX LR \n"); } - // Clears USB FIFO memory to prevent data leakage of sensitive information __attribute((used)) void clear_otg_hs_memory(void) { #ifdef STM32F4 - // reference RM0090 section 35.12.1 Figure 413 - #define USB_OTG_HS_DATA_FIFO_RAM (USB_OTG_HS_PERIPH_BASE + 0x20000U) - #define USB_OTG_HS_DATA_FIFO_SIZE (4096U) +// reference RM0090 section 35.12.1 Figure 413 +#define USB_OTG_HS_DATA_FIFO_RAM (USB_OTG_HS_PERIPH_BASE + 0x20000U) +#define USB_OTG_HS_DATA_FIFO_SIZE (4096U) // use the HAL version due to section 2.1.6 of STM32F42xx Errata sheet __HAL_RCC_USB_OTG_HS_CLK_ENABLE(); // enable USB_OTG_HS peripheral clock so @@ -211,5 +214,50 @@ __attribute((used)) void clear_otg_hs_memory(void) { #endif } +void ensure_compatible_settings(void) { +#ifdef TREZOR_MODEL_T2T1 + // Early version of bootloader on T2T1 expects 168 MHz core clock. + // So we need to set it here before handover to the bootloader. + set_core_clock(CLOCK_168_MHZ); +#endif +} + +__attribute((naked, noreturn, no_stack_protector)) void jump_to_vectbl( + uint32_t vectbl_addr, uint32_t r11) { + __asm__ volatile( + "CPSID F \n" + + "MOV R11, R1 \n" + "MOV LR, R0 \n" + + "LDR R0, =0 \n" + "MOV R1, R0 \n" + "MOV R2, R0 \n" + "MOV R3, R0 \n" + "MOV R4, R0 \n" + "MOV R5, R0 \n" + "MOV R6, R0 \n" + "MOV R7, R0 \n" + "MOV R8, R0 \n" + "MOV R9, R0 \n" + "MOV R10, R0 \n" // R11 is set to r11 argument + "MOV R12, R0 \n" + + "LDR R0, [LR] \n" // Initial MSP value + "MSR MSP, R0 \n" // Set MSP + + "LDR R0, =%[_SCB_VTOR] \n" // Reset handler + "STR LR, [R0] \n" // Set SCB->VTOR = vectb_addr + + "MOV R0, R1 \n" // Zero out R0 + + "LDR LR, [LR, #4] \n" // Reset handler + "BX LR \n" // Go to reset handler + + : // no output + : [_SCB_VTOR] "i"(&SCB->VTOR) + : // no clobber + ); +} #endif // KERNEL_MODE diff --git a/core/embed/sys/startup/stm32f4/limited_util.S b/core/embed/sys/startup/stm32f4/limited_util.S deleted file mode 100644 index ef599f1a7d..0000000000 --- a/core/embed/sys/startup/stm32f4/limited_util.S +++ /dev/null @@ -1,105 +0,0 @@ - .syntax unified - - .text - - .global memset_reg - .type memset_reg, STT_FUNC -memset_reg: - // call with the following (note that the arguments are not validated prior to use): - // r0 - address of first word to write (inclusive) - // r1 - address of first word following the address in r0 to NOT write (exclusive) - // r2 - word value to be written - // both addresses in r0 and r1 needs to be divisible by 4! - cmp r0, r1 - beq .L_loop_end - .L_loop_begin: - str r2, [r0], 4 // store the word in r2 to the address in r0, post-indexed - cmp r0, r1 - bne .L_loop_begin - .L_loop_end: - bx lr - - .global jump_to_next_stage - .type jump_to_next_stage, STT_FUNC -jump_to_next_stage: - mov r4, r0 // save input argument r0 (the address of the next stage's vector table) (r4 is callee save) - // this subroutine re-points the exception handlers before the C code - // that comprises them has been given a good environment to run. - // therefore, this code needs to disable interrupts before the VTOR - // update. then, the reset_handler of the next stage needs to re-enable interrupts. - // the following prevents activation of all exceptions except Non-Maskable Interrupt (NMI). - // according to "ARM Cortex-M Programming Guide to Memory Barrier Instructions" Application Note 321, section 4.8: - // "there is no requirement to insert memory barrier instructions after CPSID". - cpsid f - // wipe memory at the end of the current stage of code - bl clear_otg_hs_memory - ldr r2, =0 // r2 - the word-sized value to be written - ldr r0, =_handoff_clear_ram_0_start - ldr r1, =_handoff_clear_ram_0_end - bl memset_reg - ldr r0, =_handoff_clear_ram_1_start - ldr r1, =_handoff_clear_ram_1_end - bl memset_reg - mov lr, r4 - // clear out the general purpose registers before the next stage's code can run (even the NMI exception handler) - ldr r0, =0 - mov r1, r0 - mov r2, r0 - mov r3, r0 - mov r4, r0 - mov r5, r0 - mov r6, r0 - mov r7, r0 - mov r8, r0 - mov r9, r0 - mov r10, r0 - mov r11, r0 - mov r12, r0 - // give the next stage a fresh main stack pointer - ldr r0, [lr] // set r0 to the main stack pointer in the next stage's vector table - msr msp, r0 // give the next stage its main stack pointer - // point to the next stage's exception handlers - // AN321, section 4.11: "a memory barrier is not required after a VTOR update" - .set SCB_VTOR, 0xE000ED08 // reference "Cortex-M4 Devices Generic User Guide" section 4.3 - ldr r0, =SCB_VTOR - str lr, [r0] - mov r0, r1 // zero out r0 - // go on to the next stage - ldr lr, [lr, 4] // set lr to the next stage's reset_handler - bx lr - - .global shutdown_privileged - .type shutdown_privileged, STT_FUNC - // The function must be called from the privileged mode -shutdown_privileged: - cpsid f // disable all exceptions (except for NMI), the instruction is ignored in unprivileged mode - // if the exceptions weren't disabled, an exception handler (for example systick handler) - // could be called after the memory is erased, which would lead to another exception - ldr r0, =0 - mov r1, r0 - mov r2, r0 - mov r3, r0 - mov r4, r0 - mov r5, r0 - mov r6, r0 - mov r7, r0 - mov r8, r0 - mov r9, r0 - mov r10, r0 - mov r11, r0 - mov r12, r0 - ldr lr, =0xffffffff - - ldr r0, =_shutdown_clear_ram_0_start - ldr r1, =_shutdown_clear_ram_0_end - bl memset_reg - ldr r0, =_shutdown_clear_ram_1_start - ldr r1, =_shutdown_clear_ram_1_end - bl memset_reg - bl clear_otg_hs_memory - ldr r0, =1 - msr control, r0 // jump to unprivileged mode - ldr r0, =0 - b . // loop forever - - .end diff --git a/core/embed/sys/startup/stm32f4/startup_init.c b/core/embed/sys/startup/stm32f4/startup_init.c index 040f87a857..b6429a76c0 100644 --- a/core/embed/sys/startup/stm32f4/startup_init.c +++ b/core/embed/sys/startup/stm32f4/startup_init.c @@ -235,26 +235,6 @@ void set_core_clock(clock_settings_t settings) { } #endif -// reference RM0090 section 35.12.1 Figure 413 -#define USB_OTG_HS_DATA_FIFO_RAM (USB_OTG_HS_PERIPH_BASE + 0x20000U) -#define USB_OTG_HS_DATA_FIFO_SIZE (4096U) - -// Clears USB FIFO memory to prevent data leakage of sensitive information -__attribute((used)) void clear_otg_hs_memory(void) { - // use the HAL version due to section 2.1.6 of STM32F42xx Errata sheet - __HAL_RCC_USB_OTG_HS_CLK_ENABLE(); // enable USB_OTG_HS peripheral clock so - // that the peripheral memory is - // accessible - __IO uint32_t* usb_fifo_ram = (__IO uint32_t*)USB_OTG_HS_DATA_FIFO_RAM; - - for (uint32_t i = 0; i < USB_OTG_HS_DATA_FIFO_SIZE / 4; i++) { - usb_fifo_ram[i] = 0; - } - - __HAL_RCC_USB_OTG_HS_CLK_DISABLE(); // disable USB OTG_HS peripheral clock as - // the peripheral is not needed right now -} - __attribute((no_stack_protector)) void reset_handler(void) { #ifdef BOOTLOADER uint32_t r11_value; diff --git a/core/embed/sys/startup/stm32f4/util.S b/core/embed/sys/startup/stm32f4/util.S deleted file mode 100644 index 893e949238..0000000000 --- a/core/embed/sys/startup/stm32f4/util.S +++ /dev/null @@ -1,120 +0,0 @@ - .syntax unified - - .text - -#ifdef KERNEL_MODE - - .global memset_reg - .type memset_reg, STT_FUNC -memset_reg: - // call with the following (note that the arguments are not validated prior to use): - // r0 - address of first word to write (inclusive) - // r1 - address of first word following the address in r0 to NOT write (exclusive) - // r2 - word value to be written - // both addresses in r0 and r1 needs to be divisible by 4! - cmp r0, r1 - beq .L_loop_end - .L_loop_begin: - str r2, [r0], 4 // store the word in r2 to the address in r0, post-indexed - cmp r0, r1 - bne .L_loop_begin - .L_loop_end: - bx lr - - // Jump to address given in first argument R0 that points to next's stage's VTOR - // Clear memory and all registers before jump - .global jump_to_next_stage - .type jump_to_next_stage, STT_FUNC -jump_to_next_stage: - ldr r1, =0 - bl jump_to_with_flag - - // Jump to address given in first argument R0 that points to next's stage's VTOR - // Clear memory and all registers before jump. Second argument R1 is copied to R11 - // and kept after jump. - .global jump_to_with_flag - .type jump_to_with_flag, STT_FUNC -jump_to_with_flag: - mov r4, r0 // save input argument r0 (the address of the next stage's vector table) (r4 is callee save) - mov r11, r1 // save second argument in "flag" register because we'll be cleaning RAM - // this subroutine re-points the exception handlers before the C code - // that comprises them has been given a good environment to run. - // therefore, this code needs to disable interrupts before the VTOR - // update. then, the reset_handler of the next stage needs to re-enable interrupts. - // the following prevents activation of all exceptions except Non-Maskable Interrupt (NMI). - // according to "ARM Cortex-M Programming Guide to Memory Barrier Instructions" Application Note 321, section 4.8: - // "there is no requirement to insert memory barrier instructions after CPSID". - cpsid f - // wipe memory at the end of the current stage of code - bl clear_otg_hs_memory - ldr r2, =0 // r2 - the word-sized value to be written - ldr r0, =_handoff_clear_ram_0_start - ldr r1, =_handoff_clear_ram_0_end - bl memset_reg - ldr r0, =_handoff_clear_ram_1_start - ldr r1, =_handoff_clear_ram_1_end - bl memset_reg - mov lr, r4 - // clear out the general purpose registers before the next stage's except the register with flag R11 - ldr r0, =0 - mov r1, r0 - mov r2, r0 - mov r3, r0 - mov r4, r0 - mov r5, r0 - mov r6, r0 - mov r7, r0 - mov r8, r0 - mov r9, r0 - mov r10, r0 - mov r12, r0 - // give the next stage a fresh main stack pointer - ldr r0, [lr] // set r0 to the main stack pointer in the next stage's vector table - msr msp, r0 // give the next stage its main stack pointer - // point to the next stage's exception handlers - // AN321, section 4.11: "a memory barrier is not required after a VTOR update" - .set SCB_VTOR, 0xE000ED08 // reference "Cortex-M4 Devices Generic User Guide" section 4.3 - ldr r0, =SCB_VTOR - str lr, [r0] - mov r0, r1 // zero out r0 - // go on to the next stage - ldr lr, [lr, 4] // set lr to the next stage's reset_handler - bx lr - - .global shutdown_privileged - .type shutdown_privileged, STT_FUNC - // The function must be called from the privileged mode -shutdown_privileged: - cpsid f // disable all exceptions (except for NMI), the instruction is ignored in unprivileged mode - // if the exceptions weren't disabled, an exception handler (for example systick handler) - // could be called after the memory is erased, which would lead to another exception - ldr r0, =0 - mov r1, r0 - mov r2, r0 - mov r3, r0 - mov r4, r0 - mov r5, r0 - mov r6, r0 - mov r7, r0 - mov r8, r0 - mov r9, r0 - mov r10, r0 - mov r11, r0 - mov r12, r0 - ldr lr, =0xffffffff - - ldr r0, =_shutdown_clear_ram_0_start - ldr r1, =_shutdown_clear_ram_0_end - bl memset_reg - ldr r0, =_shutdown_clear_ram_1_start - ldr r1, =_shutdown_clear_ram_1_end - bl memset_reg - bl clear_otg_hs_memory - ldr r0, =1 - msr control, r0 // jump to unprivileged mode - ldr r0, =0 - b . // loop forever - -#endif - - .end diff --git a/core/embed/sys/startup/stm32u5/limited_util.S b/core/embed/sys/startup/stm32u5/limited_util.S deleted file mode 100644 index 7e7e218cf5..0000000000 --- a/core/embed/sys/startup/stm32u5/limited_util.S +++ /dev/null @@ -1,116 +0,0 @@ - .syntax unified - - .text - - .global memset_reg - .type memset_reg, STT_FUNC -memset_reg: - // call with the following (note that the arguments are not validated prior to use): - // r0 - address of first word to write (inclusive) - // r1 - address of first word following the address in r0 to NOT write (exclusive) - // r2 - word value to be written - // both addresses in r0 and r1 needs to be divisible by 4! - cmp r0, r1 - beq .L_loop_end - .L_loop_begin: - str r2, [r0], 4 // store the word in r2 to the address in r0, post-indexed - cmp r0, r1 - bne .L_loop_begin - .L_loop_end: - bx lr - - // Jump to address given in first argument R0 that points to next's stage's VTOR - // Clear memory and all registers before jump - .global jump_to_next_stage - .type jump_to_next_stage, STT_FUNC -jump_to_next_stage: - mov r4, r0 // save input argument r0 (the address of the next stage's vector table) (r4 is callee save) - // this subroutine re-points the exception handlers before the C code - // that comprises them has been given a good environment to run. - // therefore, this code needs to disable interrupts before the VTOR - // update. then, the reset_handler of the next stage needs to re-enable interrupts. - // the following prevents activation of all exceptions except Non-Maskable Interrupt (NMI). - // according to "ARM Cortex-M Programming Guide to Memory Barrier Instructions" Application Note 321, section 4.8: - // "there is no requirement to insert memory barrier instructions after CPSID". - cpsid f - // wipe memory at the end of the current stage of code - ldr r2, =0 // r2 - the word-sized value to be written - ldr r0, =_handoff_clear_ram_0_start - ldr r1, =_handoff_clear_ram_0_end - bl memset_reg - ldr r0, =_handoff_clear_ram_1_start - ldr r1, =_handoff_clear_ram_1_end - bl memset_reg - ldr r0, =_handoff_clear_ram_2_start - ldr r1, =_handoff_clear_ram_2_end - bl memset_reg - - mov lr, r4 - // clear out the general purpose registers before the next stage's code can run - ldr r0, =0 - mov r1, r0 - mov r2, r0 - mov r3, r0 - mov r4, r0 - mov r5, r0 - mov r6, r0 - mov r7, r0 - mov r8, r0 - mov r9, r0 - mov r10, r0 - mov r11, r0 - mov r12, r0 - // give the next stage a fresh main stack pointer - ldr r0, [lr] // set r0 to the main stack pointer in the next stage's vector table - msr msp, r0 // give the next stage its main stack pointer - // point to the next stage's exception handlers - // AN321, section 4.11: "a memory barrier is not required after a VTOR update" - .set SCB_VTOR, 0xE000ED08 // reference "Cortex-M4 Devices Generic User Guide" section 4.3 - ldr r0, =SCB_VTOR - str lr, [r0] - mov r0, r1 // zero out r0 - // go on to the next stage - ldr lr, [lr, 4] // set lr to the next stage's reset_handler - bx lr - - .global shutdown_privileged - .type shutdown_privileged, STT_FUNC - // The function must be called from the privileged mode -shutdown_privileged: - cpsid f // disable all exceptions (except for NMI), the instruction is ignored in unprivileged mode - // if the exceptions weren't disabled, an exception handler (for example systick handler) - // could be called after the memory is erased, which would lead to another exception - ldr r0, =0 - mov r1, r0 - mov r2, r0 - mov r3, r0 - mov r4, r0 - mov r5, r0 - mov r6, r0 - mov r7, r0 - mov r8, r0 - mov r9, r0 - mov r10, r0 - mov r11, r0 - mov r12, r0 - ldr lr, =0xffffffff - - ldr r0, =_shutdown_clear_ram_0_start - ldr r1, =_shutdown_clear_ram_0_end - bl memset_reg - ldr r0, =_shutdown_clear_ram_1_start - ldr r1, =_shutdown_clear_ram_1_end - bl memset_reg - ldr r0, =_shutdown_clear_ram_2_start - ldr r1, =_shutdown_clear_ram_2_end - bl memset_reg - ldr r0, =_shutdown_clear_ram_3_start - ldr r1, =_shutdown_clear_ram_3_end - bl memset_reg - - ldr r0, =1 - msr control, r0 // jump to unprivileged mode - ldr r0, =0 - b . // loop forever - - .end diff --git a/core/embed/sys/startup/stm32u5/util.S b/core/embed/sys/startup/stm32u5/util.S deleted file mode 100644 index b76c86430f..0000000000 --- a/core/embed/sys/startup/stm32u5/util.S +++ /dev/null @@ -1,120 +0,0 @@ - .syntax unified - - .text - -#ifdef KERNEL_MODE - - .global memset_reg - .type memset_reg, STT_FUNC -memset_reg: - // call with the following (note that the arguments are not validated prior to use): - // r0 - address of first word to write (inclusive) - // r1 - address of first word following the address in r0 to NOT write (exclusive) - // r2 - word value to be written - // both addresses in r0 and r1 needs to be divisible by 4! - cmp r0, r1 - beq .L_loop_end - .L_loop_begin: - str r2, [r0], 4 // store the word in r2 to the address in r0, post-indexed - cmp r0, r1 - bne .L_loop_begin - .L_loop_end: - bx lr - - // Jump to address given in first argument R0 that points to next's stage's VTOR - // Clear memory and all registers before jump - .global jump_to_next_stage - .type jump_to_next_stage, STT_FUNC -jump_to_next_stage: - mov r4, r0 // save input argument r0 (the address of the next stage's vector table) (r4 is callee save) - // this subroutine re-points the exception handlers before the C code - // that comprises them has been given a good environment to run. - // therefore, this code needs to disable interrupts before the VTOR - // update. then, the reset_handler of the next stage needs to re-enable interrupts. - // the following prevents activation of all exceptions except Non-Maskable Interrupt (NMI). - // according to "ARM Cortex-M Programming Guide to Memory Barrier Instructions" Application Note 321, section 4.8: - // "there is no requirement to insert memory barrier instructions after CPSID". - cpsid f - // wipe memory at the end of the current stage of code - ldr r2, =0 // r2 - the word-sized value to be written - ldr r0, =_handoff_clear_ram_0_start - ldr r1, =_handoff_clear_ram_0_end - bl memset_reg - ldr r0, =_handoff_clear_ram_1_start - ldr r1, =_handoff_clear_ram_1_end - bl memset_reg - ldr r0, =_handoff_clear_ram_2_start - ldr r1, =_handoff_clear_ram_2_end - bl memset_reg - - mov lr, r4 - // clear out the general purpose registers before the next stage's code can run - ldr r0, =0 - mov r1, r0 - mov r2, r0 - mov r3, r0 - mov r4, r0 - mov r5, r0 - mov r6, r0 - mov r7, r0 - mov r8, r0 - mov r9, r0 - mov r10, r0 - mov r11, r0 - mov r12, r0 - // give the next stage a fresh main stack pointer - ldr r0, [lr] // set r0 to the main stack pointer in the next stage's vector table - msr msp, r0 // give the next stage its main stack pointer - // point to the next stage's exception handlers - // AN321, section 4.11: "a memory barrier is not required after a VTOR update" - .set SCB_VTOR, 0xE000ED08 // reference "Cortex-M4 Devices Generic User Guide" section 4.3 - ldr r0, =SCB_VTOR - str lr, [r0] - mov r0, r1 // zero out r0 - // go on to the next stage - ldr lr, [lr, 4] // set lr to the next stage's reset_handler - bx lr - - .global shutdown_privileged - .type shutdown_privileged, STT_FUNC - // The function must be called from the privileged mode -shutdown_privileged: - cpsid f // disable all exceptions (except for NMI), the instruction is ignored in unprivileged mode - // if the exceptions weren't disabled, an exception handler (for example systick handler) - // could be called after the memory is erased, which would lead to another exception - ldr r0, =0 - mov r1, r0 - mov r2, r0 - mov r3, r0 - mov r4, r0 - mov r5, r0 - mov r6, r0 - mov r7, r0 - mov r8, r0 - mov r9, r0 - mov r10, r0 - mov r11, r0 - mov r12, r0 - ldr lr, =0xffffffff - - ldr r0, =_shutdown_clear_ram_0_start - ldr r1, =_shutdown_clear_ram_0_end - bl memset_reg - ldr r0, =_shutdown_clear_ram_1_start - ldr r1, =_shutdown_clear_ram_1_end - bl memset_reg - ldr r0, =_shutdown_clear_ram_2_start - ldr r1, =_shutdown_clear_ram_2_end - bl memset_reg - ldr r0, =_shutdown_clear_ram_3_start - ldr r1, =_shutdown_clear_ram_3_end - bl memset_reg - - ldr r0, =1 - msr control, r0 // jump to unprivileged mode - ldr r0, =0 - b . // loop forever - -#endif - - .end diff --git a/core/embed/sys/startup/unix/bootutils.c b/core/embed/sys/startup/unix/bootutils.c index 12824d0c00..347335ce25 100644 --- a/core/embed/sys/startup/unix/bootutils.c +++ b/core/embed/sys/startup/unix/bootutils.c @@ -55,16 +55,18 @@ void bootargs_get_args(boot_args_t* dest) { memcpy(dest, &g_boot_args, sizeof(boot_args_t)); } -void __attribute__((noreturn)) secure_shutdown(void) { - printf("SHUTDOWN\n"); - - // Wait some time to let the user see the displayed - // message before shutting down - hal_delay(3000); +__attribute__((noreturn)) void reboot_device(void) { + printf("reboot (normal)\n"); exit(3); } -void ensure_compatible_settings(void) { - // Left empty +__attribute__((noreturn)) void reboot_or_halt_after_rsod(void) { + printf("reboot (with timeout)\n"); + + // Wait some time to let the user see the displayed + // message before shutting down + systick_delay_ms(3000); + + exit(3); } diff --git a/core/embed/sys/task/stm32/systask.c b/core/embed/sys/task/stm32/systask.c index 7e7be92f8c..852b53a415 100644 --- a/core/embed/sys/task/stm32/systask.c +++ b/core/embed/sys/task/stm32/systask.c @@ -206,7 +206,10 @@ static void systask_kill(systask_t* task) { if (scheduler->error_handler != NULL) { scheduler->error_handler(&task->pminfo); } - secure_shutdown(); + + // We reach this point only if error_handler is NULL or + // if it returns. Neither is expected to happen. + reboot_device(); } else if (task == scheduler->active_task) { // Switch to the kernel task systask_yield_to(&scheduler->kernel_task); diff --git a/core/embed/sys/task/stm32/system.c b/core/embed/sys/task/stm32/system.c index bb8f2d4c45..782c04decf 100644 --- a/core/embed/sys/task/stm32/system.c +++ b/core/embed/sys/task/stm32/system.c @@ -222,7 +222,9 @@ system_emergency_rescue_phase_2(uint32_t arg1, uint32_t arg2) { error_handler(&pminfo); } - secure_shutdown(); + // We reach this point only if error_handler is NULL or + // if it returns. Neither is expected to happen. + reboot_device(); } __attribute((naked, noreturn, no_stack_protector)) void system_emergency_rescue( diff --git a/core/embed/sys/task/unix/system.c b/core/embed/sys/task/unix/system.c index 48157962e9..451505fa09 100644 --- a/core/embed/sys/task/unix/system.c +++ b/core/embed/sys/task/unix/system.c @@ -48,7 +48,9 @@ void system_exit(int exitcode) { } } - secure_shutdown(); + // We reach this point only if g_error_handler is NULL or + // if it returns. Neither is expected to happen. + reboot_device(); } void system_exit_error_ex(const char* title, size_t title_len, @@ -77,7 +79,9 @@ void system_exit_error_ex(const char* title, size_t title_len, } } - secure_shutdown(); + // We reach this point only if g_error_handler is NULL or + // if it returns. Neither is expected to happen. + reboot_device(); } void system_exit_error(const char* title, const char* message, @@ -117,7 +121,9 @@ void system_exit_fatal_ex(const char* message, size_t message_len, } } - secure_shutdown(); + // We reach this point only if g_error_handler is NULL or + // if it returns. Neither is expected to happen. + reboot_device(); } void system_exit_fatal(const char* message, const char* file, int line) { @@ -136,5 +142,5 @@ void system_emergency_rescue(systask_error_handler_t error_handler, error_handler(pminfo); // We should never reach this point - exit(0); + reboot_device(); } diff --git a/core/embed/util/rsod/rsod.c b/core/embed/util/rsod/rsod.c index c23a91744b..1d9e9df702 100644 --- a/core/embed/util/rsod/rsod.c +++ b/core/embed/util/rsod/rsod.c @@ -177,8 +177,8 @@ static void init_and_show_rsod(const systask_postmortem_t* pminfo) { rsod_terminal(pminfo); #endif - // Wait for the user to manually power off the device - secure_shutdown(); + // Reboots or halts (if RSOD_INFINITE_LOOP is defined) + reboot_or_halt_after_rsod(); } // Universal panic handler diff --git a/core/site_scons/models/stm32f4_common.py b/core/site_scons/models/stm32f4_common.py index 70eb42f947..498803b1d1 100644 --- a/core/site_scons/models/stm32f4_common.py +++ b/core/site_scons/models/stm32f4_common.py @@ -95,17 +95,5 @@ def stm32f4_common_files(env, defines, sources, paths): "embed/util/unit_properties/stm32/unit_properties.c", ] - # boardloader needs separate assembler for some function unencumbered by various FW+bootloader hacks - # this helps to prevent making a bug in boardloader which may be hard to fix since it's locked with write-protect - env_constraints = env.get("CONSTRAINTS") - if env_constraints and "limited_util_s" in env_constraints: - sources += [ - "embed/sys/startup/stm32f4/limited_util.S", - ] - else: - sources += [ - "embed/sys/startup/stm32f4/util.S", - ] - env.get("ENV")["SUFFIX"] = "stm32f4" env.get("ENV")["LINKER_SCRIPT"] = """embed/sys/linker/stm32f4/{target}.ld""" diff --git a/core/site_scons/models/stm32u5_common.py b/core/site_scons/models/stm32u5_common.py index 1c887a5829..dbf1ac3801 100644 --- a/core/site_scons/models/stm32u5_common.py +++ b/core/site_scons/models/stm32u5_common.py @@ -115,16 +115,4 @@ def stm32u5_common_files(env, defines, sources, paths): "embed/util/unit_properties/stm32/unit_properties.c", ] - # boardloader needs separate assembler for some function unencumbered by various FW+bootloader hacks - # this helps to prevent making a bug in boardloader which may be hard to fix since it's locked with write-protect - env_constraints = env.get("CONSTRAINTS") - if env_constraints and "limited_util_s" in env_constraints: - sources += [ - "embed/sys/startup/stm32u5/limited_util.S", - ] - else: - sources += [ - "embed/sys/startup/stm32u5/util.S", - ] - env.get("ENV")["SUFFIX"] = "stm32u5"