# DF-0604 — VERDICT

## Verdict: REPRODUCED (panic) — fix VALIDATED

The cross-CPU race on the global `pfi_buffer` is **real, triggerable, and causes
a kernel panic** (general protection fault in `rn_walktree_at`). However, the
finding's **PoC trigger mechanism was incorrect**: concurrent `SIOCAIFADDR`
(ifconfig alias) operations cannot race because DragonFlyBSD dispatches ALL
`SIOCAIFADDR`/`SIOCDIFADDR` work to netisr0 (`sys/netinet/in.c:248`:
`lwkt_domsg(netisr_cpuport(0), ...)`), so all `ifaddr_event` firings serialize
on netisr0's single thread.

The **actual race window** is between the pfioctl path (DIOCADDRULE →
`pfi_dynaddr_setup` → `pfi_table_update`) which runs on the **caller's CPU**
(not dispatched to netisr0), and the ifaddr_event path which runs on **netisr0**.
These two paths share the global `pfi_buffer`/`pfi_buffer_cnt`/`pfi_buffer_max`
with **no lock**, and can overlap.

## Mechanism (confirmed, path:line at each hop)

1. **Globals** declared file-scope at `sys/net/pf/pf_if.c:74-76`:
   `pfi_buffer`, `pfi_buffer_cnt`, `pfi_buffer_max` — shared across all CPUs,
   no lock.

