chore(core/embed): assert that bitblt probe lengths cannot overflow
What the developer wrote
Authored by tychovrahe
chore(core/embed): assert that bitblt probe lengths cannot overflow
CHECK_BB_DST and CHECK_BB_SRC multiply a stride by a height into a `size_t`
to get the length they probe. That is safe today only because every dimension
in `gfx_bitblt_t` is 16-bit: an unsigned product needs as many bits as its
operands have together, so 16 + 16 fits a 32-bit `size_t` exactly, with a
maximum product of 4294836225.
The relationship was implicit. Widening any of those fields would turn the
product into a wrapped, much smaller value, and the macros would then probe a
fraction of the memory the blit goes on to touch - the probe would start
approving what it is meant to reject. State the requirement as a static
assert so that change breaks the build instead.
No behavioural change.
[no changelog]
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
What changed, and why it matters
This is a hardening change, not a fix for an active bug. It adds a compile-time safety check to ensure that future changes to graphics dimensions cannot cause a memory-bounds check to silently undercount how much memory a drawing operation will touch. As written today the code is safe; the patch prevents a future, potentially dangerous code change from being accepted without notice.
Treat as a low-risk hardening commit. No immediate patch or incident response is required. Reviewers should verify that the static assertion correctly captures the invariant and that any future widening of gfx_bitblt_t dimensions is accompanied by a checked multiply.
Security signals we found
Defensive hardening of syscall verifier macros
Prevents potential integer overflow in memory-probe length calculation
Compile-time assertion to catch future type changes
No runtime behavior change in current configuration
Evidence from the diff
The commit adds a _Static_assert in core/embed/sys/syscall/stm32/syscall_verifiers.c. The CHECK_BB_DST and CHECK_BB_SRC macros compute probe lengths as size_t = stride * height. The assertion verifies that the sum of the bit widths of dst_stride/height and src_stride/height does not exceed sizeof(size_t). On the current 32-bit target with 16-bit gfx_bitblt_t fields this holds, so there is no runtime behavior change. The intent is to make a future widening of those fields a compile-time failure rather than allowing a silent size_t wraparound that would make the probe approve too little memory.
Changed components
core/embed/sys/syscall/stm32/syscall_verifiers.cCHECK_BB_DST / CHECK_BB_SRC macrosgfx_bitblt_t graphics blit structureInspect captured patch +12 / −0
### core/embed/sys/syscall/stm32/syscall_verifiers.c
@@ -44,6 +44,18 @@
goto access_violation; \
}
+// The macros above multiply a stride by a height into a `size_t`. That cannot
+// overflow while both dimensions stay 16-bit, since an unsigned product needs
+// only as many bits as its operands have together. Widening any of the fields
+// would need a checked multiply here instead.
+_Static_assert(sizeof(((gfx_bitblt_t *)0)->dst_stride) +
+ sizeof(((gfx_bitblt_t *)0)->height) <=
+ sizeof(size_t) &&
+ sizeof(((gfx_bitblt_t *)0)->src_stride) +
+ sizeof(((gfx_bitblt_t *)0)->height) <=
+ sizeof(size_t),
+ "bitblt dimensions may overflow the probe length");
+
// ---------------------------------------------------------------------
void sysevents_poll__verified(const sysevents_t *awaited,Why this scored 31/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.