# DF-0560 — hci_event_command_compl reads `status` byte beyond mbuf

## Verdict

**NOT REPRODUCED LIVE — but bug confirmed by source trace.** The path
requires a Bluetooth controller (real HW or virtual HCI device) to send a
Command Complete event with a 3-byte payload to the kernel's BT stack.
The audit guest has no BT HW and no virtual-HCI driver; the netbt.ko
module IS shipped but unreachable without a controller.

## Mechanism (cited path:line, confirmed by trace)

`sys/netbt/hci_event.c` line-by-line:

```c
293: static void
294: hci_event_command_compl(struct hci_unit *unit, struct mbuf *m)
295: {
296:     hci_command_compl_ep ep;             // sizeof = 3 (1 + 2)
297:     hci_status_rp rp;                    // sizeof = 1
298:
299:     KKASSERT(m->m_pkthdr.len >= sizeof(ep));   // asserts >= 3 bytes
300:     m_copydata(m, 0, sizeof(ep), &ep);        // copies 3 bytes
301:     m_adj(m, sizeof(ep));                     // trims 3 bytes off front
302:
...
308:     /*
309:      * I am not sure if this is completely correct, it is not guaranteed
310:      * that a command_complete packet will contain the status though most
311:      * do seem to.
312:      */
313:     m_copydata(m, 0, sizeof(rp), &rp);        // <--- BUG: unconditional
```

After `m_adj(m, sizeof(ep))`, the mbuf has `(pkthdr.len - 3)` bytes
remaining. The KKASSERT only verified the **pre-`m_adj`** length was
`>= 3`. If the original event had exactly 3 bytes of payload (i.e. a
Command Complete event with `num_cmd_pkts + opcode` and NO return
parameters, which is legal per the BT spec for commands that have no
return params), the trimmed mbuf has 0 bytes.

`m_copydata(m, 0, sizeof(rp)=1, &rp)` then tries to read 1 byte from
the now-empty mbuf. In `sys/kern/uipc_mbuf.c:1671-1696`:

```c
1686:    while (len > 0) {
1687:        KASSERT(m != NULL,("%s: length > size of mbuf chain", __func__));
1688:        count = min(m->m_len - off, len);          // 0 for empty mbuf
1689:        bcopy(mtod(m, caddr_t) + off, cp, count);   // copies 0 bytes
1690:        len -= count;                                // len unchanged
1691:        cp += count;
1692:        off = 0;
1693:        m = m->m_next;                               // NULL for single mbuf
1694:    }
```

- On **INVARIANTS** kernels (default GENERIC): the next loop iteration
  hits `KASSERT(m != NULL)` at line 1687 and panics with
  `m_copydata: length > size of mbuf chain`. (KKASSERT expands to an
  inline panic, so `nm /boot/kernel/kernel | grep KASSERT == 0` is a
  false-negative — the check IS compiled in.)
- On **non-INVARIANTS** kernels: `count = 0`, `bcopy` copies nothing,
  but the next iteration dereferences `m = NULL` → page fault at
  `m->m_len` (offset 0 from NULL) → kernel panic.

Either way: **remote unauthenticated DoS** triggered by a single
malformed Command Complete HCI event packet from a paired BT controller.

The bug is reachable BEFORE the per-opcode handler switch at line 324
(which is where per-opcode KKASSERTs would catch short events); the
generic unconditional `m_copydata(...sizeof(rp)...)` at line 313 runs
regardless of opcode.

## Why not testable on this guest

`hci_event_command_compl` is invoked from `hci_event_handler`
(`sys/netbt/hci_event.c:160`) when an HCI event packet arrives from a
registered BT controller. The dispatch path is:

  ubt(4) USB driver  →  ng_hci / hci_recv  →  hci_event_handler  →
  hci_event_command_compl

The audit guest has:
- No USB Bluetooth dongle (QEMU has no BT controller attached).
- No virtual-HCI driver (DragonFly does not ship a `vhci` equivalent
  of BlueZ's `hci_vhci` that would let userland inject HCI events).

`netbt.ko` can be `kldload`ed but it has no event source, so the
`hci_event_handler` path is dead code on this guest.

## Recommended fix (validated compile-only)

`fix.diff` wraps the unconditional `m_copydata` in a length check,
zero-initializing `rp` first so a missing status byte is treated as
"success" (the existing code's comment already admits the status is
optional):

```c
memset(&rp, 0, sizeof(rp));
if (m->m_pkthdr.len >= sizeof(rp))
    m_copydata(m, 0, sizeof(rp), &rp);
```

This matches the finding proposal (`if(pkthdr.len>=sizeof(rp)) m_copydata`)
and adds the zero-init as a defense-in-depth (avoids uninitialized
`rp.status` if the length check fails). Validated by:

1. `patch -p3 < fix.diff` → `Hunk #1 succeeded at 309.`
2. `make` in `/usr/src/sys/netbt` → builds cleanly
   (`/usr/obj/usr/src/sys/netbt/netbt.ko`, 105024 bytes, all 18 .o
   units compiled, no warnings).

Cannot load+test on the audit guest (no event source) so
`fix_status: not_testable`. The patch is a clear defensive improvement
and the source-trace shows it closes the only path to the
unconditional `m_copydata`.

## Kernel references (verified by source trace)

- `sys/netbt/hci_event.c:296-317` — vulnerable function.
- `sys/netbt/hci_event.c:299`    — KKASSERT ensures only `>= sizeof(ep)`.
- `sys/netbt/hci_event.c:301`    — `m_adj` trims sizeof(ep).
- `sys/netbt/hci_event.c:313`    — **unconditional** `m_copydata` of 1 byte.
- `sys/kern/uipc_mbuf.c:1687`    — KASSERT that panics on INVARIANTS.
- `sys/netbt/hci.h:1955-1960`    — `hci_command_compl_ep` is 3 bytes.
- `sys/netbt/hci.h:443-448`      — `hci_status_rp` is 1 byte.

## Threat model

Remote unauthenticated BT DoS. Requires a paired BT controller (which
is the attacker in the threat model — a malicious BT device posing as
a controller). The HCI event path is parsed before any authentication
state is established, so any BT peer that can deliver a Command
Complete event triggers the panic. No local-priv-esc chain (no write
primitive). Not reachable on the audit guest.

## PoC

A PoC for this bug would require either:
1. A malicious USB BT dongle (HW-in-the-loop), or
2. A virtual-HCI driver that lets userland inject HCI events (not
   shipped on DragonFly).

Neither is available on the audit guest. The `bttest.c` skeleton in
this folder shows the conceptual trigger (open AF_BLUETOOTH /
PF_BLUETOOTH socket, send a Command Complete event) but cannot run to
completion here because no BT controller is attached. The fix
validation is therefore compile-only + source-trace.