# DF-0855 — dirfs_findfd KKASSERT panic / NULL deref on unlinked dirfs nodes

## Verdict
**REPRODUCED** (deterministic code-level harness) — the KKASSERT panic in
`dirfs_findfd` on an unlinked dirfs node (dn_parent == NULL) is **confirmed
by faithful line-by-line source transcription**. The `fix.diff` (replace
`KKASSERT(dnp1 != NULL)` with `if (dnp1 == NULL) break;` + NULL-return guard
in `dirfs_getattr`) is **compile-validated** (patched ≡ unpatched; with the
pre-existing missing-include addressed, patched compiles cleanly to 216 KB
`.o`). A live-kernel boot test is `not_testable` because dirfs is
**vkernel64-only** (not compiled into the running X86_64_GENERIC host kernel
and no vkernel runs on this guest).

## The bug — line-by-line (`sys/vfs/dirfs/dirfs_subr.c`)

```c
450: dirfs_node_t
451: dirfs_findfd(dirfs_mount_t dmp, dirfs_node_t cur,
452:           char **pathto, char **pathfreep)
453: {
454:     dirfs_node_t dnp1;
...
469:     dnp1 = cur;
470:     while (dnp1 == cur || dnp1->dn_fd == DIRFS_NOFD) {
471:         count += dnp1->dn_namelen;
...
478:             buf[MAXPATHLEN - count] = '/';
479:         dnp1 = dnp1->dn_parent;
480:         KKASSERT(dnp1 != NULL);            /* <-- PANIC if unlinked */
481:     }
```

The loop walks up the `dn_parent` chain looking for an ancestor with a valid
fd. At **:479** it dereferences `dnp1->dn_parent`; at **:480** it
`KKASSERT(dnp1 != NULL)`. When the node has been unlinked (`dn_parent == NULL`),
the KKASSERT fires → **kernel panic** (with INVARIANTS on, which VKERNEL64
ships by default per `options INVARIANTS`).

### The author already knew this case is reachable

The sibling function `dirfs_node_absolute_path_plus` (dirfs_subr.c:412-425)
does the **exact same** parent walk, but handles the NULL case correctly:

```c
412:     dnp1 = cur;
413:     while (dirfs_node_isroot(dnp1) == 0) {
...
422:         dnp1 = dnp1->dn_parent;
423:         if (dnp1 == NULL)       /* <-- correct: break + NULL return */
424:             break;
425:     }
```

And at **:433**: `if (dnp1 && count <= MAXPATHLEN)` — the NULL check is
present. `dirfs_findfd` was simply not given the same guard. This is a
clear omission, not a design choice.

### How dn_parent becomes NULL (the unlink path)

`dirfs_nremove` (dirfs_vnops.c:906-922):
```c
907:     pathnp = dirfs_findfd(dmp, dnp, &tmp, &pathfree);   /* BEFORE unlink */
909:     error = unlinkat(pathnp->dn_fd, tmp, 0);
910:     if (error == 0) {
914:         if (dnp->dn_parent) {
915:             dirfs_node_drop(dmp, dnp->dn_parent);
916:             dnp->dn_parent = NULL;                        /* UNLINKED */
917:         }
```

After unlink, the vnode stays alive (open fd held by the process). The finding
notes `dirfs_nremove` does **not** set `dn_links=0`, so `dirfs_inactive` does
not `vrecycle` the vnode — it persists for the fd's lifetime.

### The trigger chain (fstat on unlinked fd)

```
fstat(fd)
  → VOP_GETATTR
  → dirfs_getattr (dirfs_vnops.c:388-394):
      388: if (!dirfs_node_isroot(dnp)) {
      389:     pathnp = dirfs_findfd(dmp, dnp, &tmp, &pathfree);
      391:     KKASSERT(pathnp->dn_fd != DIRFS_NOFD);   /* NULL deref too */
  → dirfs_findfd walks dn_parent → NULL → KKASSERT at :480 → PANIC
```

Even if `dirfs_findfd` returned NULL gracefully, line 391's
`KKASSERT(pathnp->dn_fd != DIRFS_NOFD)` would NULL-deref `pathnp` → panic.

## Reachability on this guest (why a harness)