2. **pfioctl path (caller's CPU)**: `pfioctl()` acquires `pf_token`
   (`pf_ioctl.c:989`) but does **not** dispatch to netisr0. When loading a rule
   with dynamic interface expansion `(vtnet0)`, `pfi_dynaddr_setup()`
   (`pf_if.c:392`) calls `pfi_kif_update()` (`pf_if.c:448`) →
   `pfi_dynaddr_update()` (`pf_if.c:486`) → `pfi_table_update()` (`pf_if.c:499`).
   This runs on whatever CPU the calling process is scheduled on (CPU X).

3. **`pfi_table_update()` body** (`pf_if.c:506-528`):
   - Line 511: `pfi_buffer_cnt = 0` (store on CPU X)
   - Line 514: `pfi_instance_add(kif->pfik_ifp, net, flags)` → dispatches fill
     to **netisr0** via `netisr_domsg(&msg.base, 0)` (`pf_if.c:629`)
   - CPU X **blocks** waiting for netisr0 reply
   - Line 522: After reply, reads `pfi_buffer_cnt` and passes `pfi_buffer`
     to `pfr_set_addrs()` — on CPU X

4. **ifaddr_event path (netisr0)**: `SIOCAIFADDR` is dispatched to netisr0
   (`in.c:248`). Inside `in_control_internal` (`in.c:421`), after processing,
   `EVENTHANDLER_INVOKE(ifaddr_event)` fires (`in.c:691/726`) **on netisr0**.
   → `pfi_ifaddr_event()` (`pf_if.c:906`) → `pfi_kif_update()` (`pf_if.c:913`)
   → `pfi_table_update()` — runs on netisr0, also using the global buffer.

5. **The overlap**: CPU X is inside `pfi_table_update` (between `cnt=0` and
   readback, blocked on netisr_domsg). Its `pfi_instance_add` dispatch message
   is queued on netisr0. Before netisr0 processes that message, it processes
   a pending SIOCAIFADDR → fires ifaddr_event → enters ITS OWN
   `pfi_table_update` → sets `cnt=0`, fills buffer, reads `cnt`, calls
   `pfr_set_addrs`. Then netisr0 processes CPU X's dispatch message, fills the
   buffer **starting from the ifaddr_event's residual count**. CPU X reads a
   contaminated `pfi_buffer_cnt` — its `pfr_set_addrs()` sees addresses from
   BOTH invocations.

6. **Corruption → panic**: the contaminated `pfr_set_addrs()` call corrupts the
   pfr_table's radix tree. The subsequent table walk hits a bad pointer:
   `Fatal trap 9: general protection fault at rn_walktree_at+0xa8: movl 0x10(%r12),%eax`.

## Evidence

**Baseline (instrumented #1 kernel, no fix):**
```
DF0604_RACE: concurrent pfi_table_update depth=2 cpu=0   (×10)
pfi_table_update: cannot set 3 new addresses into table vtnet0: 3
Fatal trap 9: general protection fault while in kernel mode
cpuid = 0; lapic id = 0
current process = Idle
Stopped at rn_walktree_at+0xa8: movl 0x10(%r12),%eax
```
→ 10 race detections, data corruption, **kernel panic** (GP fault).

**Fixed (#2 kernel, dispatch-to-netisr0 + race detector):**
→ 0 race detections, 0 panics, 0 corruption errors across **3 runs** (30s + 30s + 45s).
Guest stays up, pf rules functional throughout.

## Why the finding's proposed fix (lwkt_token) is WRONG

The finding proposes `lwkt_gettoken(&pfi_buffer_token)` around the entire
`pfi_table_update` body. **This can deadlock.** The pfioctl path (CPU X)
acquires the token, then blocks on `netisr_domsg` waiting for netisr0 to fill
the buffer. If, before netisr0 processes that fill, it processes a pending
SIOCAIFADDR → fires ifaddr_event → calls `pfi_table_update` → tries
`lwkt_gettoken(&pfi_buffer_token)` — it blocks, because CPU X holds it.

DragonFlyBSD's lwkt_token is **not released across `lwkt_domsg` blocks**
(`sys/kern/lwkt_token.c:655-719`: token remains in `td_toks` until explicit
`lwkt_reltoken`). CPU X waits for netisr0 to reply; netisr0 waits for CPU X to
release the token. **Deadlock.** (Contrast: `pf_token` is also held across
netisr_domsg, but netisr0 never tries to acquire `pf_token` in the dispatch
path, so no collision.)

## The correct fix (in `fix.diff`)

**Dispatch the entire `pfi_table_update` to netisr0** when not already on
CPU 0. Since netisr0 is single-threaded, all callers (ifaddr_event already on
netisr0, pfioctl on caller CPU, group_change events, etc.) serialize
inherently. No token needed; no deadlock possible.

The fix:
- Extracts the existing body into `_pfi_table_update_body()`
- Adds `pfi_table_update_dispatch()` netmsg handler
- `pfi_table_update()` checks `mycpuid`: if 0, calls body directly; otherwise
  dispatches via `netisr_domsg`

This **supersedes** the finding's lwkt_token proposal.

## Additional finding (memcpy direction bug)

While tracing the code, I noticed `pf_if.c:647`:
```c
memcpy(pfi_buffer, p, pfi_buffer_cnt * sizeof(*pfi_buffer));
```
This copies FROM the newly-allocated buffer `p` (uninitialized) TO the old
buffer `pfi_buffer` — **reversed**. It should be `memcpy(p, pfi_buffer, ...)`.
After growth, all existing addresses in the buffer are garbage. OpenBSD's
equivalent uses `bcopy(pfi_buffer, p, ...)` (correct direction). This is a
separate latent bug that compounds the race, but is only triggered when the
buffer grows past 64 entries (>64 addresses on a single interface/group).

## PoC changes

1. **`race_churn.c`** — C harness for concurrent `SIOCAIFADDR`/`SIOCDIFADDR`
   churn with `lwp_setaffinity` CPU pinning. Corrected to use DragonFlyBSD's
   `lwp_setaffinity(2)` and `ifra_mask` (not `ifra_netmask`).

2. **`race_live.sh`** — The corrected live race trigger. The finding's original
   PoC (concurrent ifconfig alias on two interfaces) **cannot race** because
   SIOCAIFADDR dispatches to netisr0. The corrected trigger races the pfioctl
   path (pfctl -f churn) against the ifaddr_event path (ifconfig alias churn).

3. **`race_proof.c`** — Code-level proof: a userspace pthreads program that
   replicates the exact `pfi_table_update` pattern (caller does cnt=0 →
   dispatches fill to "netisr0" thread → reads cnt back) with shared globals
   and no lock. Demonstrates massive cross-contamination (10000+ contamination
   events in seconds).

4. **`fix.diff`** — The correct dispatch-to-netisr0 fix (NOT the finding's
   lwkt_token proposal which deadlocks).