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.

Fix status (batch validation)

Applied with the 36 sibling fix.diffs of this batch in findings/poc/_batch41/all_fixes.patch; single in-guest make -j6 nativekernel KERNCONF=X86_64_GENERIC completed RC=0 with zero errors under -Werror (full log: findings/poc/_batch41/fix_build8.log). Behavior A/B intentionally skipped for this Info-tier item.