fix(core/bootloader): improve failed communication behavior
What the developer wrote
Authored by tychovrahe
fix(core/bootloader): improve failed communication behavior
[no changelog]
What changed, and why it matters
This bootloader patch changes how the Trezor device handles a failed USB/communication write. Previously, if sending a packet failed, the bootloader would call an 'ensure' function that effectively halted or panicked the device. Now the function returns a failure status up to the caller instead. This is a hardening improvement that makes the bootloader more resilient to communication errors, but the commit message frames it only as a behavior improvement with no changelog.
Review the callers of codec_send_msg() in the bootloader to confirm that returned secfalse values are handled safely and do not leave the bootloader in an inconsistent or exploitable state. Consider whether this change should be accompanied by a changelog or security note given the bootloader context.
Security signals we found
Removal of ensure() panic on failed communication write
Return-value propagation of write failures in bootloader wire protocol
Bootloader code change affecting message sending path
No changelog entry despite behavioral change
Evidence from the diff
In core/embed/projects/bootloader/wire/codec_v1.c, write_flush() was changed from a void function that calls ensure(sectrue * ok, NULL) on a failed iface->write() to a secbool-returning function that propagates the result. codec_send_msg() now returns the result of write_flush() directly. This removes an unconditional panic/halt on communication failure and allows the caller to decide how to handle a failed packet transmission. The change is small and appears defensive, but without broader context it is unclear whether it fixes a reachable fault path or merely improves robustness.
Changed components
Trezor Core bootloadercore/embed/projects/bootloader/wire/codec_v1.cwire protocol v1 codecUSB/communication packet transmissionInspect captured patch +3 / −5
diff --git a/core/embed/projects/bootloader/wire/codec_v1.c b/core/embed/projects/bootloader/wire/codec_v1.c
index 7a570218..9ed0f90e 100644
--- a/core/embed/projects/bootloader/wire/codec_v1.c
+++ b/core/embed/projects/bootloader/wire/codec_v1.c
@@ -91,7 +91,7 @@ static bool write(pb_ostream_t *stream, const pb_byte_t *buf, size_t count) {
return true;
}
-static void write_flush(packet_write_state_t *state) {
+static secbool write_flush(packet_write_state_t *state) {
size_t packet_size = state->iface->tx_packet_size;
// if packet is not filled up completely
@@ -101,7 +101,7 @@ static void write_flush(packet_write_state_t *state) {
}
// send packet
bool ok = state->iface->write(state->buf, packet_size);
- ensure(sectrue * (ok), NULL);
+ return sectrue * ok;
}
secbool codec_send_msg(wire_iface_t *iface, uint16_t msg_id,
@@ -144,9 +144,7 @@ secbool codec_send_msg(wire_iface_t *iface, uint16_t msg_id,
return secfalse;
}
- write_flush(&state);
-
- return sectrue;
+ return write_flush(&state);
}
static void read_retry(wire_iface_t *iface, uint8_t *buf) {
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.