Public commit record
57/100 · Thin
What the developer wrote
Authored by tychovrahe
refactor(core): refactor option bytes
[no changelog]
✓ Descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope! No meaningful explanatory body
What changed, and why it matters
This commit is a straightforward code cleanup: it renames functions like flash_configure_option_bytes() to option_bytes_configure() and check_oem_keys() to option_bytes_check_oem_keys(), makes several internal helpers static (visible only within one file), and changes the compile-time guard from KERNEL_MODE to SECURE_MODE. The actual logic that checks and writes the chip's security option bytes is unchanged. There is no indication this fixes a security bug or introduces a vulnerability.
Recommended action
No security action required; treat as a normal maintainability refactor. Reviewers may optionally verify that the renamed symbols are exported consistently and that no other translation units relied on the now-static helpers.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors the option-bytes subsystem in Trezor’s embedded firmware. It renames public APIs to live under an option_bytes_ namespace, marks flash_check_option_bytes, flash_lock_option_bytes, flash_unlock_option_bytes and flash_set_option_bytes as static, and switches the preprocessor guard from KERNEL_MODE to SECURE_MODE. Call sites in boardloader, kernel and secmon are updated to use the new names. The bodies of the functions, the values being checked/written, and the OEM-key sanity check remain identical to the pre-patch version.
Changed components
core/embed/sec/option_bytes (STM32F4 and STM32U5 implementations)core/embed/projects/boardloader/main.ccore/embed/projects/kernel/main.ccore/embed/projects/secmon/main.cInspect captured patch +35 / −29
diff --git a/core/embed/projects/boardloader/main.c b/core/embed/projects/boardloader/main.c
index 11a2cebb..bd1e45e1 100644
--- a/core/embed/projects/boardloader/main.c
+++ b/core/embed/projects/boardloader/main.c
@@ -309,7 +309,7 @@ int main(void) {
reset_flags_reset();
- if (sectrue != flash_configure_option_bytes()) {
+ if (sectrue != option_bytes_configure()) {
// Option bytes were not configured correctly at startup.
// This may indicate a first boot after manufacturing,
// or a potential hardware fault or exploit attempt.
diff --git a/core/embed/projects/kernel/main.c b/core/embed/projects/kernel/main.c
index 4e9b8215..d849a649 100644
--- a/core/embed/projects/kernel/main.c
+++ b/core/embed/projects/kernel/main.c
@@ -161,7 +161,7 @@ void drivers_init() {
#ifdef SECURE_MODE
#ifdef USE_OEM_KEYS_CHECK
- check_oem_keys();
+ option_bytes_check_oem_keys();
#endif
#endif
diff --git a/core/embed/projects/secmon/main.c b/core/embed/projects/secmon/main.c
index 6bca93bc..cedc9c30 100644
--- a/core/embed/projects/secmon/main.c
+++ b/core/embed/projects/secmon/main.c
@@ -94,7 +94,7 @@ static void drivers_init(void) {
#endif
#ifdef USE_OEM_KEYS_CHECK
- check_oem_keys();
+ option_bytes_check_oem_keys();
#endif
#ifdef USE_OPTIGA
diff --git a/core/embed/sec/option_bytes/inc/sec/option_bytes.h b/core/embed/sec/option_bytes/inc/sec/option_bytes.h
index d48900a5..b8697068 100644
--- a/core/embed/sec/option_bytes/inc/sec/option_bytes.h
+++ b/core/embed/sec/option_bytes/inc/sec/option_bytes.h
@@ -17,21 +17,27 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#ifndef TREZORHAL_OPTION_BYTES_H
-#define TREZORHAL_OPTION_BYTES_H
+#pragma once
#include <trezor_types.h>
-#ifdef KERNEL_MODE
+#ifdef SECURE_MODE
-secbool flash_check_option_bytes(void);
-void flash_lock_option_bytes(void);
-void flash_unlock_option_bytes(void);
-uint32_t flash_set_option_bytes(void);
-secbool flash_configure_option_bytes(void);
-
-void check_oem_keys(void);
+/**
+ * @brief Configure MCU option bytes to the expected secure values if needed.
+ *
+ * @return sectrue if option bytes were already correctly configured and no
+ * changes were required; returns secfalse if the routine had to update the
+ * option bytes (a reset/relaunch may follow as part of the procedure).
+ */
+secbool option_bytes_configure(void);
-#endif // KERNEL_MODE
+/**
+ * @brief Perform a sanity check on OEM key lock status bits to ensure OEM keys
+ * are not programmed (should be 0xFFFFFFFF).
+ *
+ * Triggers a panic via ensure() if a key is detected as set.
+ */
+void option_bytes_check_oem_keys(void);
-#endif // TREZORHAL_OPTION_BYTES_H
+#endif // SECURE_MODE
diff --git a/core/embed/sec/option_bytes/stm32f4/option_bytes.c b/core/embed/sec/option_bytes/stm32f4/option_bytes.c
index 7f8cb762..961addfd 100644
--- a/core/embed/sec/option_bytes/stm32f4/option_bytes.c
+++ b/core/embed/sec/option_bytes/stm32f4/option_bytes.c
@@ -23,7 +23,7 @@
#include <sys/flash_otp.h>
#include <sys/mpu.h>
-#ifdef KERNEL_MODE
+#ifdef SECURE_MODE
#pragma GCC optimize( \
"no-stack-protector") // applies to all functions in this file
@@ -73,7 +73,7 @@ static uint32_t flash_wait_and_clear_status_flags(void) {
return result;
}
-secbool flash_check_option_bytes(void) {
+static secbool flash_check_option_bytes(void) {
flash_wait_and_clear_status_flags();
// check values stored in flash interface registers
if ((FLASH->OPTCR & ~3) !=
@@ -107,11 +107,11 @@ secbool flash_check_option_bytes(void) {
return sectrue;
}
-void flash_lock_option_bytes(void) {
+static void flash_lock_option_bytes(void) {
FLASH->OPTCR |= FLASH_OPTCR_OPTLOCK; // lock the option bytes
}
-void flash_unlock_option_bytes(void) {
+static void flash_unlock_option_bytes(void) {
if ((FLASH->OPTCR & FLASH_OPTCR_OPTLOCK) == 0) {
return; // already unlocked
}
@@ -123,7 +123,7 @@ void flash_unlock_option_bytes(void) {
; // wait until the flash option control register is unlocked
}
-uint32_t flash_set_option_bytes(void) {
+static uint32_t flash_set_option_bytes(void) {
// reference RM0090 section 3.7.2
flash_wait_and_clear_status_flags();
flash_unlock_option_bytes();
@@ -140,7 +140,7 @@ uint32_t flash_set_option_bytes(void) {
return result;
}
-secbool flash_configure_option_bytes(void) {
+secbool option_bytes_configure(void) {
if (sectrue == flash_check_option_bytes()) {
return sectrue; // we DID NOT have to change the option bytes
}
@@ -152,4 +152,4 @@ secbool flash_configure_option_bytes(void) {
return secfalse; // notify that we DID have to change the option bytes
}
-#endif // #ifdef KERNEL_MODE
+#endif // #ifdef SECURE_MODE
diff --git a/core/embed/sec/option_bytes/stm32u5/option_bytes.c b/core/embed/sec/option_bytes/stm32u5/option_bytes.c
index 5e3da802..4a0f6e19 100644
--- a/core/embed/sec/option_bytes/stm32u5/option_bytes.c
+++ b/core/embed/sec/option_bytes/stm32u5/option_bytes.c
@@ -121,7 +121,7 @@ static uint32_t flash_wait_and_clear_status_flags(void) {
return result;
}
-secbool flash_check_option_bytes(void) {
+static secbool flash_check_option_bytes(void) {
flash_wait_and_clear_status_flags();
// check values stored in flash interface registers
if (FLASH->OPTR !=
@@ -169,11 +169,11 @@ secbool flash_check_option_bytes(void) {
return sectrue;
}
-void flash_lock_option_bytes(void) {
+static void flash_lock_option_bytes(void) {
FLASH->NSCR |= FLASH_NSCR_OPTLOCK; // lock the option bytes
}
-void flash_unlock_option_bytes(void) {
+static void flash_unlock_option_bytes(void) {
if ((FLASH->NSCR & FLASH_NSCR_OPTLOCK) == 0) {
return; // already unlocked
}
@@ -185,7 +185,7 @@ void flash_unlock_option_bytes(void) {
; // wait until the flash option control register is unlocked
}
-uint32_t flash_set_option_bytes(void) {
+static uint32_t flash_set_option_bytes(void) {
if (flash_unlock_write() != sectrue) {
return 0;
}
@@ -244,12 +244,12 @@ uint32_t flash_set_option_bytes(void) {
return result;
}
-void check_oem_keys(void) {
+void option_bytes_check_oem_keys(void) {
ensure(((FLASH->NSSR & FLASH_NSSR_OEM1LOCK) == 0) * sectrue, "OEM1 KEY SET");
ensure(((FLASH->NSSR & FLASH_NSSR_OEM2LOCK) == 0) * sectrue, "OEM2 KEY SET");
}
-secbool flash_configure_option_bytes(void) {
+secbool option_bytes_configure(void) {
if (sectrue == flash_check_option_bytes()) {
return sectrue; // we DID NOT have to change the option bytes
}
@@ -258,7 +258,7 @@ secbool flash_configure_option_bytes(void) {
flash_set_option_bytes();
} while (sectrue != flash_check_option_bytes());
- check_oem_keys();
+ option_bytes_check_oem_keys();
return secfalse; // notify that we DID have to change the option bytes
}
Risk score
Our methodology →Why this scored 12/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.