dirfs is **vkernel64-only**. Confirmed:
- `grep -c dirfs /usr/src/sys/conf/files` ⇒ **0** (not in host-kernel file list)
- `grep -c dirfs /usr/src/sys/platform/vkernel64/conf/files` ⇒ **3** (`optional dirfs`)
- `/boot/kernel/dirfs*` ⇒ does not exist; `kldstat | grep dirfs` ⇒ none
- Running kernel: `6.5-DEVELOPMENT #0` (X86_64_GENERIC) — no `options DIRFS`

dirfs is a pass-through filesystem that runs **inside a vkernel** (a userspace
process simulating a kernel). There is no vkernel running on this guest.
Per the dead-code/latent-bug clause (DF-0594/0616/0281 precedent, and the
sibling findings DF-0806/0807/0808), the **deterministic harness** is the
accepted proof: it transcribes the exact buggy loop (:469-481) and the
unlink setup (:914-916), proving the KKASSERT fires on the first iteration
when `cur->dn_parent == NULL`.

## Reproduction — harness (`harness.c`)

`./harness` builds a 2-level dirfs tree (root with valid fd, child with
DIRFS_NOFD), then:

1. **Happy path** (parent intact): both BUGGY and FIXED `dirfs_findfd`
   succeed, returning the root node with the relative path.
2. **Unlinked path** (simulating `dirfs_nremove` setting `dn_parent = NULL`):
   - BUGGY: `KKASSERT(dnp1 != NULL)` fires → **PANIC** (simulated as
     `g_panic_fired=1` + return NULL).
   - FIXED: `if (dnp1 == NULL) break;` exits the loop gracefully, returns
     NULL → caller returns ESTALE. No panic, no leak.

```
=== DF-0855 dirfs_findfd KKASSERT panic / NULL deref ===

[happy path, parent intact]
  BUGGY: pathnp=0x8004904c0 panic=0 path="file.txt"
  FIXED: pathnp=0x8004904c0 panic=0 path="file.txt"

[AFTER dirfs_nremove: file->dn_parent = NULL (unlinked, vnode alive)]
[fstat(fd) -> VOP_GETATTR -> dirfs_getattr -> dirfs_findfd]
  BUGGY: pathnp=0x0 panic=1
         ** PANIC: assertion "dnp1 != NULL" failed in dirfs_findfd at dirfs_subr.c:480
  FIXED: pathnp=0x0 tmp=NULL pf=0x0
         => no panic, no leak; caller (dirfs_getattr) returns ESTALE

=== SUMMARY ===
DF_0855_BUG_PANIC_ON_UNLINKED_NODE   = YES (KKASSERT fires)
DF_0855_FIX_RETURNS_NULL_NO_PANIC    = YES
```

Deterministic across 3 consecutive runs (run.log, run.stress.log).

## Impact ceiling

- **Claimed (finding)**: KKASSERT panic / NULL deref → DoS (Medium, CWE-476/617).
  **CONFIRMED** at the code/harness level.
- **Impact**: any unprivileged vkernel user who can open+unlink a file then
  fstat the fd triggers an **unrecoverable kernel panic** (DoS). This is a
  **vkernel** panic (the vkernel process crashes), not a host-kernel panic —
  dirfs code runs in the vkernel userspace process.
- On this guest (host kernel, no dirfs), the primitive is not live-reachable.
  The harness is the proof at the code/transcription level.
- **No escalation path**: this is a NULL-deref/KKASSERT panic (DoS), not a
  memory-corruption primitive. No slab grooming, no control-flow hijack, no
  uid=0 chain. The realistic ceiling is **panic / DoS**.

## Exploit chain

**none** (not applicable). This is a NULL-deref / KKASSERT panic — a DoS,
not a memory-corruption primitive. There is no write primitive, no UAF, no
type confusion. The harness proves the panic deterministically; there is no
escalation path to develop.

## The fix — `fix.diff`

Two changes:

### 1. `dirfs_findfd` (dirfs_subr.c:479-480) — root cause

```diff
 		dnp1 = dnp1->dn_parent;
-		KKASSERT(dnp1 != NULL);
+		if (dnp1 == NULL)
+			break;
```

Mirror the correct pattern from `dirfs_node_absolute_path_plus` (:422-424).
When `dn_parent` is NULL (unlinked node), break out of the loop; the
existing `if (dnp1 && count <= MAXPATHLEN)` check at :483 handles the NULL
return gracefully.

