# DF-0503 — ng_fec stored-pointer UAF on member ifnets

## Verdict: CODE-PATH CONFIRMED; runtime UAF trigger blocked by a separate constructor bug

## Mechanism (confirmed at code level)

`ng_fec_addport` in **sys/netgraph7/ng_fec.c:349-457** looks up a
member interface via `ifunit(iface)` at **line 370** with **no
`ifnet_lock` held**, then stores the returned `struct ifnet *` in
`new->fec_if` at **line 450**.  `ng_fec_delport` (line 460-518) repeats
the pattern at **line 481**.

**sys/net/if_var.h:894-896** explicitly mandates:

> ifunit() must only be called in non-netisr threads and ifnet lock
> must be held before calling this function and for the accessing of
> the ifp returned by this function.

The stored `fec_if` pointer is dereferenced indefinitely from three
runtime paths:

- **`ng_fec_tick`** (1 Hz callout, lines 687-691): `ifp = p->fec_if;
  (*ifp->if_ioctl)(ifp, SIOCGIFMEDIA, ...)` — derefs `ifp->if_ioctl`.
- **`ng_fec_input`** (netisr RX path, lines 885-888): TAILQ_FOREACH
  compares `p->fec_if == m0->m_pkthdr.rcvif`.
- **`ng_fec_start` / `ng_fec_choose_port`** (TX path, lines 1066-1095):
  TAILQ_FOREACH + `*ifp = p->fec_if` (writes the chosen port's ifp).

If a member interface is later detached/destroyed (`ifconfig destroy`,
hot-unplug), the kernel frees the `struct ifnet` and may reuse the
slab.  The next 1 Hz `ng_fec_tick` dereferences the dangling pointer,
producing a UAF read (and indirect function call through `if_ioctl`).

## Runtime trigger attempt

A root-driven attempt to set up a fec bundle via `ngctl mkpeer` on
the test guest instead hit a **separate latent bug** in
`ng_fec_constructor` — a `panic: trying to free NULL pointer` in
`_kfree()` called from `ng_fec_constructor+0x3ae`.  This blocks the
DF-0503 trigger path entirely: you can't construct a fec node to add
member interfaces to, because the constructor itself panics on the
malformed hook-name parsing.

```
panic: trying to free NULL pointer
cpuid = 0
_kfree() at _kfree+0x558
_kfree() at _kfree+0x558
ng_fec_constructor() at ng_fec_constructor+0x3ae
ng_mkpeer() at ng_mkpeer+0x4c
ng_generic_msg() at ng_generic_msg+0x415
ng_send_msg() at ng_send_msg+0xf1
```

(Saved in `panic.txt`.  This is a *different* bug worth a separate
finding; it does at least confirm that the ng_fec module is exercised
and the DF-0503 code paths are reachable in principle — just not
constructible on this kernel without first fixing the constructor
bug.)

## Reachability — root-only throughout

- `ng_fec` module load: `kldload` (root)
- bundle configuration: `ngctl mkpeer/msg` (root — netgraph control
  socket is `socket(AF_NETGRAPH)`, root-only)
- member interface detach: `ifconfig destroy` (root)
- UAF then fires automatically from `ng_fec_tick` (1 Hz callout)

So this is a **root-configured, root-triggered** UAF — a kernel
hardening gap, not an unpriv→root vector.  The realistic scenario:
an admin has set up a fec bundle of N member interfaces and later
detaches one (USB NIC hot-unplug, virtual NIC removal).  The bundle
continues to deref the freed ifnet.

The v1 twin in **sys/netgraph/fec/ng_fec.c** (the actually-loadable
`ng_fec.ko`) has the same defect at lines 345, 408, 433 (ifunit), 490,
525, 551, 581, 764, 974 (deref sites).

## Fix

`fix.diff`:

1. Wraps the `ifunit()` calls in `ifnet_lock()`/`ifnet_unlock()` in
   both `ng_fec_addport` (line 370) and `ng_fec_delport` (line 481),
   satisfying the `if_var.h` contract.
2. Adds a global `ifnet_detach_event` handler
   (`ng_fec_ifnet_detach_event`) that walks every existing fec bundle
   on interface departure and NULLs out any matching `p->fec_if`
   pointer.
3. Adds `if (p->fec_if == NULL) continue;` / skip guards in the
   deref sites (`ng_fec_tick` at :687 and `ng_fec_choose_port` at
   :1066) so they tolerate the now-possible NULL.
4. Registers/deregisters the event handler in `ng_fec_mod_event`.

The v1 module would need the same fix applied to
`sys/netgraph/fec/ng_fec.c`.

## Build / run

```
ssh dfbsd 'kldload ng_fec ng_socket if_tap'    # root setup
ssh dfbsd 'ifconfig tap0 create && ifconfig tap1 create && ifconfig tap0 up && ifconfig tap1 up'
ssh dfbsd 'ngctl mkpeer fec fec0 inet/inet/0'  # may hit the constructor bug
# Then configure the bundle, add tap0/tap1 as members, bring it up,
# and `ifconfig tap0 destroy` to trigger the UAF via ng_fec_tick.
```