fix(core): stop ble_read() consuming a packet it cannot return
What the developer wrote
Authored by tychovrahe
fix(core): stop ble_read() consuming a packet it cannot return
`ble_read()` dequeued from the RX queue and only then checked that the caller's
buffer was big enough. `tsqueue_dequeue()` resets the entry and advances the
read index unconditionally, and there is no re-enqueue on the error path, so a
buffer below `BLE_RX_PACKET_SIZE` silently destroyed a received packet and
reported zero bytes - indistinguishable to the caller from an empty queue.
The size test never needed the dequeued data. Once `read_len` is known to be
`BLE_DATA_SIZE`, `max_len < read_len - BLE_DATA_HEADER_SIZE` is just
`max_len < BLE_RX_PACKET_SIZE`, so hoist it above the dequeue. That also
retires an expression that was only in bounds thanks to the `||`
short-circuit ordering.
`py_iface_read()` rejects a short buffer before it gets here and the bootloader
wire layer requires an exact size, so the only way in is an applet calling the
`ble_read` syscall directly - which loses packets addressed to itself. No
caller changes behaviour: for every `max_len` at or above the packet size the
function does exactly what it did.
[no changelog]
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
What changed, and why it matters
This commit fixes a bug in the Bluetooth code of Trezor hardware wallets. When a program asked to read a Bluetooth packet using a buffer that was too small, the code would first remove the packet from the receive queue and only then realize the buffer was too small. Because the removed packet was not put back, it was silently lost. The fix checks the buffer size before removing the packet, so an undersized read now simply reports 'nothing available' instead of destroying data. The commit notes that the only realistic way to trigger this is an applet making a direct system call with a too-small buffer, which would cause that applet to lose packets meant for it.
Treat as a low-severity reliability/defensive fix. No urgent action required beyond normal patch uptake. If the project tracks security-relevant fixes, this could be noted as a minor hardening change for the BLE syscall interface.
Security signals we found
Silent data loss / packet drop on undersized buffer
Race-free logic bug in Bluetooth RX queue handling
Potential denial-of-service against applet expecting BLE packets
Fix is defensive and preserves queue state on error path
Evidence from the diff
In core/embed/io/ble/stm32/ble.c, ble_read() previously called tsqueue_dequeue() before validating max_len. tsqueue_dequeue() resets the slot and advances the read index unconditionally, so if max_len < BLE_RX_PACKET_SIZE, the packet was consumed and discarded with no way to re-enqueue, returning 0 bytes as if the queue were empty. The patch hoists the max_len < BLE_RX_PACKET_SIZE check before the dequeue, preserving the packet. The remaining post-dequeue check only validates read_len == BLE_DATA_SIZE. The commit message states that py_iface_read() already rejects short buffers and the bootloader wire layer requires exact sizes, so the only reachable path is an applet invoking the ble_read syscall directly.
Changed components
core/embed/io/ble/stm32/ble.cTrezor Core Bluetooth LE receive pathble_read() syscall implementationInspect captured patch +6 / −2
### core/embed/io/ble/stm32/ble.c
@@ -921,6 +921,11 @@ uint32_t ble_read(uint8_t *data, uint16_t max_len) {
return 0;
}
+ if (max_len < BLE_RX_PACKET_SIZE) {
+ // the packet would not fit; leave it in the queue rather than drop it
+ return 0;
+ }
+
irq_key_t key = irq_lock();
tsqueue_t *queue = &drv->rx_queue;
@@ -931,8 +936,7 @@ uint32_t ble_read(uint8_t *data, uint16_t max_len) {
tsqueue_dequeue(queue, rx_data, sizeof(rx_data), &read_len, NULL);
- if (read_len != BLE_DATA_SIZE ||
- max_len < (read_len - BLE_DATA_HEADER_SIZE)) {
+ if (read_len != BLE_DATA_SIZE) {
irq_unlock(key);
return 0;
}Why this scored 45/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.