# DF-0525 — ng_fec_tick UAF race vs. NGM_FEC_DEL_IFACE

**File:** `sys/netgraph/fec/ng_fec.c`
**Severity (filed):** High
**Class:** Use-after-free (list-node), race condition
**Module:** `ng_fec.ko` (old netgraph v1) — **shipped** in `/boot/kernel/`, loadable.

## The claim (source-level, CONFIRMED)

`ng_fec_tick()` (`ng_fec.c:562`) runs from a 1 Hz callout (`callout_reset` at
`:535`/`:616`) and walks the bundle's port list:

```c
TAILQ_FOREACH(p, &b->ng_fec_ports, fec_list) {   /* ng_fec.c:579 */
    ifp = p->fec_if;                              /* :581 deref of list node */
    ...
}
```

The comment at `:575-578` explicitly admits **no lock is held**:

> Note: serializer for parent interface not held on entry, and cannot be held
> during the loop to avoid a deadlock.

Meanwhile `ng_fec_delport()` (`:417`), invoked from the `NGM_FEC_DEL_IFACE`
control message (`:1176-1178`), mutates the **same list** under the global
`ifnet_lock()`:

```c
ifnet_lock();                              /* :430 */
...
TAILQ_REMOVE(&b->ng_fec_ports, p, fec_list); /* :464 */
kfree(p, M_NETGRAPH);                       /* :465  <-- node freed   */
b->fec_ifcnt--;
ifnet_unlock();                             /* :468 */
```

Because `ng_fec_tick` does **not** take `ifnet_lock()`, a concurrent delport
can `TAILQ_REMOVE` + `kfree(p)` the very node the tick is about to (or is)
dereferencing via `TAILQ_FOREACH`. The tick then dereferences freed memory
(`p->fec_if`, `p->fec_ifstat`) → **UAF**. `ng_fec_addport()` (`:328`, inserts
at `:409` under `ifnet_lock` from the same call site) has the symmetric issue.

## Reproduction status

- **Source-level:** CONFIRMED. The locking asymmetry is real and explicit in
  the comment; the race window is the whole tick loop body.
- **Live trigger:** NOT reproduced. The shipped `ng_fec.ko` deterministically
  **panics during node construction** (`kfree(NULL)` reached from
  `ng_fec_constructor`, `panic: trying to free NULL pointer`) — a *separate*
  ng_fec defect — so no fec bundle / callout tick can ever be created on
  master DEV #0 to exercise the race. See `panic.txt`.
- **Privilege boundary:** **root-only.** The netgraph control socket
  (`ngc_attach`, `sys/netgraph/socket/ng_socket.c:172`) requires
  `caps_priv_check(.., SYSCAP_RESTRICTEDROOT)`, so creating an fec node and
  issuing `NGM_FEC_ADD_IFACE`/`NGM_FEC_DEL_IFACE` requires root. There is no
  unprivileged path to the race.

## Impact (realistic)

Root→kernel UAF race. Best-case observable effect is a **self-inflicted kernel
panic / DoS** by a root context that manages fec bundles and races add/del
port against the 1 Hz monitor tick. It is **not** an unprivileged→root
escalation (root-only reachability; and the freed object is a list node, not
a controllable credential/function-pointer victim). Treated as a
reliability/hardening bug in shipped netgraph code.

## Build / run

```
./build.sh   # apply fix.diff, build ng_fec.ko (validates fix compiles)
./run.sh     # attempt the race as root (panics at constructor on master DEV)
```
Full untrimmed logs: `build.log`, `panic.txt`, `env.txt`.

## Fix

`fix.diff` — acquire the global `ifnet_lock()` across `ng_fec_tick()`'s
`TAILQ_FOREACH` (the same lock `ng_fec_addport`/`ng_fec_delport` already hold
when mutating the list). This is minimal and matches the existing mutation-side
locking. `ifp->if_ioctl(SIOCGIFMEDIA)` under `ifnet_lock` is safe — delport
already calls `if_ioctl` under `ifnet_lock` (`:455`).