### 2. `dirfs_getattr` (dirfs_vnops.c:388-397) — caller NULL guard

```diff
 	if (!dirfs_node_isroot(dnp)) {
 		pathnp = dirfs_findfd(dmp, dnp, &tmp, &pathfree);

-		KKASSERT(pathnp->dn_fd != DIRFS_NOFD);
+		if (pathnp == NULL) {
+			/* Node has been unlinked (dn_parent == NULL). */
+			error = ESTALE;
+		} else {
+			KKASSERT(pathnp->dn_fd != DIRFS_NOFD);

-		error = dirfs_node_stat(pathnp->dn_fd, tmp, dnp);
-		dirfs_dropfd(dmp, pathnp, pathfree);
+			error = dirfs_node_stat(pathnp->dn_fd, tmp, dnp);
+			dirfs_dropfd(dmp, pathnp, pathfree);
+		}
 	} else {
```

Even with the findfd fix, `dirfs_getattr` would NULL-deref `pathnp` at the
`KKASSERT(pathnp->dn_fd != DIRFS_NOFD)` line. Return ESTALE (the standard
POSIX code for "stale file handle" — the file was unlinked, the fd is
still valid but the path-based metadata refresh can no longer resolve).

### Other callers (noted, not fixed — separate paths)

The following callers also deref `dirfs_findfd`'s return without a NULL
check, and would benefit from the same guard:
- `dirfs_readlink` (vnops.c:1326-1329) — `pathnp->dn_fd` used in readlinkat
- `dirfs_nmkdir` (vnops.c:1054-1055) — `dnp1->dn_fd` used as pfd
- `dirfs_alloc_file` (subr.c:191-194) — `pathnp->dn_fd` used in openat
- `dirfs_open_helper` (subr.c:596-597) — `pathnp->dn_fd` used as parentfd

These are separate trigger paths (not the fstat→getattr chain cited in this
finding) and are left for follow-up hardening.

## Fix validation (Phase 8)

- **`patch -p1 --dry-run`**: RC=0, both hunks succeed (dirfs_subr.c:477,
  dirfs_vnops.c:388).
- **Patched source reads correctly**: verified `if (dnp1 == NULL) break;`
  at subr.c:479-480 and the if/else ESTALE guard at vnops.c:389-397.
- **Compile-neutral proven**: compiled PATCHED and UNPATCHED `dirfs_subr.c`
  in the VKERNEL64_DIRFS build environment. Both fail with the **identical**
  error set — all pre-existing `M_WAITOK`/`M_ZERO`/`kmalloc`/`kfree`
  undeclared at lines 62, 63, 80, 392, 466 (missing `#include <sys/malloc.h>`
  — a pre-existing source bug unrelated to this fix). **Zero** errors at the
  fix lines 479-480. Patched ≡ unpatched.
- **Clean compile with include fix**: adding `#include <sys/malloc.h>` (the
  pre-existing missing include) to the patched source → compiles cleanly to
  a **216 KB `.o`** with zero errors. This proves the fix itself is valid C
  that the compiler accepts.
- **Live boot test: `not_testable`** — dirfs is not in the host kernel and
  no vkernel runs on this guest, so the `fstat`→`dirfs_getattr`→`dirfs_findfd`
  path cannot be exercised on a live kernel here. The harness transcription
  is the deterministic proof that the fix closes the panic.

## PoC changes

- **`harness.c`** — written from scratch (no pre-existing PoC). Faithful
  transcription of `dirfs_findfd` (dirfs_subr.c:450-497) with both BUGGY
  (KKASSERT) and FIXED (break+NULL) variants, plus the unlink setup from
  `dirfs_nremove` (vnops.c:914-916).
- **`build.sh` / `run.sh`** — standard build/run wrappers.
- **`fix.diff`** — authored post-verification: break+NULL in findfd +
  ESTALE guard in getattr.

## How to reproduce

```
ssh dfbsd-maxx   # unprivileged (uid 1001)
cd poc/DF-0855
./build.sh && ./run.sh
# expected: "BUG_PANIC_ON_UNLINKED_NODE = YES" and
#           "FIX_RETURNS_NULL_NO_PANIC = YES", exit 0
```