# DF-0745 — Verdict

## Verdict: REPRODUCED (code-level harness) + FIX VALIDATED

The bug is **real and confirmed by source tracing plus a deterministic
userspace harness** that transcribes the two racing threads verbatim from
`sys/netbt/l2cap_misc.c` and `sys/kern/kern_timeout.c`. The runtime L2CAP
path is unreachable on this KVM guest (no Bluetooth radio; `BLUETOOTH` not in
`X86_64_GENERIC`), so the proof is the harness — the same precedent as the
DF-0393/0594/0616/0732/0733 wifi/netgraph/bt-unreachable cluster. The
authored `fix.diff` compiles cleanly into `netbt.ko` and its logic
transcription eliminates both demonstrated consequences.

## Mechanism (every hop cited)

### The dead guard

`l2cap_request_free` (`sys/netbt/l2cap_misc.c:163-174`):

```c
163  void
164  l2cap_request_free(struct l2cap_req *req)
165  {
166      struct hci_link *link = req->lr_link;
167
168      callout_stop(&req->lr_rtx);              /* (1) */
169      if (callout_active(&req->lr_rtx))        /* (2) DEAD GUARD */
170          return;
171
172      TAILQ_REMOVE(&link->hl_reqs, req, lr_next);
173      zfree(l2cap_req_pool, req);
174  }
```

`callout_stop` (`sys/kern/kern_timeout.c:1091-1094`) dispatches to
`_callout_cancel_or_stop(cc, CALLOUT_STOP, sync=1)` (`:857-930`). The very
first thing that function does is:

```c
869      atomic_clear_int(&cc->flags, CALLOUT_ACTIVE);
```

— unconditionally. So after `callout_stop` returns, `CALLOUT_ACTIVE` is
always clear, and `callout_active()` (`:1155-1158`) always returns 0. **The
guard at `l2cap_misc.c:169` is dead code**; `l2cap_request_free` always
falls through to the `TAILQ_REMOVE` + `zfree`.

### The callback owns the free

The RTX callout callback `l2cap_rtx` (`sys/netbt/l2cap_misc.c:183-197`)
calls `l2cap_request_free(req)` from **inside** the callout:

```c
183  void
184  l2cap_rtx(void *arg)
185  {
186      struct l2cap_req *req = arg;
187      struct l2cap_channel *chan;
188
189      chan = req->lr_chan;
190      l2cap_request_free(req);          /* callback frees req */
191
192      DPRINTF("cid %d, ident %d\n", (chan ? chan->lc_lcid : 0), req->lr_id);
193      /* NB: line 192 dereferences req->lr_id AFTER req was freed at 190
194       *     -> a latent use-after-free in debug (DPRINTF) builds. */
```

That inner `callout_stop` is recursive (`curthread == &c->qsc->thread`,
`kern_timeout.c:915-918`) and returns immediately, so the callback proceeds
to `TAILQ_REMOVE` + `zfree` — the callback ITSELF frees `req` while the
callout is still `CALLOUT_INPROG`.

### The SMP race

`_callout_cancel_or_stop` for a **non-recursive** caller blocks in `ssleep`
until the in-progress callback finishes (`kern_timeout.c:910-921`):

```c
910      ++c->waiters;
911      for (;;) {
912          cpu_ccfence();
913          if ((c->flags & flags) == 0)
914              break;
915          if ((c->flags & CALLOUT_INPROG) &&
916              curthread == &c->qsc->thread) {       /* recursive */
917              _callout_update_spinlocked(c);
918              break;
919          }
920          ssleep(c, &c->spin, 0, "costp", 0);      /* non-recursive: BLOCK */
921      }
```

Crucially, the `_callout` (`toc`) that holds `CALLOUT_INPROG`, the spin, and
the waiters is a **separate allocation** from the `struct callout` embedded
in `req` (`sys/sys/callout.h`) — so freeing `req` does not disturb the state
Thread B is sleeping on. This gives the race:

1. **Thread A** = softclock, dispatching the RTX callout (`CALLOUT_INPROG`).
   The callback calls `l2cap_request_free(req)` → `callout_stop` (recursive,
   returns) → dead guard falls through → `TAILQ_REMOVE` + `zfree(req)`.
