# DF-0808 — dirfs_nrename NULL deref panic on over-length or unlinked rename target

## Verdict
**NOT REPRODUCED** — the finding's claimed impact (NULL deref panic) is a
**FALSE POSITIVE for the security consequence**. The underlying code defect
(missing NULL check on `dirfs_node_absolute_path_plus` return) IS real, but
`rename(NULL, ...)` returns **EFAULT (errno 14)**, NOT a segfault/panic.
The kernel's `copyinstr(NULL)` catches the bad address and returns EFAULT
cleanly — proven by **live call + ktrace** on the guest.

## The code defect — REAL but mischaracterized

The cited path is accurate. `dirfs_nrename` (sys/vfs/dirfs/dirfs_vnops.c:977-981):

```c
977: 	tpath = dirfs_node_absolute_path_plus(dmp, tdnp,
978: 					      tncp->nc_name, &tpathfree);
979: 	fpath = dirfs_node_absolute_path_plus(dmp, fdnp,
980: 					      fncp->nc_name, &fpathfree);
981: 	error = rename(fpath, tpath);          // NO NULL CHECK
```

`dirfs_node_absolute_path_plus` (dirfs_subr.c:377-443) returns NULL when:
- `cur == NULL` (line 390)
- assembled path > MAXPATHLEN (line 433 condition fails: `dnp1 && count <= MAXPATHLEN`)
- parent chain broken / unlinked (dnp1==NULL after loop break at line 423)

All three NULL-return paths are confirmed by faithful code transcription in
the harness. The missing NULL check between the path construction and the
`rename()` call IS a real code defect.

## Why the claimed panic does NOT occur

The finding claims: *"rename(NULL,...) derefs NULL in kernel = panic."*

This is **incorrect**. In dirfs, `rename` is the standard **libc rename()**
(dirfs_vnops.c includes `<unistd.h>` at line 39; there is no `#define rename`
anywhere in `sys/`). libc's `rename()` is a thin syscall stub that passes the
raw pointers to the kernel. The kernel's `kern_rename` → `nlookup` →
`copyinstr(NULL)` catches the NULL address via the page-fault handler and
returns **EFAULT**, not a panic.

**Proof** — live on the guest (same libc + kernel as dirfs would use):

```
$ ktrace rename(NULL, "/tmp/x")
912:1  CALL  rename(0,0x400b82)
912:1  RET   rename -1 errno 14 Bad address

$ rename(NULL, "/tmp/x")    → r=-1, errno=14 (Bad address)
$ rename("/tmp/f", NULL)    → r=-1, errno=14 (Bad address)
$ rename(NULL, NULL)        → r=-1, errno=14 (Bad address)
ALL DONE — process did not crash, exit 0
```

All three variants return EFAULT. The process does **not** crash. This is
deterministic across 3 consecutive runs (run.log, run.2.log, run.3.log —
identical output).

## Why a live kernel trigger is not possible

dirfs is **vkernel64-only**:
- `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
- dirfs_vnops.c includes `<stdio.h>`, `<unistd.h>` (userspace headers) — it
  literally cannot compile into the real host kernel

The running kernel is `6.5-DEVELOPMENT #0` (X86_64_GENERIC) which does not
include `options DIRFS`. There is no vkernel running on this guest. So the
`rename(2)` → `dirfs_nrename` → `dirfs_node_absolute_path_plus` path cannot
be exercised on a live kernel here.

However, the **actual `rename(NULL,...)` behavior** is testable on the guest
because dirfs uses the same libc `rename()`. The harness tests this live,
plus transcribes the `dirfs_node_absolute_path_plus` function faithfully.

## Impact assessment

- **Claimed (finding)**: NULL deref panic / DoS (Medium, CWE-476) — **FALSE**
- **Actual**: `rename()` returns EFAULT (errno 14) instead of ENAMETOOLONG
  when the assembled absolute path exceeds MAXPATHLEN or the parent is
  unlinked. This is a **POSIX correctness bug** (wrong error code), not a
  security-relevant panic or DoS.
- The error propagates cleanly: `dirfs_nrename` returns EFAULT → VFS returns
  EFAULT → user's `rename(2)` returns EFAULT. No crash, no memory corruption,
  no information leak.
- **Impact severity**: Info / Low (wrong error code, no security consequence).

## Exploit chain

**none** (not applicable). This is not a memory-corruption primitive — it is a
NULL pointer passed to a syscall that returns EFAULT. There is no corruption,
no control flow hijack, and no escalation path. The NULL deref panic claimed
by the finding does not occur.

## The fix — `fix.diff`

Despite the false-positive impact, the missing NULL check IS a code defect
that should be fixed for correctness. The fix adds a NULL check after the
two `dirfs_node_absolute_path_plus` calls, returning `ENAMETOOLONG` instead
of silently getting `EFAULT`:

```diff
 	tpath = dirfs_node_absolute_path_plus(dmp, tdnp,
 					      tncp->nc_name, &tpathfree);
 	fpath = dirfs_node_absolute_path_plus(dmp, fdnp,
 					      fncp->nc_name, &fpathfree);
-	error = rename(fpath, tpath);
-	if (error < 0)
-		error = errno;
+	if (fpath == NULL || tpath == NULL) {
+		error = ENAMETOOLONG;
+	} else {
+		error = rename(fpath, tpath);
+		if (error < 0)
+			error = errno;
+	}
```

The harness validates: with the fix, the over-length-path scenario returns
`ENAMETOOLONG (63)` instead of `EFAULT (14)`. The cleanup path
(`dirfs_dropfd(dmp, NULL, pathfree)` at lines 1017-1018) is already safe
with NULL `pathfree` (checked at dirfs_subr.c:503).

**Compile neutrality**: both patched and unpatched `dirfs_vnops.c` compile
identically with the same warnings (`filt_dirfswrite`/`filt_dirfsvnode`
unused-function warnings, pre-existing). The fix introduces zero new
compile errors.

**Applies cleanly**: `git apply --check` RC=0; `patch -p1 --dry-run` hunk #1
succeeded at line 978.

## Fix validation (Phase 8)

**not_applicable** — the finding's claimed impact (panic) does not reproduce
(`rename(NULL,...)` returns EFAULT), and dirfs is vkernel-only (not in the
host kernel, so no single-fix host kernel can include it). The fix is a
correctness improvement validated by:
1. Harness transcription (shows ENAMETOOLONG instead of EFAULT)
2. `git apply --check` RC=0
3. Compile-neutral (patched ≡ unpatched)

No kernel build/boot test is needed because there is no reproduced bug to
validate against.

## PoC changes

- **`harness.c`** — written from scratch (no pre-existing PoC). Faithful
  transcription of `dirfs_node_absolute_path_plus` (dirfs_subr.c:377-443)
  and `dirfs_nrename` (dirfs_vnops.c:977-1020), plus live `rename(NULL,...)`
  calls proving EFAULT (not crash).
- **`build.sh` / `run.sh`** — standard build/run wrappers.
- **`fix.diff`** — authored post-verification: NULL check → ENAMETOOLONG.

## How to reproduce

```
ssh dfbsd-maxx   # unprivileged (uid 1001)
cd poc/DF-0808
./build.sh && ./run.sh
# expected: "rename(NULL,...) returns EFAULT" (no crash), proving the
#           finding's "NULL deref panic" claim is false
```