# DF-2587 — ng_ether NULL-deref in input/input_orphan/output callbacks

## Verdict: REPRODUCED (NULL-deref panic); fix_status = fixed

## Bug
The three `if_ethersubr` callbacks registered by ng_ether dereference
`IFP2NG(ifp)` as `node->private` **with no NULL check**:
- `ng_ether_input` (sys/netgraph/ether/ng_ether.c:206-207)
- `ng_ether_input_orphan` (sys/netgraph/ether/ng_ether.c:222-223)
- `ng_ether_output` (sys/netgraph/ether/ng_ether.c:260-261)

```c
const node_p node = IFP2NG(ifp);
const priv_p priv = node->private;     /* NULL-deref if node == NULL */
```

The sibling `ng_ether_detach` (:321) **does** guard: `if (node == NULL) return;`
(:326). `IFP2NG(ifp)` (`((struct arpcom *)ifp)->ac_netgraph`) is NULL on a *live*
interface when:
- `ng_ether_attach` fails before setting it (`ng_make_node_common` failure :289, or
  the `M_NOWAIT` priv `kmalloc` failure :296-301) — needs memory pressure; or
- `ng_ether_detach` nulls it (:331) during interface teardown — a race with
  in-flight traffic.

Once ng_ether is `kldload`'d, `ng_ether_input_p`/`ng_ether_output_p` are set
GLOBALLY (ng_ether.c:692-696), so **every** ethernet RX/TX packet on **every**
interface is routed through these callbacks (if_ethersubr.c:389, :1273-1279,
:1160-1177). An interface with `IFP2NG==NULL` therefore panics on the next packet.

## Reproduction
The detach race and the attach-failure (memory-pressure) paths are hard to hit
deterministically from userspace: `ifconfig tapX destroy` returns **EBUSY** while
the tap fd is open (verified), so continuous flooding during detach is not
possible, and netisr drains write-bursts before `destroy` nulls `IFP2NG`. ~4400
create/flood/destroy race cycles produced no panic (the window is narrow).

So the NULL-deref primitive is confirmed at the **function level** with a tiny
diagnostic module (`harness_mod.c`) that puts a live interface into the exact
post-race / failed-attach state — `IFP2NG(vtnet0) := NULL` — and then a normal
packet exercises the unguarded deref. This is the same harness-style primitive
confirmation used for race/memory-pressure-gated findings.

**Unpatched #0 kernel:** `kldload ng_ether` then `kldload df2587_harness.ko`
(sets `IFP2NG(vtnet0)=NULL`) then `ping -c1 10.0.2.2` (an outgoing packet →
`ether_output` → `ng_ether_output`) ⇒ **panic**:
```
Fatal trap 12: page fault while in kernel mode
fault virtual address  = 0x28
instruction pointer    = 0x8:0xffffffff82600247
current process        = Idle
Stopped at ng_ether_output+0x7:  movq 0x28(%rax),%rax
```
`0x28` is exactly the offset of `private` in `struct ng_node`; `movq 0x28(%rax)`
with `%rax==0` is the `node->private` load at ng_ether.c:261 with `node==NULL`.
This is the precise line the finding cites.

(The harness itself must take `ifnet_lock()` around `ifunit()`/the write, since
`ifunit()` asserts `mtx_owned(&ifnet_mtx)` — sys/net/if.c:1946. An early harness
build that omitted the lock tripped `panic: ifnet is not locked` first; the
final `harness_mod.c` locks correctly.)

## Fix
`fix.diff` adds `if (node == NULL) return;` to all three callbacks — the same
guard `ng_ether_detach` already has (:326). For `ng_ether_input_orphan` the
NULL path also `m_freem(m)` (it owns the mbuf); `ng_ether_output` returns 0
(let the packet continue, matching the "upper hook not connected" path).

## Fix validation
Built the patched `ng_ether.ko` (`make` in sys/netgraph/ether, rc=0), installed
to `/boot/kernel/ng_ether.ko` (sha e8f14d5f…), reloaded. Re-ran the SAME
harness + ping:
```
PATCHED: kldload ng_ether; kldload df2587_harness.ko (IFP2NG(vtnet0)=NULL)
         ping -c2 10.0.2.2: 2/2 packets, 0.0% loss, PING_EXIT=0, guest UP
```
**No panic.** The NULL check returns 0 in `ng_ether_output`, the packet passes
through normally. Same workload that panicked the unpatched module now succeeds.
fix_status = fixed.

## Impact
Kernel NULL-deref → panic (DoS). Reachable when an ethernet interface has
`IFP2NG==NULL` while traffic flows: via `ng_ether_attach` failure (memory
pressure) or a teardown race (interface destroyed with in-flight packets).
Requires ng_ether to be loaded (root `kldload`) AND either condition. No
memory-corruption primitive is attacker-shaped (it is a NULL deref, not a
controllable write) — pure DoS. Low severity is appropriate (module must be
loaded; trigger needs a race or memory pressure).