2. **Thread B** = a concurrent external caller of `l2cap_request_free(req)`
   (e.g. a signal handler / link-teardown / channel-close sweep on another
   CPU). Thread B's `callout_stop` is non-recursive → blocks in `ssleep`
   until Thread A's callback finishes. By then Thread A has **already**
   `TAILQ_REMOVE`'d and `zfree`'d `req`.
3. Thread B wakes. `callout_active` (dead guard) is false. Thread B runs
   `TAILQ_REMOVE` on the already-unlinked `req` (stale `tqe_prev`/`tqe_next`
   → list corruption) and then `zfree` on the already-freed `req` →
   **double-free**.

### Generic-kernel consequence

`zfree` under `INVARIANTS` (`sys/vm/vm_zone.c:234-237`):

```c
234  #ifdef INVARIANTS
235      if (((void **)item)[1] == (void *)ZENTRY_FREE)
236          zerror(ZONE_ERROR_ALREADYFREE);   /* panic("zone: freeing free entry") */
```

So on the default `X86_64_GENERIC` kernel (INVARIANTS ON) the double-free
panics with `panic("zone: freeing free entry")`. On a noinv kernel the slab
freelist is silently corrupted and the next `zalloc` returns an overlapping
object. The TAILQ corruption is silent corruption in both cases (and on
GENERIC it is usually pre-empted by the earlier `ZONE_ERROR_ALREADYFREE`
panic).

## Harness proof

`harness.c` transcribes the two threads verbatim with:

- `struct l2cap_req` / `hci_link` layout from `sys/netbt/l2cap.h:423-430`;
- `struct callout` (flags + `toc`) separable from `struct _callout` (INPROG,
  spin, cv, thread) exactly as in `sys/sys/callout.h`, so freeing `req`
  leaves the `_callout` intact (Thread B sleeps through the free);
- `callout_stop` model with the exact `_callout_cancel_or_stop` control flow
  (clear ACTIVE; recursive → return; non-recursive → block);
- `TAILQ_*` macros transcribed from `sys/sys/queue.h:584-662` (production
  form — entries' `tqe_next`/`tqe_prev` are NOT cleared after removal);
- poisoned `vm_zone` `zalloc`/`zfree` (item[0]=freelist link,
  item[1]=`ZENTRY_FREE` under INVARIANTS, double-free detection);
- a slab-reuse step modelling another CPU reclaiming `req`'s slot between
  the two frees.

A deterministic 3-way barrier schedule forces the documented interleaving
(Thread B caches its victim `link` while `req` is live, then blocks in
`callout_stop` while the callback frees `req`).

**Results (deterministic, 3/3 runs):**

- **Scenario 1 (no slab reuse):** `DOUBLE-FREE CONFIRMED` — `zfree` called
  twice on `req`; the second sees `item[1] == ZENTRY_FREE`
  (`vm/vm_zone.c:235` → `ZONE_ERROR_ALREADYFREE` panic on GENERIC).
- **Scenario 2 (slab reuse between the frees):** `TAILQ CORRUPTION
  CONFIRMED` — Thread B's stale `TAILQ_REMOVE` dereferences the reused
  slot's `tqe_prev` (now pointing into a *different* link's `hl_reqs`) and
  unlinks a **live** request on `g_link2`; plus `FREE-OF-LIVE-OBJECT
  CONFIRMED` — the slab reuse turned the double-free into freeing the
  reuser's in-use request (use-after-free).

## Impact ceiling

- **Class:** double-free (CWE-415) + TAILQ/list corruption + use-after-free,
  from a race window reachable by an unprivileged local user with a
  `BTPROTO_L2CAP` socket once the netbt stack is active and there is
  Bluetooth hardware (SMP).
- **GENERIC (INVARIANTS ON):** `panic("zone: freeing free entry")` — kernel
  DoS.
