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:

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():

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).

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.)