# DF-0525 — VERDICT

**Verdict:** NOT REPRODUCED at runtime (source-CONFIRMED real race; live trigger
pre-empted by a separate ng_fec constructor defect; root-only reachability).
**Impact:** none for an unprivileged user (root-only → at best self-inflicted DoS/panic).
**Confidence:** likely (race confirmed by rigorous line-by-line source trace; not
runtime-observed because the module cannot be instantiated on master DEV).

## Mechanism (source-confirmed)

1. `ng_fec_init()` (`sys/netgraph/fec/ng_fec.c:502`) arms a 1 Hz callout:
   `callout_reset(&priv->fec_timeout, hz, ng_fec_tick, priv);` (`:535`).
2. `ng_fec_tick()` (`:562`) iterates the port list **without the list lock**:
   - `:575-578` comment: "serializer for parent interface not held on entry,
     and cannot be held during the loop to avoid a deadlock." (This refers to
     the *ifnet serializer*, a different synchronization object from the
     *global ifnet_lock* that protects the port list.)
   - `:579` `TAILQ_FOREACH(p, &b->ng_fec_ports, fec_list)` — derefs `p`.
3. `ng_fec_delport()` (`:417`), reached from `NGM_FEC_DEL_IFACE`
   (`:1176-1178`), holds the global `ifnet_lock()` (`:430`) and:
   - `:464` `TAILQ_REMOVE(&b->ng_fec_ports, p, fec_list);`
   - `:465` `kfree(p, M_NETGRAPH);`  ← list node freed.
   - `:468` `ifnet_unlock();`
4. Because tick holds **no** lock on the list, a concurrent delport can free
   the node tick is traversing → `p->fec_if`/`p->fec_ifstat` (`:581`,`:594`,
   `:602`) dereference freed memory → **UAF**. Symmetric hazard with
   `ng_fec_addport()` (`:328`, `TAILQ_INSERT_TAIL` at `:409`).

The race is real and the lock asymmetry is explicit. This is not a false
positive.

## Why NOT reproduced at runtime

Two compounding facts:

**(A) Separate constructor defect blocks node creation.** On master DEV #0
(`with-src`), `kldload ng_fec.ko` succeeds, but creating any fec node panics
immediately:

```
ngctl mkpeer fec ether fec
-> panic: trying to free NULL pointer
   ng_fec_constructor() at ng_fec_constructor+0x3ae
```

`kern_slaballoc.c:1407` fires on `kfree(NULL)` reached from
`ng_fec_constructor()` (`ng_fec.c:1071-1152`). The constructor's error paths
also embed a latent double-free: at `:1093`/`:1101` they call
`kfree(ifp, M_NETGRAPH)`, but `ifp = &priv->arpcom.ac_if` (`:1085`) is an
**embedded** member of `priv`, not a separately allocated object. Because no
fec node can be created, no bundle/tick exists, so the DF-0525 race cannot be
exercised. (The constructor bug should be filed as a separate finding.)

**(B) Root-only reachability (no privilege boundary).** The netgraph control
socket requires root: `ngc_attach()` (`sys/netgraph/socket/ng_socket.c:172`)
does `caps_priv_check(ai->p_ucred, SYSCAP_RESTRICTEDROOT | __SYSCAP_NULLCRED)`.
Creating an fec node and issuing `NGM_FEC_ADD_IFACE`/`NGM_FEC_DEL_IFACE` is
therefore root-only. Per the audit threat model, a root→kernel race is
game-over-by-definition on the privilege axis; the only realistic impact is a
root-triggered panic (DoS). The freed object is a list node (`struct
ng_fec_portlist`), not a credential/function-pointer victim, so even on a
non-INVARIANTS kernel there is no straightforward escalation primitive —
content control over the freed/reclaimed node would require heap grooming
into a same-bucket victim, and there is no unprivileged path to drive it.

## Fix-validation (fix.diff)

`fix.diff` acquires `ifnet_lock()`/`ifnet_unlock()` around the
`TAILQ_FOREACH` in `ng_fec_tick()`, matching the lock that
`ng_fec_addport`/`ng_fec_delport` already hold when mutating the list. It is
git-apply-able (`git apply --check` clean) and **compiles**: a patched
`ng_fec.ko` module build completes with `rc=0` (see `build.log`).

- `fix_status`: **not_testable** (runtime). The specific tick/delport race
  cannot be exercised live because (a) the module deterministically panics on
  construction and (b) the path is root-only with no unprivileged driver. The
  fix was validated to **apply + compile**; it is the minimal correct change
  (it makes the iteration hold the exact lock the mutators hold).
- `fix_baseline_reproduced`: 0 (the race was not live-reproduced even on the
  unpatched baseline, for the reasons above).
- `fix_patched_reproduced`: 0.

## PoC changes

Created the evidence pack from scratch (the folder was empty on handoff):
`trigger_df0525.sh` (documented race setup), `build.sh`, `run.sh`,
`README.md`, this `VERDICT.md`, `panic.txt`, `env.txt`, `build.log`,
`fix.diff`, `manifest.json`.

## Recommended fix (summary)

In `sys/netgraph/fec/ng_fec.c`, wrap `ng_fec_tick()`'s `TAILQ_FOREACH`
(`:579`-`:612`) in `ifnet_lock()` / `ifnet_unlock()` so the port-list
iteration is serialized against `ng_fec_addport`/`ng_fec_delport`, which
already mutate that list under `ifnet_lock()`. (Supersedes the finding's
"acquire consistent lock" proposal by naming the concrete lock.)