From 7f7193afc873c0acca18c12361925dabcacffbee Mon Sep 17 00:00:00 2001 From: cepetr Date: Mon, 30 Sep 2024 09:29:28 +0200 Subject: [PATCH] refactor(core): adjust system api for syscall verifier [no changelog] --- core/embed/rust/src/trezorhal/fatal_error.rs | 36 ++--- .../trezorhal/stm32f4/syscall_dispatch.c | 18 ++- core/embed/trezorhal/stm32f4/syscall_stubs.c | 16 ++- core/embed/trezorhal/stm32f4/systask.c | 24 ++-- core/embed/trezorhal/stm32f4/system.c | 124 ++++++++++-------- core/embed/trezorhal/systask.h | 8 +- core/embed/trezorhal/system.h | 43 +++--- core/embed/trezorhal/unix/system.c | 47 +++++-- 8 files changed, 200 insertions(+), 116 deletions(-) diff --git a/core/embed/rust/src/trezorhal/fatal_error.rs b/core/embed/rust/src/trezorhal/fatal_error.rs index dec821bd815..fe62d4abe01 100644 --- a/core/embed/rust/src/trezorhal/fatal_error.rs +++ b/core/embed/rust/src/trezorhal/fatal_error.rs @@ -1,27 +1,29 @@ mod ffi { extern "C" { - // error_handling.h - pub fn error_shutdown(msg: *const cty::c_char) -> !; + // system.h + pub fn system_exit_error_ex( + title: *const cty::c_char, + title_len: usize, + message: *const cty::c_char, + message_len: usize, + footer: *const cty::c_char, + footer_len: usize, + ) -> !; } } pub fn error_shutdown(msg: &str) -> ! { - const MAX_LEN: usize = 63; - let mut buffer: [u8; MAX_LEN + 1] = [0; MAX_LEN + 1]; - - // Copy the message to the buffer - let msg_bytes = msg.as_bytes(); - let len = if msg_bytes.len() < MAX_LEN { - msg_bytes.len() - } else { - MAX_LEN - }; - buffer[..len].copy_from_slice(&msg_bytes[..len]); - unsafe { - // SAFETY: `buffer` is a valid null-terminated string - // and the function never returns. - ffi::error_shutdown(buffer.as_ptr() as *const cty::c_char); + // SAFETY: we pass a valid string to the C function + // and the function does not return. + ffi::system_exit_error_ex( + core::ptr::null(), + 0, + msg.as_ptr() as *const cty::c_char, + msg.len(), + core::ptr::null(), + 0, + ); } } diff --git a/core/embed/trezorhal/stm32f4/syscall_dispatch.c b/core/embed/trezorhal/stm32f4/syscall_dispatch.c index dfe2426e00d..3b346e9b6e6 100644 --- a/core/embed/trezorhal/stm32f4/syscall_dispatch.c +++ b/core/embed/trezorhal/stm32f4/syscall_dispatch.c @@ -76,17 +76,23 @@ __attribute((no_stack_protector)) void syscall_handler(uint32_t *args, case SYSCALL_SYSTEM_EXIT_ERROR: { systask_t *task = systask_active(); const char *title = (const char *)args[0]; - const char *message = (const char *)args[1]; - const char *footer = (const char *)args[2]; - systask_exit_error(task, title, message, footer); + size_t title_len = (size_t)args[1]; + const char *message = (const char *)args[2]; + size_t message_len = (size_t)args[3]; + const char *footer = (const char *)args[4]; + size_t footer_len = (size_t)args[5]; + systask_exit_error(task, title, title_len, message, message_len, footer, + footer_len); } break; case SYSCALL_SYSTEM_EXIT_FATAL: { systask_t *task = systask_active(); const char *message = (const char *)args[0]; - const char *file = (const char *)args[1]; - int line = (int)args[2]; - systask_exit_fatal(task, message, file, line); + size_t message_len = (size_t)args[1]; + const char *file = (const char *)args[2]; + size_t file_len = (size_t)args[3]; + int line = (int)args[4]; + systask_exit_fatal(task, message, message_len, file, file_len, line); } break; case SYSCALL_SYSTICK_CYCLES: { diff --git a/core/embed/trezorhal/stm32f4/syscall_stubs.c b/core/embed/trezorhal/stm32f4/syscall_stubs.c index 045e0779a04..f8472c46861 100644 --- a/core/embed/trezorhal/stm32f4/syscall_stubs.c +++ b/core/embed/trezorhal/stm32f4/syscall_stubs.c @@ -33,17 +33,19 @@ void system_exit(int exit_code) { ; } -void system_exit_error(const char *title, const char *message, - const char *footer) { - syscall_invoke3((uint32_t)title, (uint32_t)message, (uint32_t)footer, - SYSCALL_SYSTEM_EXIT_ERROR); +void system_exit_error_ex(const char *title, size_t title_len, + const char *message, size_t message_len, + const char *footer, size_t footer_len) { + syscall_invoke6((uint32_t)title, title_len, (uint32_t)message, message_len, + (uint32_t)footer, footer_len, SYSCALL_SYSTEM_EXIT_ERROR); while (1) ; } -void system_exit_fatal(const char *message, const char *file, int line) { - syscall_invoke3((uint32_t)message, (uint32_t)file, line, - SYSCALL_SYSTEM_EXIT_FATAL); +void system_exit_fatal_ex(const char *message, size_t message_len, + const char *file, size_t file_len, int line) { + syscall_invoke5((uint32_t)message, message_len, (uint32_t)file, file_len, + line, SYSCALL_SYSTEM_EXIT_FATAL); while (1) ; } diff --git a/core/embed/trezorhal/stm32f4/systask.c b/core/embed/trezorhal/stm32f4/systask.c index 56456f7c08b..3528eb02ef0 100644 --- a/core/embed/trezorhal/stm32f4/systask.c +++ b/core/embed/trezorhal/stm32f4/systask.c @@ -23,6 +23,7 @@ #include #include "bootutils.h" +#include "common.h" #include "irq.h" #include "mpu.h" #include "syscall.h" @@ -234,8 +235,9 @@ void systask_exit(systask_t* task, int exit_code) { systask_kill(task); } -void systask_exit_error(systask_t* task, const char* title, const char* message, - const char* footer) { +void systask_exit_error(systask_t* task, const char* title, size_t title_len, + const char* message, size_t message_len, + const char* footer, size_t footer_len) { systask_scheduler_t* scheduler = &g_systask_scheduler; if (task == NULL) { @@ -250,21 +252,25 @@ void systask_exit_error(systask_t* task, const char* title, const char* message, pminfo->privileged = (task == &scheduler->kernel_task); if (title != NULL) { - strncpy(pminfo->error.title, title, sizeof(pminfo->error.title) - 1); + size_t len = MIN(title_len, sizeof(pminfo->error.title) - 1); + strncpy(pminfo->error.title, title, len); } if (message != NULL) { - strncpy(pminfo->error.message, message, sizeof(pminfo->error.message) - 1); + size_t len = MIN(message_len, sizeof(pminfo->error.message) - 1); + strncpy(pminfo->error.message, message, len); } if (footer != NULL) { - strncpy(pminfo->error.footer, footer, sizeof(pminfo->error.footer) - 1); + size_t len = MIN(footer_len, sizeof(pminfo->error.footer) - 1); + strncpy(pminfo->error.footer, footer, len); } systask_kill(task); } -void systask_exit_fatal(systask_t* task, const char* message, const char* file, +void systask_exit_fatal(systask_t* task, const char* message, + size_t message_len, const char* file, size_t file_len, int line) { systask_scheduler_t* scheduler = &g_systask_scheduler; @@ -280,11 +286,13 @@ void systask_exit_fatal(systask_t* task, const char* message, const char* file, pminfo->privileged = (task == &scheduler->kernel_task); if (message != NULL) { - strncpy(pminfo->fatal.expr, message, sizeof(pminfo->fatal.expr) - 1); + size_t len = MIN(message_len, sizeof(pminfo->fatal.expr) - 1); + strncpy(pminfo->fatal.expr, message, len); } if (file != NULL) { - strncpy(pminfo->fatal.file, file, sizeof(pminfo->fatal.file) - 1); + size_t len = MIN(file_len, sizeof(pminfo->fatal.file) - 1); + strncpy(pminfo->fatal.file, file, len); } pminfo->fatal.line = line; diff --git a/core/embed/trezorhal/stm32f4/system.c b/core/embed/trezorhal/stm32f4/system.c index 351c359b0b5..abe59f11259 100644 --- a/core/embed/trezorhal/stm32f4/system.c +++ b/core/embed/trezorhal/stm32f4/system.c @@ -19,13 +19,19 @@ #include STM32_HAL_H -#include "system.h" +#include + #include "bootutils.h" #include "mpu.h" #include "systask.h" +#include "system.h" #include "systick.h" #include "systimer.h" +#ifndef HardFault_IRQn +#define HardFault_IRQn (-13) // not defined in stm32lib/cmsis/stm32429xx.h +#endif + #ifdef KERNEL_MODE void system_init(systask_error_handler_t error_handler) { @@ -38,61 +44,18 @@ void system_init(systask_error_handler_t error_handler) { void system_exit(int exitcode) { systask_exit(NULL, exitcode); } -void system_exit_error(const char* title, const char* message, - const char* footer) { - systask_exit_error(NULL, title, message, footer); +void system_exit_error_ex(const char* title, size_t title_len, + const char* message, size_t message_len, + const char* footer, size_t footer_len) { + systask_exit_error(NULL, title, title_len, message, message_len, footer, + footer_len); } -void system_exit_fatal(const char* message, const char* file, int line) { - systask_exit_fatal(NULL, message, file, line); +void system_exit_fatal_ex(const char* message, size_t message_len, + const char* file, size_t file_len, int line) { + systask_exit_fatal(NULL, message, message_len, file, file_len, line); } -#endif // KERNEL_MODE - -#ifndef HardFault_IRQn -#define HardFault_IRQn (-13) // not defined in stm32lib/cmsis/stm32429xx.h -#endif - -#ifdef STM32U5 -const char* system_fault_message(const system_fault_t* fault) { - switch (fault->irqn) { - case HardFault_IRQn: - return "(HF)"; - case MemoryManagement_IRQn: - return "(MM)"; - case BusFault_IRQn: - return "(BF)"; - case UsageFault_IRQn: - return (fault->cfsr & SCB_CFSR_STKOF_Msk) ? "(SO)" : "(UF)"; - case SecureFault_IRQn: - return "(SF)"; - case GTZC_IRQn: - return "(IA)"; - case NonMaskableInt_IRQn: - return "(CS)"; - default: - return "(FAULT)"; - } -} -#else // STM32U5 -const char* system_fault_message(const system_fault_t* fault) { - switch (fault->irqn) { - case HardFault_IRQn: - return "(HF)"; - case MemoryManagement_IRQn: - return (fault->sp < fault->sp_lim) ? "(SO)" : "(MM)"; - case BusFault_IRQn: - return "(BF)"; - case UsageFault_IRQn: - return "(UF)"; - case NonMaskableInt_IRQn: - return "(CS)"; - default: - return "(FAULT)"; - } -} -#endif // STM32U5 - __attribute__((used)) static void emergency_reset(void) { // TODO: reset peripherals (at least DMA, DMA2D) @@ -308,3 +271,60 @@ __attribute((naked, no_stack_protector)) void system_emergency_rescue( : // no clobber ); } + +#endif // KERNEL_MODE + +#ifdef STM32U5 +const char* system_fault_message(const system_fault_t* fault) { + switch (fault->irqn) { + case HardFault_IRQn: + return "(HF)"; + case MemoryManagement_IRQn: + return "(MM)"; + case BusFault_IRQn: + return "(BF)"; + case UsageFault_IRQn: + return (fault->cfsr & SCB_CFSR_STKOF_Msk) ? "(SO)" : "(UF)"; + case SecureFault_IRQn: + return "(SF)"; + case GTZC_IRQn: + return "(IA)"; + case NonMaskableInt_IRQn: + return "(CS)"; + default: + return "(FAULT)"; + } +} +#else // STM32U5 +const char* system_fault_message(const system_fault_t* fault) { + switch (fault->irqn) { + case HardFault_IRQn: + return "(HF)"; + case MemoryManagement_IRQn: + return (fault->sp < fault->sp_lim) ? "(SO)" : "(MM)"; + case BusFault_IRQn: + return "(BF)"; + case UsageFault_IRQn: + return "(UF)"; + case NonMaskableInt_IRQn: + return "(CS)"; + default: + return "(FAULT)"; + } +} +#endif // STM32U5 + +void system_exit_error(const char* title, const char* message, + const char* footer) { + size_t title_len = title != NULL ? strlen(title) : 0; + size_t message_len = message != NULL ? strlen(message) : 0; + size_t footer_len = footer != NULL ? strlen(footer) : 0; + system_exit_error_ex(title, title_len, message, message_len, footer, + footer_len); +} + +void system_exit_fatal(const char* message, const char* file, int line) { + size_t message_len = message != NULL ? strlen(message) : 0; + size_t file_len = file != NULL ? strlen(file) : 0; + system_exit_fatal_ex(message, message_len, file, file_len, line); +} diff --git a/core/embed/trezorhal/systask.h b/core/embed/trezorhal/systask.h index 90170fdaf1c..58cfbe6abcf 100644 --- a/core/embed/trezorhal/systask.h +++ b/core/embed/trezorhal/systask.h @@ -168,13 +168,15 @@ void systask_exit(systask_t* task, int exit_code); // Terminates the task with an error message // // (see `systask_exit()` for more details) -void systask_exit_error(systask_t* task, const char* title, const char* message, - const char* footer); +void systask_exit_error(systask_t* task, const char* title, size_t title_len, + const char* message, size_t message_len, + const char* footer, size_t footer_len); // Terminates the task with a fatal error message // // (see `systask_exit()` for more details) -void systask_exit_fatal(systask_t* task, const char* message, const char* file, +void systask_exit_fatal(systask_t* task, const char* message, + size_t message_len, const char* file, size_t file_len, int line); #endif // KERNEL_MODE diff --git a/core/embed/trezorhal/system.h b/core/embed/trezorhal/system.h index 07418111220..5c952c1bff3 100644 --- a/core/embed/trezorhal/system.h +++ b/core/embed/trezorhal/system.h @@ -22,6 +22,8 @@ #include +#ifdef KERNEL_MODE + // Initializes the fundamental system services // (MPU, SysTick, systimer and task scheduler). // @@ -29,6 +31,23 @@ // with an error void system_init(systask_error_handler_t error_handler); +// Calls the error handler in the emergency mode. +// +// This function is called when the system encounters a critical error +// and needs to perform a useful action (such as displaying an error message) +// before it is reset or shut down. +// +// The function may be called from any context, including interrupt context. +// It completely resets stack pointers, clears the .bss segment, reinitializes +// the .data segment, and calls the `error_handler` callback. +// +// The system will be in a state similar to a reset when `main()` is called +// (but with some hardware peripherals still initialized and running). +__attribute__((noreturn)) void system_emergency_rescue( + systask_error_handler_t error_handler, const systask_postmortem_t* pminfo); + +#endif // KERNEL_MODE + // Terminates the current task normally with the given exit code. // // If the current task is the kernel task, the error handler is called with the @@ -42,27 +61,21 @@ void system_exit(int exitcode); void system_exit_error(const char* title, const char* message, const char* footer); +// Like `system_exit_error`, but with explicit lengths for the strings. +void system_exit_error_ex(const char* title, size_t title_len, + const char* message, size_t message_len, + const char* footer, size_t footer_len); + // Terminates the current task with a fatal error message. // // See the notes for `system_exit` regarding the behavior of the error handler void system_exit_fatal(const char* message, const char* file, int line); +// Like `system_exit_fatal`, but with explicit lengths for the strings. +void system_exit_fatal_ex(const char* message, size_t message_len, + const char* file, size_t file_len, int line); + // Returns string representation of the system fault. const char* system_fault_message(const system_fault_t* fault); -// Calls the error handler in the emergency mode. -// -// This function is called when the system encounters a critical error -// and needs to perform a useful action (such as displaying an error message) -// before it is reset or shut down. -// -// The function may be called from any context, including interrupt context. -// It completely resets stack pointers, clears the .bss segment, reinitializes -// the .data segment, and calls the `error_handler` callback. -// -// The system will be in a state similar to a reset when `main()` is called -// (but with some hardware peripherals still initialized and running). -__attribute__((noreturn)) void system_emergency_rescue( - systask_error_handler_t error_handler, const systask_postmortem_t* pminfo); - #endif // TREZORHAL_SYSTEM_H diff --git a/core/embed/trezorhal/unix/system.c b/core/embed/trezorhal/unix/system.c index 728bce47ef6..8fbab5bf1ea 100644 --- a/core/embed/trezorhal/unix/system.c +++ b/core/embed/trezorhal/unix/system.c @@ -49,18 +49,26 @@ void system_exit(int exitcode) { secure_shutdown(); } -void system_exit_error(const char* title, const char* message, - const char* footer) { +void system_exit_error_ex(const char* title, size_t title_len, + const char* message, size_t message_len, + const char* footer, size_t footer_len) { fprintf(stderr, "ERROR: %s\n", message); fflush(stderr); if (g_error_handler != NULL) { systask_postmortem_t pminfo = {0}; + size_t len; pminfo.reason = TASK_TERM_REASON_ERROR; - strncpy(pminfo.error.title, title, sizeof(pminfo.error.title) - 1); - strncpy(pminfo.error.message, message, sizeof(pminfo.error.message) - 1); - strncpy(pminfo.error.footer, footer, sizeof(pminfo.error.footer) - 1); + + len = MIN(title_len, sizeof(pminfo.error.title) - 1); + strncpy(pminfo.error.title, title, len); + + len = MIN(message_len, sizeof(pminfo.error.message) - 1); + strncpy(pminfo.error.message, message, len); + + len = MIN(footer_len, sizeof(pminfo.error.footer) - 1); + strncpy(pminfo.error.footer, footer, len); if (g_error_handler != NULL) { g_error_handler(&pminfo); @@ -70,7 +78,18 @@ void system_exit_error(const char* title, const char* message, secure_shutdown(); } -void system_exit_fatal(const char* message, const char* file, int line) { +void system_exit_error(const char* title, const char* message, + const char* footer) { + size_t title_len = title != NULL ? strlen(title) : 0; + size_t message_len = message != NULL ? strlen(message) : 0; + size_t footer_len = footer != NULL ? strlen(footer) : 0; + + system_exit_error_ex(title, title_len, message, message_len, footer, + footer_len); +} + +void system_exit_fatal_ex(const char* message, size_t message_len, + const char* file, size_t file_len, int line) { fprintf(stderr, "ERROR: %s\n", message); if (file) { fprintf(stderr, "FILE: %s:%d\n", file, line); @@ -79,10 +98,16 @@ void system_exit_fatal(const char* message, const char* file, int line) { if (g_error_handler != NULL) { systask_postmortem_t pminfo = {0}; + size_t len; pminfo.reason = TASK_TERM_REASON_FATAL; - strncpy(pminfo.fatal.file, file, sizeof(pminfo.fatal.file) - 1); - strncpy(pminfo.fatal.expr, message, sizeof(pminfo.fatal.expr) - 1); + + len = MIN(message_len, sizeof(pminfo.fatal.expr) - 1); + strncpy(pminfo.fatal.file, file, len); + + len = MIN(file_len, sizeof(pminfo.fatal.file) - 1); + strncpy(pminfo.fatal.expr, message, len); + pminfo.fatal.line = line; if (g_error_handler != NULL) { @@ -93,6 +118,12 @@ void system_exit_fatal(const char* message, const char* file, int line) { secure_shutdown(); } +void system_exit_fatal(const char* message, const char* file, int line) { + size_t message_len = message != NULL ? strlen(message) : 0; + size_t file_len = file != NULL ? strlen(file) : 0; + systask_exit_fatal_ex(message, message_len, file, file_len, line); +} + const char* system_fault_message(const system_fault_t* fault) { // Not used in simulator return "(FAULT)";