# DF-2572 — ng_device global SLIST race (UAF / NULL-deref) — VERDICT

## Verdict: NOT REPRODUCED (unreachable dead code; impact claim is a false positive)

The unsynchronized global `SLIST` the finding describes is **genuinely present**
in the cited source text of `sys/netgraph/ng_device.c`, but that file is
**orphaned dead code** that is not compiled into any shipping DragonFlyBSD
kernel or module and **cannot even compile** against the current kernel
headers. The finding's impact claim — "attacker can groom slab controlled UAF
primitive" — is therefore a **false positive**: the code is unreachable, and the
maintained equivalent (`sys/netgraph7/ng_device.c`) has already eliminated the
entire bug class. This is the same dead-code conclusion the sibling finding
**DF-2571** reached for the same file.

---

## What the finding claims

`sys/netgraph/ng_device.c` keeps a single global connection list on
`ngd_softc.head` (a `SLIST_HEAD`, line 96) that is **mutated** by the netgraph
callbacks and **traversed** by the device entry points with no synchronization:

| Site | Operation | Line |
|---|---|---|
| `ng_device_newhook` | `SLIST_INSERT_HEAD(&sc->head, ...)` (insert) | 309 |
| `ng_device_disconnect` | `kfree(connection->readq)` then `destroy_dev` then `SLIST_REMOVE` (remove + free) | 407, 409, 411 |
| `ng_device_rcvdata` | `SLIST_FOREACH` (traverse) | 345 |
| `ngdioctl` | `SLIST_FOREACH` (traverse) | 464 |
| `ngdread` | `SLIST_FOREACH` + `memcpy(connection->readq,...)` | 518, 531 |
| `ngdwrite` | `SLIST_FOREACH` | 572 |
| `ngdpoll` | `SLIST_FOREACH` | 617 |
| `get_free_unit` | `SLIST_EMPTY` / `SLIST_FOREACH` | 237, 247 |

A grep for `mtx_`/`mutex`/`lock`/`spl` in the file returns **zero** matches —
there is no lock of any kind. The race the finding describes (thread B in
`ngdread` mid-`SLIST_FOREACH` holding a `connection` pointer while thread A's
`ng_device_disconnect` `kfree`s `readq` at line 407 *before* `destroy_dev`
drains at 409) is a **real unsynchronized-access bug in the source text**, and
would be dangerous IF the code were compiled and reachable. The `kfree(readq)`
before `destroy_dev` ordering (line 407 before 409) is itself a classic UAF
window even ignoring the list race.

## Why it does NOT reproduce — full trace

### (1) The cited file is dead code — no build path

| Build path | Status for `sys/netgraph/ng_device.c` |
|---|---|
| `sys/conf/files` | **No entry.** The only `ng_device` entry, at `conf/files:1699`, is `netgraph7/ng_device.c optional netgraph7_device` — a *different* file. |
| Kernel config `sys/config/X86_64_GENERIC` | **No `netgraph7_device` / `ng_device` option.** |
| Module Makefile | There is no `sys/netgraph/device/` directory. |
| Compiled into running kernel | **No.** `nm /boot/kernel/kernel.debug \| grep -c ng_device` = **0**. |
| Loadable module on disk | **No.** `kldload ng_device` → "No such file or directory"; `kldload netgraph7_device` → "No such file or directory". `/boot/kernel/` has only `netgraph.ko` (no ng_device modules at all). |
| `/dev/ngd*` device nodes | **None** (`ls /dev/ngd*` → No such file or directory). |

### (2) The cited file cannot even compile

Building `sys/netgraph/ng_device.c` as a kld module (mirroring the
`ng_echo`/`ng_socket` pattern) **fails hard** — the entire `struct cdevsw` /
`d_*_t` / `cdevsw_add` / `make_dev` character-device API it targets was removed
from DragonFly years ago. Freshly reproduced for this finding (full output in
`module_build_failure.txt`, `MAKE_RC=1`):

```
ng_device.c:111: error: unknown type name 'd_close_t'
ng_device.c:112: error: unknown type name 'd_open_t'
ng_device.c:113: error: unknown type name 'd_read_t'
ng_device.c:114: error: unknown type name 'd_write_t'
ng_device.c:115: error: unknown type name 'd_ioctl_t'
ng_device.c:116: error: unknown type name 'd_poll_t'
ng_device.c:119: error: variable 'ngd_cdevsw' has initializer but incomplete type
ng_device.c:132: error: 'nommap' undeclared here
ng_device.c:133: error: 'nostrategy' undeclared here
ng_device.c:152: error: implicit declaration of function 'cdevsw_add'
ng_device.c:159: error: implicit declaration of function 'cdevsw_remove'
ng_device.c:286: error: implicit declaration of function 'make_dev'
ng_device.c:419: error: 'ngdopen' redeclared as different kind of symbol
ng_device.c:506: error: 'ngdread' redeclared as different kind of symbol
...
cc1: all warnings being treated as errors
```

