DF-0683 — hci_disable (hci_unit.c) teardown races hci_intr (no lock)

Verdict

SOURCE-CONFIRMED, NOT TESTABLE ON THIS GUEST. The race is unambiguous in source; the trigger requires a registered hci_unit, which requires Bluetooth hardware (or a virtual BT adapter) — absent from this VM. There is also no unprivileged trigger: the realistic race window needs SIOCSBTFLAGS (root) or driver detach (kldunload/unplug) concurrent with active Bluetooth traffic.

Mechanism

sys/netbt/hci_unit.c:217-263 hci_disable:

void
hci_disable(struct hci_unit *unit)
{
    ...
    (*unit->hci_if->disable)(unit->hci_dev);
    unit->hci_flags &= ~BTF_RUNNING;

    /* close down any links ... */
    for (acl = 0 ; acl < 2 ; acl++) {                  // :238
        next = TAILQ_FIRST(&unit->hci_links);
        while ((link = next) != NULL) {
            next = TAILQ_NEXT(link, hl_next);
            if (acl || link->hl_type != HCI_LINK_ACL)
                hci_link_free(link, ECONNABORTED);      // TAILQ_REMOVE + kfree
        }
    }
    while ((memo = LIST_FIRST(&unit->hci_memos)) != NULL)
        hci_memo_free(memo);

    /* (no need to hold hci_devlock, the driver is disabled) */   // :250

    IF_DRAIN(&unit->hci_eventq);                        // :252
    unit->hci_eventqlen = 0;
    IF_DRAIN(&unit->hci_aclrxq);
    unit->hci_aclrxqlen = 0;
    IF_DRAIN(&unit->hci_scorxq);
    unit->hci_scorxqlen = 0;
    IF_DRAIN(&unit->hci_cmdwait);
    IF_DRAIN(&unit->hci_scodone);
}

The comment at :250 ("no need to hold hci_devlock, the driver is disabled") is wrong: hci_intr (sys/netbt/hci_unit.c:328-411) is netisr-driven — btintr in sys/netbt/bt_input.c walks hci_unit_list and calls hci_intr unconditionally on every BT interrupt — and does take hci_devlock (:335) around its eventqlen/queue TAILQ touches. After hci_disable returns, in-flight netisr work can still be running concurrently on another CPU. The teardown happens without hci_devlock, so:

Race (a) — counter underflow + NULL deref

• hci_intr:337 reads eventqlen > 0.
• hci_disable:253 zeroes eventqlen.
• hci_intr:338 IF_DEQUEUE returns NULL (queue drained by IF_DRAIN).
• hci_intr:339 eventqlen-- underflows to 65535 (uint16_t).
• hci_intr:342 KKASSERT(m != NULL) panics — or m->m_pkthdr.len at :345 NULL-derefs.

Race (b) — IF_DRAIN vs IF_DEQUEUE mbuf lifetime

• Concurrent IF_DRAIN and IF_DEQUEUE on the same ifqueue (a singly-linked mbuf list with no synchronization) corrupts the list / frees an mbuf still referenced by the other side → UAF.

Race (c) — hci_links TAILQ walk UAF

• hci_intr:395 does TAILQ_FOREACH(link, &unit->hci_links, hl_next) after releasing hci_devlock at :390.
• hci_disable:238-245 does TAILQ_REMOVE + kfree on the same links with no lock → UAF on struct hci_link.

Trigger (theoretical)

• SIOCSBTFLAGS (sys/netbt/hci_ioctl.c:224) toggling BTF_UP while BT traffic is being processed (root-only).
• Driver detach / kldunload while BT traffic flowing (root-only).

Both paths require root AND a live hci_unit (Bluetooth hardware or virtual adapter). The guest has neither.

Why not testable here

• No Bluetooth hardware / virtual adapter in the VM ⇒ no registered hci_unit ⇒ hci_disable is never called.
• Even with HW, the race needs careful timing of (unprivileged?) traffic vs (root-only) SIOCSBTFLAGS.

So this finding is not live-testable on this guest. The source-level race is unambiguous and high-confidence.

Realistic impact ceiling

• Panic (DoS) via KKASSERT(m != NULL) or NULL-deref m->m_pkthdr.len — Race (a). Deterministic-ish on a busy BT link.
• Speculative UAF on struct hci_link / freed mbufs (Races b, c). With heap grooming, possibly escalatable, but the precondition (root + BT HW) makes this a root→kernel hardening gap rather than an unprivileged privesc.

Realistic class: Medium local DoS (root-triggered, BT-HW-dependent) with speculative UAF that would need a lot of work to convert.

Fix

sys/netbt/hci_unit.c: 1. Acquire hci_devlock around the entire teardown in hci_disable (links, memos, queue drains, counter zeroes). Removes the races by making the teardown atomic w.r.t. hci_intr. 2. Defense-in-depth in hci_intr: after IF_DEQUEUE returns NULL despite *rxqlen > 0, skip the KKASSERT and re-enter (goto another) instead of panicking — so a race that somehow survives the lock doesn't crash.

See fix.diff (git-apply-able, applies cleanly, compiles, kernel boots).

Fix validation (Phase 8)

fix_status: not_testable. The fix.diff applies and compiles as part of the single-fix-combined kernel (6.5-DEVELOPMENT #1 built and booted). The race itself cannot be reproduced on a guest without Bluetooth hardware, so no live before/after measurement is possible here. The fix is the textbook correct approach for "concurrent teardown vs consumer" races.