refactor(core): startup args api improvement
What changed, and why it matters
This commit is a code cleanup in the Trezor firmware's bootloader startup system. It replaces a fixed-size on-stack certificate buffer with a new 'reserve/commit/discard' API that lets the bootloader allocate space for the MCU attestation certificate directly in the shared startup-arguments buffer. There is no indication in the commit or supplied references that this fixes a security bug; it appears to be a defensive refactor to avoid large stack buffers and make the API more flexible.
No immediate security action is required. Treat as a normal code-quality refactor. If reviewing for a security release, confirm that the new reserve/commit/discard state machine is used correctly everywhere and that `startup_args_discard()` is called on all error paths to avoid leaving a pending reservation.
Security signals we found
Eliminates a large fixed-size stack buffer for the MCU attestation certificate, reducing stack pressure and potential for stack-related issues
Adds explicit reservation state (`g_reservation_pending`) and bounds checks (`entry->size >= size`, `size <= UINT16_MAX`) to the startup-args allocator
No changelog entry and title frames the change as a refactor/API improvement, not a security fix
Evidence from the diff
The change refactors startup_args_add() in core/embed/sys/startup/startup_args.c to be implemented on top of three new primitives: startup_args_reserve(), startup_args_commit(), and startup_args_discard(). The reserve function allocates an arg_entry_t in the output buffer, zeros the value area, and returns a pointer so the caller can write in place. commit() finalizes the entry size and advances the buffer; discard() cancels a pending reservation. In core/embed/projects/bootloader/main.c, the previous code that read the MCU device certificate into a uint8_t mcu_device_cert[MCU_ATTESTATION_MAX_CERT_SIZE] stack buffer and then called startup_args_add() is replaced by pass_mcu_attestation_cert(), which first queries the certificate size, reserves space, reads the certificate directly into the reserved buffer, and commits. g_args_out_ptr is also made static.
Changed components
core/embed/sys/startup/startup_args.ccore/embed/sys/startup/inc/sys/startup_args.hcore/embed/projects/bootloader/main.cInspect captured patch +151 / −17
diff --git a/core/embed/projects/bootloader/main.c b/core/embed/projects/bootloader/main.c
index 9404cbbd..777a1bc3 100644
--- a/core/embed/projects/bootloader/main.c
+++ b/core/embed/projects/bootloader/main.c
@@ -380,6 +380,41 @@ static void drivers_deinit(void) {
void failed_jump_to_firmware(void) { error_shutdown("(glitch)"); }
+#ifdef USE_MCU_ATTESTATION
+ts_t pass_mcu_attestation_cert(void) {
+ TSH_DECLARE;
+
+ ts_t status;
+ secbool ok;
+ void *buffer = NULL;
+ size_t cert_size = 0;
+
+ ok = secret_mcu_device_cert_size(&cert_size);
+ TSH_CHECK(ok == sectrue, TS_ENOENT);
+
+ status = startup_args_reserve(STARTUP_ARGS_TYPE_MCU_DEVICE_CERT, cert_size,
+ &buffer);
+ TSH_CHECK_OK(status);
+
+ ok = secret_mcu_device_cert_read(buffer, cert_size, &cert_size);
+ TSH_CHECK(ok == sectrue, TS_EIO);
+
+ status = startup_args_commit(cert_size);
+ TSH_CHECK_OK(status);
+
+ buffer = NULL;
+
+cleanup:
+
+ if (buffer != NULL) {
+ startup_args_discard();
+ }
+
+ TSH_RETURN;
+}
+
+#endif
+
void real_jump_to_firmware(void) {
const image_header *hdr = NULL;
vendor_header vhdr = {0};
@@ -445,16 +480,7 @@ void real_jump_to_firmware(void) {
#endif
#ifdef USE_MCU_ATTESTATION
- {
- uint8_t mcu_device_cert[MCU_ATTESTATION_MAX_CERT_SIZE];
- size_t mcu_device_cert_size = 0;
- if (sectrue == secret_mcu_device_cert_read(mcu_device_cert,
- sizeof(mcu_device_cert),
- &mcu_device_cert_size)) {
- startup_args_add(STARTUP_ARGS_TYPE_MCU_DEVICE_CERT, mcu_device_cert,
- mcu_device_cert_size);
- }
- }
+ pass_mcu_attestation_cert();
#endif
#ifdef USE_SECRET
diff --git a/core/embed/sys/startup/inc/sys/startup_args.h b/core/embed/sys/startup/inc/sys/startup_args.h
index eac79de0..519b600b 100644
--- a/core/embed/sys/startup/inc/sys/startup_args.h
+++ b/core/embed/sys/startup/inc/sys/startup_args.h
@@ -62,9 +62,62 @@ typedef enum {
* @return TS_OK on success, or an error code on failure:
* TS_ENOMEM if the output buffer does not have enough space
* TS_EEXIST if an entry with the same type already exists in the buffer
+ * TS_EBUSY if there is a pending reservation
*/
ts_t startup_args_add(startup_args_type_t type, const void* value, size_t size);
+/*
+ * @brief Reserves space for an argument in the output buffer to be passed to
+ * the next stage of the boot process, and returns a pointer to the reserved
+ * space where the caller can write the argument value directly.
+ *
+ * This function allows the caller to write the argument value directly into the
+ * output buffer without needing to copy it from a separate location. The caller
+ * must call `startup_args_commit()` after writing the value to finalize the
+ * addition of the argument to the output buffer. If the caller decides not to
+ * add the argument after reserving space, it must call `startup_args_discard()`
+ *
+ * @param type Argument type (defined by the caller)
+ * @param size Size of the argument value in bytes
+ * @param buffer Output pointer to store the address of the reserved space for
+ * the argument value. The caller can write the value directly to this address.
+ *
+ * @return TS_OK on success, or an error code on failure:
+ * TS_ENOMEM if the output buffer does not have enough space
+ * TS_EEXIST if an entry with the same type already exists in the buffer
+ * TS_EBUSY if there is already a pending reservation
+ */
+ts_t startup_args_reserve(startup_args_type_t type, size_t size, void** buffer);
+
+/*
+ * @brief Commits the previously reserved argument in the output buffer.
+ *
+ * This function must be called after `startup_args_reserve()` to finalize the
+ * addition of the argument to the output buffer. It updates the size of the
+ * reserved entry to the actual size of the argument value written by the
+ * caller, and advances the buffer size to account for the new entry. If this
+ * function fails, the reservation remains pending and the caller must either
+ * retry with valid arguments or call `startup_args_discard()`.
+ *
+ * @param size Size of the argument value in bytes (must match or be less than
+ * the size used in the previous call to `startup_args_reserve()`)
+ *
+ * @return TS_OK on success, or an error code on failure:
+ * TS_EINVAL if there is no pending reservation, or if the size is
+ * greater than the size reserved in the previous call to
+ * `startup_args_reserve()`
+ */
+ts_t startup_args_commit(size_t size);
+
+/*
+ * @brief Discards the previously reserved argument in the output buffer.
+ *
+ * This function must be called if the caller decides not to add the reserved
+ * argument to the output buffer. It resets the reservation state, allowing
+ * subsequent calls to `startup_args_reserve()` to succeed.
+ */
+void startup_args_discard(void);
+
/*
* @brief Retrieves the pointer to the output arguments structure that can
* be passed to the next stage of the boot process.
diff --git a/core/embed/sys/startup/startup_args.c b/core/embed/sys/startup/startup_args.c
index 6251fd12..f1a0763b 100644
--- a/core/embed/sys/startup/startup_args.c
+++ b/core/embed/sys/startup/startup_args.c
@@ -58,7 +58,11 @@ static struct {
// call to `startup_args_add()`. If `startup_args_add()` is not called at all,
// this will remain NULL and the output buffer and `g_args_buffer`
// will be optimized out by the linker.
-startup_args_t* g_args_out_ptr;
+static startup_args_t* g_args_out_ptr;
+
+// Flag indicating whether there is a pending reservation that has not
+// been committed yet.
+static bool g_reservation_pending = false;
// Input arguments passed from the previous stage, initialized with
// `startup_args_import()` and accessed with `startup_args_get()`.
@@ -80,6 +84,34 @@ ts_t startup_args_add(startup_args_type_t type, const void* value,
size_t size) {
TSH_DECLARE;
+ void* dest = NULL;
+
+ TSH_CHECK_ARG(size == 0 || value != NULL);
+
+ ts_t status = startup_args_reserve(type, size, &dest);
+ TSH_CHECK_OK(status);
+
+ if (size > 0) {
+ memcpy(dest, value, size);
+ }
+
+ startup_args_commit(size);
+
+cleanup:
+ TSH_RETURN;
+}
+
+ts_t startup_args_reserve(startup_args_type_t type, size_t size,
+ void** buffer) {
+ TSH_DECLARE;
+
+ *buffer = NULL;
+
+ TSH_CHECK_ARG(size <= UINT16_MAX);
+ TSH_CHECK_ARG(type != STARTUP_ARGS_TYPE_INVALID);
+
+ TSH_CHECK(!g_reservation_pending, TS_EBUSY);
+
startup_args_t* args = g_args_out_ptr;
if (args == NULL) {
@@ -90,10 +122,6 @@ ts_t startup_args_add(startup_args_type_t type, const void* value,
g_args_out_ptr = args;
}
- TSH_CHECK_ARG(size <= UINT16_MAX);
- TSH_CHECK_ARG(type != STARTUP_ARGS_TYPE_INVALID);
- TSH_CHECK_ARG(size == 0 || value != NULL);
-
TSH_CHECK(find_entry(args, type) == NULL, TS_EEXIST);
uint32_t entry_size = ARG_ENTRY_SIZE(size);
@@ -102,16 +130,43 @@ ts_t startup_args_add(startup_args_type_t type, const void* value,
arg_entry_t* entry = (arg_entry_t*)(args->data + args->size);
entry->type = type;
entry->size = size;
+
if (size > 0) {
- memcpy(entry->value, value, size);
+ // Zero out the reserved space for the argument value
+ memset(entry->value, 0, size);
}
- args->size += entry_size;
+ *buffer = entry->value;
+
+ g_reservation_pending = true;
cleanup:
TSH_RETURN;
}
+ts_t startup_args_commit(size_t size) {
+ TSH_DECLARE;
+
+ startup_args_t* args = g_args_out_ptr;
+
+ TSH_CHECK(g_reservation_pending, TS_EINVAL);
+ TSH_CHECK(args != NULL, TS_EINVAL);
+
+ arg_entry_t* entry = (arg_entry_t*)(args->data + args->size);
+
+ TSH_CHECK(entry->size >= size, TS_EINVAL);
+
+ entry->size = size;
+ args->size += ARG_ENTRY_SIZE(size);
+
+ g_reservation_pending = false;
+
+cleanup:
+ TSH_RETURN;
+}
+
+void startup_args_discard(void) { g_reservation_pending = false; }
+
const startup_args_t* startup_args_export(void) { return g_args_out_ptr; }
ts_t startup_args_import(const startup_args_t* args) {
Why this scored 12/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.