# DF-0663 — VERDICT

## Verdict: REPRODUCED at source level; cited file is DEAD CODE (not built)

The SLIST_REMOVE-on-never-inserted-element logic at
`sys/netgraph/ng_device.c:290` and `:300` is **real and would panic** the
kernel if executed — verified by source trace and by a userspace harness
that reproduces the exact macro expansion. **However**, the cited file
`sys/netgraph/ng_device.c` (the OLD netgraph v0 implementation) is **not
built by any standard kernel configuration or module**: it does not appear
in `sys/conf/files` or any module `Makefile`. Only the rewritten
`sys/netgraph7/ng_device.c` is built (`optional netgraph7_device`), and
that file has completely different code with no SLIST_REMOVE bug.

## Mechanism (cited line-by-line)

1. **`sys/netgraph/ng_device.c:273` `ng_device_newhook()`.** Allocates a
   fresh `new_connection` (not yet on any list).
2. **Line 286 `make_dev(...)` error branch.** If `make_dev` returns NULL
   the code jumps to cleanup at line 290:
   ```c
   288: if (new_connection->ngddev == NULL) {
   290:     SLIST_REMOVE(&sc->head, new_connection, ngd_connection, links);
   291:     kfree(new_connection, M_DEVBUF);
   ```
   `new_connection` was **never** inserted into `sc->head` (the
   `SLIST_INSERT_HEAD(&sc->head, new_connection, links)` happens only at
   line 309, after all error checks).
3. **Line 295 `kmalloc(... readq ...)` error branch** at line 300 has the
   same incorrect `SLIST_REMOVE` call.
4. **`sys/sys/queue.h:208-220` `SLIST_REMOVE` macro:**
   ```c
   if (SLIST_FIRST((head)) == (elm)) { SLIST_REMOVE_HEAD(...); }
   else {
       struct type *curelm = SLIST_FIRST((head));
       while (SLIST_NEXT(curelm, field) != (elm))
           curelm = SLIST_NEXT(curelm, field);     /* <-- no NULL check */
       SLIST_REMOVE_AFTER(curelm, field);
   }
   ```
   With `elm` not in the list, the `while` walks past the last element
   (where `SLIST_NEXT(curelm, field) == NULL`), assigns `curelm = NULL`,
   and re-evaluates `SLIST_NEXT(NULL, field)` → **NULL dereference**.

   If the list is empty (`SLIST_FIRST == NULL`), the `else` branch starts
   with `curelm = NULL` and the first `SLIST_NEXT(NULL, ...)` deref panics
   immediately.

5. **Harness verification.** `findings/poc/DF-0663/df0663_slist_sim.c`
   reproduces the exact SLIST_REMOVE logic in userspace with an empty
   list and an uninserted element. Run:
   ```
   [ng_device newhook SLIST_REMOVE on never-inserted conn] Case 1: empty list
     SLIST_FIRST(&sc_head_empty) = 0x0
     new_conn = 0x8004902c0
     invoking SLIST_REMOVE(&sc_head_empty, new_conn, ...)
   Segmentation fault (core dumped)
   EXIT=139
   ```
   The segfault proves the macro dereferences NULL on this path.

## Why the live kernel cannot trigger this

```
$ grep "ng_device" /home/maxx/dfbsd/dfbsd/sys/conf/files
netgraph7/ng_device.c    optional netgraph7_device
```

Only `netgraph7/ng_device.c` is registered in `sys/conf/files`. The OLD
`netgraph/ng_device.c` (which the finding cites) is **not registered** —
it is not built into the kernel and not built as a module by any
Makefile. The active netgraph7 implementation
(`sys/netgraph7/ng_device.c:264-312` `ng_device_newhook`) is a complete
rewrite that uses `priv_p` and `IF_` queues, has no SLIST, and no
equivalent bug. The OLD file is dead code retained for historical
reference.

Verified on the running guest: `ls /boot/kernel/ | grep ng_device`
returns nothing; `nm /boot/kernel/kernel | grep ng_device_newhook`
returns nothing.

## Privilege / threat model

- The vulnerable file is **not built on any standard kernel**. To make
  it live, an admin would have to manually add it to `conf/files` (e.g.
  `netgraph/ng_device.c optional netgraph_device`) and rebuild — no
  DragonFly release has shipped it in decades (netgraph7 superseded it).
- **Impact if it WERE built:** kernel panic from a privilege that can
  create netgraph hooks (root, via `AF_NETGRAPH` socket; or kernel-only
  paths). The trigger requires `make_dev()` or `M_NOWAIT` `kmalloc()` to
  fail — i.e. memory pressure / device-number exhaustion. With
  INVARIANTS ON, KASSERT failures in the slab allocator may catch some
  of these first.
- **Bottom line:** latent dead-code bug, not exploitable on stock
  DragonFly. Recommend deleting the dead file or porting the netgraph7
  implementation in place.

## Recommended fix

The finding's correct code-shape fix is to **remove the bogus
SLIST_REMOVE calls** at `:290` and `:300` (since `new_connection` was
never inserted). However, since the file is dead code, the better
recommendation is to **delete `sys/netgraph/ng_device.c`** (and its
header) to prevent confusion. `findings/poc/DF-0663/fix.diff` carries
both options: a minimal correctness fix (remove SLIST_REMOVE), and a
note recommending deletion of the dead file.