fix(core): fix double evaluation of status in TSH_CHECK_xxx
What the developer wrote
Authored by cepetr
fix(core): fix double evaluation of status in TSH_CHECK_xxx
[no changelog]
What changed, and why it matters
This commit fixes a subtle C macro bug where a status value could be evaluated twice. In some cases, the value used for logging could differ from the value actually stored as the error result, which might hide or misreport what went wrong inside the Trezor firmware. The patch stores the status in a variable first, then logs that same stored value so the two always match.
Review all call sites of TSH_CHECK, TSH_CHECK_ARG, and TSH_CHECK_SEC to confirm no status expression relies on the prior double-evaluation behavior, and verify that downstream error handling and logging now behave consistently. Consider adding unit tests that pass side-effecting expressions to these macros.
Security signals we found
Double-evaluation bug in C preprocessor macro
Potential inconsistency between logged error code and returned error code
Side-effect-sensitive status expression handling
Error-handling path in embedded runtime library
Evidence from the diff
The TSH_CHECK_xxx family of macros previously evaluated the status argument twice: once inside TSH_LOG_((status)) and once in the assignment __status = status. If status is an expression with side effects, or if the two evaluations could yield different values (e.g., volatile reads, concurrent state changes, or macro expansion subtleties), the logged status and the returned status could diverge. The patch reorders the statements so __status is assigned first, then TSH_LOG_((__status)) uses the stored value, guaranteeing a single evaluation of the original expression and consistency between the recorded error and the logged error.
Changed components
core/embed/rtl/inc/rtl/error_handling.hTSH_CHECK macroTSH_CHECK_ARG macroTSH_CHECK_SEC macroInspect captured patch +4 / −4
diff --git a/core/embed/rtl/inc/rtl/error_handling.h b/core/embed/rtl/inc/rtl/error_handling.h
index 3d8fbc00..558d96ae 100644
--- a/core/embed/rtl/inc/rtl/error_handling.h
+++ b/core/embed/rtl/inc/rtl/error_handling.h
@@ -237,8 +237,8 @@ __fatal_error(const char *msg, const char *file, int line);
do { \
ts_t _status = status; \
if (ts_error(_status)) { \
- TSH_LOG_((status)); \
__status = _status; \
+ TSH_LOG_((__status)); \
goto cleanup; \
} \
} while (0)
@@ -253,8 +253,8 @@ __fatal_error(const char *msg, const char *file, int line);
#define TSH_CHECK(cond, status) \
do { \
if (!(cond)) { \
- TSH_LOG_((status)); \
__status = status; \
+ TSH_LOG_((__status)); \
goto cleanup; \
} \
} while (0)
@@ -268,8 +268,8 @@ __fatal_error(const char *msg, const char *file, int line);
#define TSH_CHECK_ARG(cond) \
do { \
if (!(cond)) { \
- TSH_LOG_(TS_EINVAL); \
__status = TS_EINVAL; \
+ TSH_LOG_(__status); \
goto cleanup; \
} \
} while (0)
@@ -284,8 +284,8 @@ __fatal_error(const char *msg, const char *file, int line);
#define TSH_CHECK_SEC(seccond, status) \
do { \
if ((seccond) != sectrue) { \
- TSH_LOG_((status)); \
__status = status; \
+ TSH_LOG_((__status)); \
goto cleanup; \
} \
} while (0)
Why this scored 44/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.