fix(core): THP write timeout for single packet
What the developer wrote
Authored by Martin Milata
fix(core): THP write timeout for single packet
[no changelog]
What changed, and why it matters
This commit fixes a bug in how the Trezor hardware wallet handles sending data over its USB-like wire protocol. Previously, a single outgoing packet could wait forever for the host to accept it, potentially freezing the device and preventing it from handling other communication channels. The fix adds a 5-second timeout so the device can recover and continue operating. This is a reliability and denial-of-service improvement rather than a fix for theft of funds or keys.
Treat as a low-to-moderate reliability/security fix. Users should update firmware when a release containing this commit is available. No immediate emergency action is warranted because no key extraction or unauthorized transaction evidence is present in the diff.
Security signals we found
Unbounded wait on outgoing I/O removed
Timeout added to wire write operation
Channel kill on persistent write blockage
Denial-of-service / device freeze mitigation
Evidence from the diff
In core/src/trezor/wire/thp/interface_context.py, the THP (Trezor Host Protocol) interface context previously used loop.race() between write_all_packets() and a single global _WRITE_TIMEOUT (loop.sleep(5000)). Because loop.sleep() returns a singleton/future that could fire only once, and because write_all_packets() may send multiple packets, the timeout could effectively be consumed or behave incorrectly on subsequent writes, leaving a single packet write unbounded. The patch removes the global _WRITE_TIMEOUT, adds a timeout_ms parameter directly to the wait() future for POL_WRITE, and replaces the race with a try/except around write_all_packets() catching Timeout. On timeout, it logs an error and kills the active channel. This ensures every write wait is independently bounded by _WRITE_TIMEOUT_MS.
Changed components
core/src/trezor/wire/thp/interface_context.pyTrezor Host Protocol (THP) wire implementationUSB/wire write pathInspect captured patch +13 / −5
diff --git a/core/src/trezor/wire/thp/interface_context.py b/core/src/trezor/wire/thp/interface_context.py
index e9dba547..9bf05886 100644
--- a/core/src/trezor/wire/thp/interface_context.py
+++ b/core/src/trezor/wire/thp/interface_context.py
@@ -4,7 +4,7 @@ from typing import TYPE_CHECKING
import trezorthp
from storage.cache_thp import PREEMPTING_PACKET, clear_sessions_without_channel
from trezor import config, io, loop, utils
-from trezor.loop import race, wait
+from trezor.loop import Timeout, race, wait
from ..protocol_common import ChannelPreemptedException
from . import get_encoded_device_properties
@@ -33,7 +33,6 @@ _PREEMPT_TIMEOUT_MS = const(1_000)
# 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 = loop.sleep(_WRITE_TIMEOUT_MS)
_KEY_REQUIRED_VALS = (trezorthp.KEY_REQUIRED, trezorthp.KEY_REQUIRED_UNLOCK)
@@ -106,7 +105,9 @@ class InterfaceContext:
def __init__(self, iface: WireInterface, thp_ctx: ThpContext) -> None:
self._iface = iface
self._read = wait(iface.iface_num() | io.POLL_READ)
- self._write = wait(iface.iface_num() | io.POLL_WRITE)
+ self._write = wait(
+ iface.iface_num() | io.POLL_WRITE, timeout_ms=_WRITE_TIMEOUT_MS
+ )
# Currently only one active channel is allowed in a session. Without session restart
# this might become a dict[int, Channel].
self.active_channel: Channel | None = None
@@ -290,8 +291,15 @@ class InterfaceContext:
yield self._write_box
if __debug__ and _TRACE:
log.debug(__name__, "write requested", iface=iface)
- result = yield race(self.write_all_packets(), _WRITE_TIMEOUT)
- if isinstance(result, int):
+ try:
+ yield from self.write_all_packets()
+ except Timeout:
+ if __debug__:
+ log.error(
+ __name__,
+ f"write blocked for {_WRITE_TIMEOUT_MS} ms",
+ iface=iface,
+ )
if self.active_channel:
self.active_channel.kill(trezorthp.ThpError("Write is blocked"))
self.clear_closed_sessions()
Why this scored 42/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.