refactor(core/sys): make writing bootargs a privilege, not a convention
What the developer wrote
Authored by tychovrahe
refactor(core/sys): make writing bootargs a privilege, not a convention
The smcall surface exposed a raw bootargs setter, letting unprivileged code name
any boot command. It had no user -- firmware goes through the reboot_* calls in
bootutils.h, each of which fixes its own -- so remove it rather than filter it.
The dispatch's fatal default is then the enforcement.
Make the rule structural too: bootargs_set is declared only under SECURE_MODE,
and system_emergency_rescue is compiled only where bootargs can be written. It
linked before only because --gc-sections dropped it as unreferenced.
[no changelog]
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
What changed, and why it matters
This commit removes a low-level interface that let any software component tell the device what to do on its next reboot. The change makes reboot control a privileged operation only, so unprivileged code can no longer pick arbitrary boot commands. It is a hardening/refactoring change rather than a documented fix for a specific active bug.
Treat as a positive hardening change. Review whether any downstream firmware or test code still expected SMCALL_BOOTARGS_SET or bootargs_set() from unprivileged contexts, since the enum value was renumbered and the symbol is now conditionally compiled. No immediate incident response is indicated by the commit materials alone.
Security signals we found
Removal of unprivileged secure-monitor call SMCALL_BOOTARGS_SET
bootargs_set() now gated behind SECURE_MODE build flag
system_emergency_rescue() now compiled only where bootargs can be written
Commit message describes prior surface as letting unprivileged code name any boot command
No changelog entry, tagged as refactor
Evidence from the diff
The patch removes SMCALL_BOOTARGS_SET from the secure-monitor-call (SMC) surface and its unprivileged stub bootargs_set(). The bootargs_set() function is now declared only when SECURE_MODE is defined, and system_emergency_rescue is guarded by the same macro. Previously, the SMC dispatcher exposed a raw setter for the boot command and boot arguments, which could let unprivileged code request any boot_command_t on reboot. The commit message states the interface had no user and that firmware should use the fixed reboot_* helpers in bootutils.h instead. The dispatch switch’s fatal default now enforces the restriction structurally.
Changed components
core/embed/sys/smcall/stm32/smcall_dispatch.ccore/embed/sys/smcall/stm32/smcall_numbers.hcore/embed/sys/smcall/stm32/smcall_stubs.ccore/embed/sys/smcall/stm32/smcall_verifiers.ccore/embed/sys/smcall/stm32/smcall_verifiers.hcore/embed/sys/startup/inc/sys/bootargs.hcore/embed/sys/task/stm32/system.cInspect captured patch +7 / −30
### core/embed/sys/smcall/stm32/smcall_dispatch.c
@@ -61,13 +61,6 @@
__attribute((no_stack_protector)) void smcall_handler(uint32_t *args,
uint32_t smcall) {
switch (smcall) {
- case SMCALL_BOOTARGS_SET: {
- boot_command_t command = args[0];
- const void *args_ptr = (const void *)args[1];
- size_t args_len = args[2];
- bootargs_set__verified(command, args_ptr, args_len);
- } break;
-
case SMCALL_BOOTARGS_GET_ARGS: {
boot_args_t *boot_args = (boot_args_t *)args[0];
bootargs_get_args__verified(boot_args);
### core/embed/sys/smcall/stm32/smcall_numbers.h
@@ -30,8 +30,7 @@ typedef struct {
// Secure monitor call identifiers
typedef enum {
- SMCALL_BOOTARGS_SET = 1,
- SMCALL_BOOTARGS_GET_ARGS,
+ SMCALL_BOOTARGS_GET_ARGS = 1,
SMCALL_BOOT_IMAGE_CHECK,
SMCALL_BOOT_IMAGE_REPLACE,
### core/embed/sys/smcall/stm32/smcall_stubs.c
@@ -30,10 +30,6 @@
#include <sys/bootargs.h>
-void bootargs_set(boot_command_t command, const void *args, size_t args_size) {
- smcall_invoke3(command, (uint32_t)args, args_size, SMCALL_BOOTARGS_SET);
-}
-
void bootargs_get_args(boot_args_t *args) {
smcall_invoke1((uint32_t)args, SMCALL_BOOTARGS_GET_ARGS);
}
### core/embed/sys/smcall/stm32/smcall_verifiers.c
@@ -33,20 +33,6 @@
// ---------------------------------------------------------------------
-void bootargs_set__verified(boot_command_t command, const void *args,
- size_t args_size) {
- // args are optional, so we allow NULL with size 0
- if (!probe_read_access_opt(args, args_size)) {
- goto access_violation;
- }
-
- bootargs_set(command, args, args_size);
- return;
-
-access_violation:
- apptask_access_violation();
-}
-
void bootargs_get_args__verified(boot_args_t *args) {
if (!probe_write_access(args, sizeof(*args))) {
goto access_violation;
### core/embed/sys/smcall/stm32/smcall_verifiers.h
@@ -24,9 +24,6 @@
// ---------------------------------------------------------------------
#include <sys/bootargs.h>
-void bootargs_set__verified(boot_command_t command, const void *args,
- size_t args_size);
-
void bootargs_get_args__verified(boot_args_t *args);
// ---------------------------------------------------------------------
### core/embed/sys/startup/inc/sys/bootargs.h
@@ -69,7 +69,9 @@ void bootargs_init(uint32_t r11_register);
// Configures the boot command and associated arguments for the next reboot.
// The arguments must adhere to the boot_args_t structure layout.
// Args are optional, so NULL with size 0 is allowed.
+#ifdef SECURE_MODE
void bootargs_set(boot_command_t command, const void* args, size_t args_size);
+#endif
// Returns the last boot command saved during bootloader startup
boot_command_t bootargs_get_command();
### core/embed/sys/task/stm32/system.c
@@ -87,6 +87,8 @@ void system_deinit(void) {
mpu_reconfig(MPU_MODE_DISABLED);
}
+#ifdef SECURE_MODE
+
__attribute((noreturn, no_stack_protector)) static void
system_emergency_rescue_phase_2(uint32_t arg1, uint32_t arg2) {
systask_error_handler_t error_handler = (systask_error_handler_t)arg1;
@@ -163,6 +165,8 @@ __attribute((naked, noreturn, no_stack_protector)) void system_emergency_rescue(
system_emergency_rescue_phase_2);
}
+#endif // SECURE_MODE
+
#endif // KERNEL_MODE
#ifdef STM32U5Why this scored 60/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.