fix(core): Fix broken Tropic initialization in prodtest.
What the developer wrote
Authored by Andrew Kozlik
fix(core): Fix broken Tropic initialization in prodtest.
Partially reverts 6cc6a8779a9a7bb002a76f87dda4d1ed3ff58c8b.
[no changelog]
(cherry picked from commit efcb0f1944c67742dd155c22a11c61cc51791a0a)
What changed, and why it matters
This commit fixes a bug in the factory testing tool (prodtest) for Trezor hardware wallets that use a Tropic secure chip. The previous code accidentally marked the Tropic chip as initialized even when initialization actually failed, and it also shut down the chip during cleanup even on success. The fix makes the success/failure status accurate and stops the premature shutdown, so production tests can run correctly.
Treat as a functional bug fix rather than an active security vulnerability. Verify that prodtest now correctly initializes the Tropic chip and that no other callers depend on the old drv->initialized behavior. No urgent security patch is indicated for end-user firmware.
Security signals we found
Secure-element initialization state confusion
Potential teardown of valid secure session due to incorrect cleanup path
Production test (prodtest) only, not normal device firmware runtime
Fix is a partial revert of a prior commit that introduced the regression
Evidence from the diff
The patch corrects state management in core/embed/sec/tropic/tropic.c. Previously, session_start() set drv->initialized = false at entry and drv->initialized = true on success, returning that flag. If an error occurred before the success assignment, the function returned false, but callers relying on drv->initialized could see stale state. More importantly, tropic_init() called lt_init(), then on any failure jumped to cleanup which called tropic_deinit(); because tropic_deinit() likely checks drv->initialized, and session_start had not yet run, drv->initialized was false, so deinit was a no-op. However, the real bug introduced by the reverted commit appears to be that tropic_init() unconditionally executed cleanup -> tropic_deinit() and returned false, even after successful initialization and session_start, effectively tearing down a working Tropic session. The fix introduces an explicit ret variable, sets drv->initialized = true immediately after lt_init succeeds, and removes the cleanup path so a successful init no longer returns false. This is described as a fix for ‘broken Tropic initialization in prodtest’ and is a partial revert of an earlier change.
Changed components
core/embed/sec/tropic/tropic.cTropic secure chip driverTrezor Core prodtestInspect captured patch +5 / −6
diff --git a/core/embed/sec/tropic/tropic.c b/core/embed/sec/tropic/tropic.c
index 36d162d0a..b848e8bba 100644
--- a/core/embed/sec/tropic/tropic.c
+++ b/core/embed/sec/tropic/tropic.c
@@ -61,7 +61,7 @@ static bool tropic_get_tropic_pubkey(lt_handle_t *handle,
static bool session_start(tropic_driver_t *drv,
pkey_index_t pairing_key_index) {
- drv->initialized = false;
+ bool ret = false;
curve25519_key trezor_private = {0};
switch (pairing_key_index) {
@@ -101,12 +101,12 @@ static bool session_start(tropic_driver_t *drv,
}
drv->pairing_key_index = pairing_key_index;
- drv->initialized = true;
+ ret = true;
cleanup:
memzero(trezor_private, sizeof(trezor_private));
- return drv->initialized;
+ return ret;
}
bool tropic_init(void) {
@@ -123,8 +123,9 @@ bool tropic_init(void) {
#endif
if (lt_init(&drv->handle) != LT_OK) {
- goto cleanup;
+ return false;
}
+ drv->initialized = true;
// Wait for Tropic to boot before issuing any session commands.
uint32_t boot_start_ms = hal_ticks_ms();
@@ -149,8 +150,6 @@ bool tropic_init(void) {
}
#endif
-cleanup:
- tropic_deinit();
return false;
}
Why this scored 32/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.