fix(core): fix deadlock in systask_kill()
What the developer wrote
Authored by cepetr
fix(core): fix deadlock in systask_kill()
[no changelog]
What changed, and why it matters
This commit fixes a deadlock bug in the Trezor firmware's task-killing code. Previously, the code that terminates a background task was marked as a function that never returns, but under some conditions it could actually return and then spin forever doing nothing. That could freeze the device. The fix moves the 'never return' guarantee to the outer system-exit functions and lets the internal task-killer return normally after handing control back to the kernel task.
Treat as a reliability/security hardening fix. Review whether any caller of `systask_exit*` outside `system.c` relies on the old `noreturn` contract. Verify on hardware/emulator that task termination no longer hangs under fault injection. No immediate CVE action is indicated without evidence of exploitable trigger.
Security signals we found
Deadlock in task termination path
Incorrect use of noreturn attribute on function that can return
Kernel/task scheduler hang risk
Potential denial-of-service via unrecoverable firmware lockup
Evidence from the diff
The patch removes the __attribute__((noreturn)) annotation and the trailing while (1) {} spin loop from systask_kill() in both the STM32 and Unix implementations, and from systask_exit(), systask_exit_error(), and systask_exit_fatal() in the public header. Instead, it adds the noreturn attribute and an explicit infinite loop to the higher-level system_exit*() wrappers in system.c. The deadlock likely occurred because systask_kill() could yield to the kernel task and then, if execution resumed in the killed task’s context, fall through to the spin loop while holding locks or preventing cleanup. By making systask_kill() a normal returning function and ensuring only the top-level exit entry points are noreturn, the scheduler can clean up the task context correctly.
Changed components
core/embed/sys/task/stm32/systask.ccore/embed/sys/task/unix/systask.ccore/embed/sys/task/system.ccore/embed/sys/task/inc/sys/systask.hInspect captured patch +28 / −25
diff --git a/core/embed/sys/task/inc/sys/systask.h b/core/embed/sys/task/inc/sys/systask.h
index cf60819d..3e838055 100644
--- a/core/embed/sys/task/inc/sys/systask.h
+++ b/core/embed/sys/task/inc/sys/systask.h
@@ -360,7 +360,7 @@ systask_id_t systask_id(const systask_t* task);
* @param task Pointer to the task to terminate, or NULL.
* @param exit_code Exit code for the task.
*/
-void __attribute__((noreturn)) systask_exit(systask_t* task, int exit_code);
+void systask_exit(systask_t* task, int exit_code);
/**
* @brief Terminates the task with an error message
@@ -375,9 +375,9 @@ void __attribute__((noreturn)) systask_exit(systask_t* task, int exit_code);
* @param footer Footer string.
* @param footer_len Length of the footer.
*/
-void __attribute__((noreturn)) 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);
+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);
/**
* @brief Terminates the task with a fatal error message
@@ -391,11 +391,9 @@ void __attribute__((noreturn)) systask_exit_error(
* @param file_len Length of the file string.
* @param line Line number.
*/
-void __attribute__((noreturn)) systask_exit_fatal(systask_t* task,
- const char* message,
- size_t message_len,
- const char* file,
- size_t file_len, int line);
+void systask_exit_fatal(systask_t* task, const char* message,
+ size_t message_len, const char* file, size_t file_len,
+ int line);
/**
* @brief Prints the post-mortem information about the task to the debug output
diff --git a/core/embed/sys/task/stm32/systask.c b/core/embed/sys/task/stm32/systask.c
index 44e03577..0598181d 100644
--- a/core/embed/sys/task/stm32/systask.c
+++ b/core/embed/sys/task/stm32/systask.c
@@ -340,7 +340,7 @@ uint32_t systask_get_r0(systask_t* task) {
return stack[STK_FRAME_R0];
}
-static void __attribute__((noreturn)) systask_kill(systask_t* task) {
+static void systask_kill(systask_t* task) {
systask_scheduler_t* scheduler = &g_systask_scheduler;
task->killed = 1;
@@ -362,10 +362,6 @@ static void __attribute__((noreturn)) systask_kill(systask_t* task) {
// Switch to the kernel task
systask_yield_to(&scheduler->kernel_task);
}
-
- while (1) {
- // This point should never be reached
- }
}
bool systask_is_alive(const systask_t* task) {
diff --git a/core/embed/sys/task/system.c b/core/embed/sys/task/system.c
index be49b550..f9aa9075 100644
--- a/core/embed/sys/task/system.c
+++ b/core/embed/sys/task/system.c
@@ -24,18 +24,31 @@
#ifdef KERNEL_MODE
-void system_exit(int exitcode) { systask_exit(NULL, exitcode); }
+void __attribute__((noreturn)) system_exit(int exitcode) {
+ systask_exit(NULL, exitcode);
+ while (1) {
+ // This point should never be reached
+ }
+}
-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) {
+void __attribute__((noreturn)) 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);
+ while (1) {
+ // This point should never be reached
+ }
}
-void system_exit_fatal_ex(const char* message, size_t message_len,
- const char* file, size_t file_len, int line) {
+void __attribute__((noreturn)) 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);
+ while (1) {
+ // This point should never be reached
+ }
}
#endif // KERNEL_MODE
diff --git a/core/embed/sys/task/unix/systask.c b/core/embed/sys/task/unix/systask.c
index 0a1afe3b..594cd740 100644
--- a/core/embed/sys/task/unix/systask.c
+++ b/core/embed/sys/task/unix/systask.c
@@ -224,7 +224,7 @@ bool systask_push_call(systask_t* task, void* fn, uintptr_t arg1,
return true;
}
-static void __attribute__((noreturn)) systask_kill(systask_t* task) {
+static void systask_kill(systask_t* task) {
systask_scheduler_t* scheduler = &g_systask_scheduler;
systask_print_pminfo(task);
@@ -248,10 +248,6 @@ static void __attribute__((noreturn)) systask_kill(systask_t* task) {
// Switch to the kernel task
systask_yield_to(&scheduler->kernel_task);
}
-
- while (1) {
- // This point should never be reached
- }
}
bool systask_is_alive(const systask_t* task) {
Why this scored 42/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.