fix(core): unblock THP loop after 5s write timeout
What the developer wrote
Authored by Roman Zeyde
fix(core): unblock THP loop after 5s write timeout
Stop retransmission if writes are blocked - e.g. due to USB flow control.
It allows restarting the event loop to handle other THP channels.
`loop.race()` is used, since it schedules a single timeout task per message - instead of `loop.wait.timeout_ms`, which schedules a timeout task once per packet.
[no changelog]
What changed, and why it matters
This change fixes a bug in the Trezor hardware wallet's THP (Trezor Host Protocol) communication layer. Previously, if the device was trying to resend a message but the USB host stopped accepting data (for example due to USB flow control), the retransmission loop could get stuck forever, blocking other THP channels. The patch adds a 5-second timeout so the loop can break out and let other communication channels proceed. It is a reliability/availability fix rather than a clear-cut vulnerability patch, though the stuck-loop condition could be abused to deny service to other channels.
Treat as a hardening/reliability fix. Review whether the 5-second timeout is appropriate for all host environments and whether raising Timeout is handled correctly by callers to avoid unintended side effects. No immediate security advisory is required unless vendor or reporter classifies it as a vulnerability.
Security signals we found
Denial-of-service mitigation: blocked USB write could previously stall the event loop indefinitely
Timeout added to bounded retransmission loop (50 iterations)
Exception type changed from ThpError to Timeout to signal event-loop restart
Communication protocol layer (THP) affected
Evidence from the diff
In core/src/trezor/wire/thp/channel.py, the _retransmit loop now races self.ctx.write_payload(header, payload) against a 5-second sleep. If write_payload does not complete within 5 seconds, the race returns an int (the timeout task result), the loop breaks, and a Timeout exception is raised instead of the previous ThpError(‘Retransmission timeout’). The intent is to prevent a blocked USB write from starving the event loop and stalling other THP channels. The change replaces a per-packet timeout approach with a single timeout per message using loop.race().
Changed components
core/src/trezor/wire/thp/channel.pyTrezor Host Protocol (THP) channel retransmission logicUSB/THP write pathInspect captured patch +14 / −3
diff --git a/core/src/trezor/wire/thp/channel.py b/core/src/trezor/wire/thp/channel.py
index 4ac08a87a..01676bb20 100644
--- a/core/src/trezor/wire/thp/channel.py
+++ b/core/src/trezor/wire/thp/channel.py
@@ -21,7 +21,7 @@ from storage.cache_thp import (
is_there_a_channel_to_replace,
)
from trezor import protobuf, utils, workflow
-from trezor.loop import Timeout
+from trezor.loop import Timeout, race, sleep
from ..protocol_common import Message
from . import (
@@ -58,6 +58,11 @@ if TYPE_CHECKING:
_MAX_RETRANSMISSION_COUNT = const(50)
_MIN_RETRANSMISSION_COUNT = const(2)
+# Stop retransmission if writes are blocked - e.g. due to USB flow control.
+# It allows restarting the event loop to handle other THP channels.
+_WRITE_TIMEOUT_MS = const(5_000)
+_WRITE_TIMEOUT = sleep(_WRITE_TIMEOUT_MS)
+
class Reassembler:
def __init__(self, cid: int, read_buf: ThpBuffer) -> None:
@@ -388,7 +393,12 @@ class Channel:
ABP.set_sending_allowed(self.channel_cache, False)
for i in range(_MAX_RETRANSMISSION_COUNT):
- await self.ctx.write_payload(header, payload)
+ result = await race(self.ctx.write_payload(header, payload), _WRITE_TIMEOUT)
+ if isinstance(result, int):
+ if __debug__:
+ log.error(__name__, "Sending is stuck for %d ms", _WRITE_TIMEOUT_MS)
+ break
+
# starting from 100ms till ~3.42s
timeout_ms = round(10200 - 1010000 / (100 + i))
try:
@@ -403,7 +413,8 @@ class Channel:
ABP.set_send_seq_bit_to_opposite(self.channel_cache)
return
- raise ThpError("Retransmission timeout")
+ # restart event loop due to unresponsive channel
+ raise Timeout("THP retransmission timeout")
def _encrypt(self, buffer: utils.BufferType, noise_payload_len: int) -> None:
if __debug__:
Why this scored 47/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.