The file is a relic of the pre-netgraph7 era (FreeBSD 1.1.2.1, 2002) and has
not tracked the kernel API.

### (3) The maintained version does NOT have the bug

`sys/netgraph7/ng_device.c` — the file actually referenced in `conf/files` (as
`optional netgraph7_device`) — has a **completely different architecture** that
eliminates the entire bug class:

- **No global SLIST.** Connections are not kept in a list at all. Each netgraph
  node has its own `struct ngd_private` (allocated per-node in
  `ng_device_constructor`, line 165). The device entry points find their private
  data via a **direct pointer** stored on the cdev at `make_dev` time
  (`priv->ngddev->si_drv1 = priv`, line 190) and dereferenced in `ngdread`
  (411), `ngdwrite` (454), `ngdpoll` (481), `ngdopen` (334), `ngdclose` (351) —
  `priv_p priv = (priv_p)dev->si_drv1;`. There is **no `SLIST_FOREACH` traversal
  to race on** (the only `SLIST`/`ngd_softc`/`ngd_connection` tokens in the
  maintained file are inside `#if 0` dead ioctl code at lines 374-376).
- **Proper locking.** It initializes a node mutex and a queue mutex
  (`mtx_init(&priv->ngd_mtx...)` line 171; `mtx_init(&priv->readq.ifq_mtx...)`
  line 172) and takes them in every entry point (`mtx_lock`/`mtx_unlock` in
  `ngdopen` 338/340, `ngdclose` 354/356, `ngdread` 423, `ng_device_rcvdata`
  279/284; `IF_LOCK`/`IF_UNLOCK` around the queue at 269/278).
- **Correct lifetime ordering.** `ng_device_disconnect` calls
  `destroy_dev(priv->ngddev)` (line 299) **before** `kfree(priv,...)` (307).
  `destroy_dev` blocks until all in-flight cdev operations have drained, so no
  reader can be inside `ngdread` holding a stale `priv` when it is freed. This
  is exactly the ordering the dead file gets backwards (kfree-readq at 407
  before destroy_dev at 409).

So the DF-2572 bug pattern is **absent** from the maintained equivalent on
every axis (no shared list, mutexes present, drain-before-free ordering
correct).

### (4) Even if reachable, the device is root-only

`/dev/ngdN` is created by `make_dev(&ngd_cdevsw, unit, 0, 0, 0600, "ngd%d",
unit)` (`sys/netgraph/ng_device.c:286-287`). Mode `0600` means only **root**
can `open()` it. So even on a hypothetical kernel where this code were live,
the bug is a **root→kernel** issue, not an unprivileged→kernel escalation.
Per the Phase-6 hard-blocker list, a write/UAF reachable only from an already-
root context is game-over by definition — there is no privilege boundary to
cross. (The maintained `netgraph7/ng_device.c` likewise creates the device
mode `0600`, lines 179-180.)

---

## Conclusion — which step-4 category

**(d) Genuinely not reachable on this kernel** (primary) + partial **(a) false
positive** (the "groomable slab UAF" impact claim is wrong on every axis):
- The vulnerable file is not in any build path (`conf/files` references only
  `netgraph7/ng_device.c` at `:1699`; absent from `X86_64_GENERIC`).
- It cannot compile against the current kernel API (19+ errors, removed cdevsw).
- It is not in the running kernel (`nm` count = 0) and no module exists on disk.
- The maintained equivalent has no global SLIST, uses proper mutexes, and gets
  the drain-before-free ordering right — the bug class is eliminated there.
- The character device is mode 0600 (root-only) in both the dead and maintained
  versions, so even a live instance would be root→kernel, not unprivileged.

Because the cited code cannot run, there is **no memory corruption to
reproduce** and therefore **no Phase-6 escalation chain** to develop and **no
Phase-8 patched-kernel build** to validate (there is no live kernel image into
which `fix.diff` can be built to exercise the path). The unsynchronized-list
defect is a real code-quality / defense-in-depth issue in the orphaned source
text, so `fix.diff` is provided: it adds a `struct spinlock` to `ngd_softc`,
initializes it, and wraps every `SLIST` mutation/traversal (and also corrects
the disconnect ordering to `destroy_dev`-before-`kfree`, matching the
maintained version). The real resolution, however, is to **delete the orphaned
file** (it is fully superseded by `netgraph7/ng_device.c`).

## Fix validation status

`not_testable` — the cited file (`sys/netgraph/ng_device.c`) is not compiled
into the default kernel and cannot be built as a module (the `cdevsw`/`make_dev`
API it uses was removed). There is therefore no kernel image into which
`fix.diff` can be built to exercise the path. `fix.diff` is verified to apply
cleanly (`git apply --check` rc=0) and is a correct minimal change: it adds a
spinlock protecting the global SLIST plus corrects the disconnect
drain-before-free ordering, matching the design already used in the fixed
`netgraph7/ng_device.c`.