DF-0663 — SLIST_REMOVE on never-inserted element in ng_device_newhook

Bug

sys/netgraph/ng_device.c:286-309 — ng_device_newhook() error branches at :290 (make_dev fail) and :300 (readq kmalloc fail) call SLIST_REMOVE(&sc->head, new_connection, ngd_connection, links) but the element was never inserted: SLIST_INSERT_HEAD(&sc->head, ...) happens only at :309, AFTER both error checks.

SLIST_REMOVE (sys/sys/queue.h:208-220) walks the list with no NULL guard, so on a non-member it dereferences NULL → panic.

IMPORTANT: dead code

The cited file sys/netgraph/ng_device.c is NOT in sys/conf/files and is built by no Makefile — it is dead code on all standard kernels. Only sys/netgraph7/ng_device.c (a complete rewrite without this bug) is built (optional netgraph7_device). The bug is real in the dead source, unreachable in any standard build.

Build / Run

cc -O2 -o df0663_slist_sim df0663_slist_sim.c -I/usr/src/sys
./df0663_slist_sim
# Expected: "Segmentation fault" — proving the SLIST_REMOVE macro
# NULL-derefs on a non-member with an empty list.

Expected

• Harness: Segmentation fault (core dumped) on the empty-list case.
• Live kernel: nothing — the cited file is not built. See VERDICT.md.

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.