From 924452272164333575b9e744ae7dae0cacc2d906 Mon Sep 17 00:00:00 2001 From: matejcik Date: Fri, 24 Mar 2023 11:24:34 +0100 Subject: [PATCH] fix(core): remove `shutdown()` In a very weird situation, our declaration of `shutdown()` shadows a function `shutdown(int, int)` from sys/socket, which _just happens_ to be called by libxcb when closing the sdl window. This calls `main_clean_exit` which calls into micropython and causes at best an uncaught NLR and at worst an outright segfault because by that time the micropython environment doesn't exist anymore. I didn't think this sort of thing would be possible but here we are?? Fixed by removing `__shutdown()` and replacing `shutdown` with `trezor_shutdown` --- core/embed/rust/src/trezorhal/fatal_error.rs | 4 ++-- core/embed/trezorhal/common.c | 6 +++--- core/embed/trezorhal/common.h | 2 +- core/embed/unix/common.c | 10 ++++------ core/embed/unix/common.h | 2 +- core/embed/unix/touch/touch.c | 3 --- 6 files changed, 11 insertions(+), 16 deletions(-) diff --git a/core/embed/rust/src/trezorhal/fatal_error.rs b/core/embed/rust/src/trezorhal/fatal_error.rs index 551462b3bf..37a9532854 100644 --- a/core/embed/rust/src/trezorhal/fatal_error.rs +++ b/core/embed/rust/src/trezorhal/fatal_error.rs @@ -1,7 +1,7 @@ mod ffi { extern "C" { // trezorhal/common.c - pub fn shutdown() -> !; + pub fn trezor_shutdown() -> !; } } @@ -13,7 +13,7 @@ use heapless::String; use crate::ui::util::u32_to_str; fn shutdown() -> ! { - unsafe { ffi::shutdown() } + unsafe { ffi::trezor_shutdown() } } #[cfg(feature = "bootloader")] diff --git a/core/embed/trezorhal/common.c b/core/embed/trezorhal/common.c index 3de8319267..01a83eacf2 100644 --- a/core/embed/trezorhal/common.c +++ b/core/embed/trezorhal/common.c @@ -43,7 +43,7 @@ // from util.s extern void shutdown_privileged(void); -void __attribute__((noreturn)) shutdown(void) { +void __attribute__((noreturn)) trezor_shutdown(void) { #ifdef USE_SVC_SHUTDOWN svc_shutdown(); #else @@ -88,7 +88,7 @@ __fatal_error(const char *expr, const char *msg, const char *file, int line, #endif display_printf("\nPlease contact Trezor support.\n"); #endif - shutdown(); + trezor_shutdown(); } void __attribute__((noreturn)) @@ -108,7 +108,7 @@ error_shutdown(const char *label, const char *msg) { display_printf("\nPlease unplug the device.\n"); #endif display_backlight(255); - shutdown(); + trezor_shutdown(); } #ifndef NDEBUG diff --git a/core/embed/trezorhal/common.h b/core/embed/trezorhal/common.h index 52aab3b0f1..2a61f4302e 100644 --- a/core/embed/trezorhal/common.h +++ b/core/embed/trezorhal/common.h @@ -51,7 +51,7 @@ #define STAY_IN_BOOTLOADER_FLAG 0x0FC35A96 -void __attribute__((noreturn)) shutdown(void); +void __attribute__((noreturn)) trezor_shutdown(void); void __attribute__((noreturn)) __fatal_error(const char *expr, const char *msg, const char *file, int line, diff --git a/core/embed/unix/common.c b/core/embed/unix/common.c index 29f1605553..6317b95848 100644 --- a/core/embed/unix/common.c +++ b/core/embed/unix/common.c @@ -34,15 +34,13 @@ extern void main_clean_exit(); extern float DISPLAY_GAMMA; -void __attribute__((noreturn)) __shutdown(void) { +void __attribute__((noreturn)) trezor_shutdown(void) { printf("SHUTDOWN\n"); main_clean_exit(3); for (;;) ; } -void __attribute__((noreturn)) shutdown(void) { __shutdown(); } - #ifdef RGB16 #define COLOR_FATAL_ERROR RGB16(0x7F, 0x00, 0x00) #else @@ -92,7 +90,7 @@ __fatal_error(const char *expr, const char *msg, const char *file, int line, printf("Hint:\nIsn't the emulator already running?\n"); #endif hal_delay(3000); - shutdown(); + trezor_shutdown(); } void __attribute__((noreturn)) @@ -135,7 +133,7 @@ uint32_t hal_ticks_ms() { static int SDLCALL emulator_event_filter(void *userdata, SDL_Event *event) { switch (event->type) { case SDL_QUIT: - __shutdown(); + trezor_shutdown(); return 0; case SDL_KEYUP: if (event->key.repeat) { @@ -143,7 +141,7 @@ static int SDLCALL emulator_event_filter(void *userdata, SDL_Event *event) { } switch (event->key.keysym.sym) { case SDLK_ESCAPE: - __shutdown(); + trezor_shutdown(); return 0; case SDLK_p: display_save("emu"); diff --git a/core/embed/unix/common.h b/core/embed/unix/common.h index 89d7da71d0..4c528e7018 100644 --- a/core/embed/unix/common.h +++ b/core/embed/unix/common.h @@ -40,7 +40,7 @@ }) #endif -void __attribute__((noreturn)) shutdown(void); +void __attribute__((noreturn)) trezor_shutdown(void); void __attribute__((noreturn)) __fatal_error(const char *expr, const char *msg, const char *file, int line, diff --git a/core/embed/unix/touch/touch.c b/core/embed/unix/touch/touch.c index 7ccca305a1..7585e3b85a 100644 --- a/core/embed/unix/touch/touch.c +++ b/core/embed/unix/touch/touch.c @@ -21,9 +21,6 @@ #include #include -extern void __shutdown(void); -extern const char *display_save(const char *prefix); - #if defined TREZOR_MODEL_T #include "touch.h"