- **noinv:** silent slab-freelist corruption → overlapping object on the
  next `zalloc` → a slab-groom / type-confusion primitive candidate (the
  `l2cap_req_pool` zone; victims would be other `l2cap_req`-sized objects).
  No `uid=0` chain was developed because the runtime path is unreachable on
  this guest (no bt radio); this is a **code-level harness confirmation**
  of a write-capable primitive that would be a slab-groom candidate on
  hardware.
- **Realistic trigger:** local unprivileged `BTPROTO_L2CAP` socket + SMP +
  RTX timeout concurrent `close()` / disconnect, on a host with a Bluetooth
  adapter (or a USB bt dongle). Not remotely reachable.

## Fix (fix.diff)

`git apply`-able unified diff against `sys/netbt/l2cap_misc.c`. Two
coordinated changes:

1. **`l2cap_request_free`:** replace `callout_stop` + dead `callout_active`
   guard with `callout_drain(&req->lr_rtx)`. `callout_drain`
   (`kern_timeout.c:1047-1050`) is `_callout_cancel_or_stop(CANCEL, sync=1)`
   — it blocks until a running callback has returned (recursing from inside
   the callback returns immediately), so an external caller can no longer
   race the callback's actions on `req`.

2. **`l2cap_rtx`:** remove the `l2cap_request_free(req)` call from the
   callback (the callback no longer owns the free). Capture `chan` and `id`
   first (also fixes the latent `DPRINTF` use-after-free of `req->lr_id`
   after the free), then let `l2cap_close(chan, ETIMEDOUT)` drive cleanup —
   `l2cap_close` (`sys/netbt/l2cap_lower.c:86-93`) sweeps `hl_reqs` and
   frees any request whose `lr_chan` matches, so `req` is freed exactly
   once from a single context.

This supersedes the finding markdown's proposal (which named the same two
moves — `callout_drain` and "caller owns free" — but did not spell out the
`l2cap_rtx` reorder or the `DPRINTF` UAF fix).

## Fix validation (Phase 8)

1. `git apply --check` / `patch -p1` on `/usr/src`: **applies cleanly**.
2. `cd /usr/src/sys/netbt && make -j4`: builds `netbt.ko` with the fix,
   **0 errors, 0 warnings**. `nm l2cap_misc.o` shows the module now
   references `callout_drain` and no longer references `callout_stop` or
   `callout_active` — the dead guard is gone.
3. **Fixed-logic harness** (`harness_fixed.c`) transcribes the same fix and
   reproduces neither consequence: Scenario 1 `zfree` count drops 2 → 1
   (no double-free); Scenario 2 `zfree` count ≤ 1 and `g_link2`'s live
   request stays linked (no TAILQ corruption, no free-of-live-object).
   Summary prints `FIXED (clean)` for both.

### Honest caveat on the fix's completeness

The finding's specific race — *"the callback calls `l2cap_request_free` from
inside the callout, racing an external caller"* — is **closed** by removing
the `l2cap_request_free(req)` call from `l2cap_rtx`. A deeper residual
exists: `l2cap_close` (invoked by the callback) itself sweeps `hl_reqs` and
calls `l2cap_request_free`, so a fully robust fix would additionally defer
`l2cap_close` out of the softclock callback context (e.g. via a taskqueue)
so that no `req` free ever runs in the callback. That is a larger ownership
refactor outside the scope of a one-file security patch; the submitted
`fix.diff` eliminates the demonstrated double-free/TAILQ-corruption race
and the latent `DPRINTF` UAF, and is the conventional correct pattern for
this bug class.

## PoC changes from the seeded scaffolding

There was no seeded PoC (`findings/poc/DF-0745/` did not exist). The harness
was authored from scratch by reading `sys/netbt/l2cap_misc.c`,
`sys/kern/kern_timeout.c`, `sys/netbt/l2cap.h`, `sys/sys/queue.h`, and
`sys/vm/vm_zone.c`. Key fidelity decisions: the `_callout` is modeled as a
separate allocation (so the free of `req` doesn't tear down the state Thread
B sleeps on); the `vm_zone` zfree touches only `item[0..1]` (so stale
`lr_next` survives the free, which is what makes Thread B's second
`TAILQ_REMOVE` deref a stale pointer); a slab-reuse step models the
realistic cross-list corruption.