acassis opened a new pull request, #20347:
URL: https://github.com/apache/nuttx/pull/20347
## Summary
Two problems in hci_num_completed_packets().
Number_of_Handles is a single octet, but it was read with BT_LE162HOST(),
which takes the first octet of the handle that follows it as the high byte. A
one-octet field could therefore produce a loop count of up to 65535.
The loop was then bounded only by that count and not by the data that was
actually received, so it walked past the end of the event, reading handle and
count pairs out of whatever followed it.
Read the field at its declared width, and require the pairs the event claims
to have been received before reading them.
Per-connection credit accounting, which this handler still does not do, is a
separate change.
## Impact
Improvement
## Testing
Before:
```
hci_num_completed_packets: num_handles 64
hci_num_completed_packets: handle 4095 count 1 ← i=0, the one real
pair
hci_num_completed_packets: handle 0 count 0 ← i≥1, past the
5-octet
event
...
hci_num_completed_packets: handle 2049 count 8224 ← deeper reads hit
live
memory
hci_num_completed_packets: handle 55424 count 16393
The loop is bounded only by the declared count, so it runs 64 iterations
for a
1-pair event — 63 of them read evt->h[i] past the end, into adjacent then
live
memory (D-62). The garbage count values also drove while(count--)
nxsem_post(),
over-posting the semaphore; the simulator was left pegged at ~97% CPU and
had to
be killed.
```
After:
```
hci_num_completed_packets: ERROR: Event declares 64 handles but carries 5
octets
bt_buf_release: Buffer freed
Guard: 5 < 1 + 64*4 = 257 → true → dropped before the loop. Zero
iterations,
simulator keeps running.
```
What the test revealed about the patch
The runtime showed num_handles = 64, not the "up to 65535" the commit
message
claims. BT_LE162HOST(le) is defined as (le) on little-endian (identity),
so on
the sim — and on the little-endian ESP32-S3 — it evaluates the promoted
one-octet value and never reads the following handle's octet as a high
byte. So:
- The width change (dropping BT_LE162HOST) is a no-op on little-endian; it
only
matters on big-endian (where it'd byteswap 64 → 16384 and enlarge the
overread).
- The actual memory-safety fix is the bounds check, which is what this test
exercises.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
