# DF-0813 — NULL deref in hammer2_flush() retry loop when chain->parent becomes NULL ## Verdict **NOT REPRODUCED** (race-condition DoS; code defect CONFIRMED real but trigger is an extremely tight race that did not fire in ~15 min of aggressive hammer2 churn). ## The code defect (CONFIRMED — path:line) The finding is correct at the source level. `sys/vfs/hammer2/hammer2_flush.c` contains a retry loop in `hammer2_flush()` whose body dereferences `info.parent` without a NULL guard, while BOTH the setup and teardown of the same function correctly guard against NULL: **Setup (guarded):** `hammer2_flush.c:382-383` ```c if ((info.parent = chain->parent) != NULL) hammer2_chain_ref(info.parent); ``` **Retry loop body (BUG — unguarded):** `hammer2_flush.c:403-405` ```c if (info.parent != chain->parent) { ... hammer2_chain_drop(info.parent); /* line 403 — no NULL check */ info.parent = chain->parent; /* line 404 — now NULL if chain->parent==NULL */ hammer2_chain_ref(info.parent); /* line 405 — derefs NULL -> panic */ } ``` **Teardown (guarded):** `hammer2_flush.c:434-435` ```c if (info.parent) hammer2_chain_drop(info.parent); ``` The inconsistency is unambiguous: setup and teardown guard `info.parent` against NULL; the loop body does not. If the loop body runs with `info.parent == NULL` (or `chain->parent == NULL` after line 404), the calls to `hammer2_chain_drop(NULL)` / `hammer2_chain_ref(NULL)` dereference a NULL pointer: - `hammer2_chain_ref(NULL)` → `atomic_fetchadd_int(&chain->refs, 1)` at `hammer2_chain.c:260` with `chain==NULL` → access at `offsetof(hammer2_chain_t, refs)` below the zero page → fatal trap 12 (page fault). - `hammer2_chain_drop(NULL)` → `KKASSERT(chain->refs > 0)` at `hammer2_chain.c:350` → same NULL deref. ## Mechanism (theoretically reachable) For the loop body at line 397 to execute, `info.parent != chain->parent` must hold on a retry. This requires `chain->parent` to change during `flush_core`: 1. `flush_core` unlocks `chain` at `hammer2_flush.c:662` (`hammer2_chain_unlock`). 2. During the unlock window (662→665 relock), a concurrent unlink/delete can run `_hammer2_chain_delete_helper` (`hammer2_chain.c:3520`), which under `chain->core.spin` + `parent->core.spin` (lines 3550-3551) sets `chain->parent = NULL` at `hammer2_chain.c:3559`. 3. After relock, `flush_core` detects `chain->parent != parent` at line 681, sets `retry=1`, and returns to the loop. 4. Back at line 397: `info.parent` (old non-NULL parent) != `chain->parent` (now NULL) → body enters → line 404 sets `info.parent=NULL` → line 405 `hammer2_chain_ref(NULL)` → **panic**. The path is reachable in principle. `hammer2_chain_delete` is called from the VOP unlink/remove xop path (`hammer2_xops.c:420,449,641,...`), and the flush is driven by user-initiated `sync()`/`fsync()`. ## Why it did NOT reproduce (the race is extremely tight) Two stress harnesses (`churn_h2.c` = create/write/fsync/unlink/sync workers; `dirstress_h2.c` = heavy single-directory topology churn to force indirect-block split/merge + concurrent sync) were run for a cumulative ~15 minutes across two hammer2 mounts (`/mnt/h2` vn-backed image and `/tmp/h2churn` on the root hammer2 fs), with `vfs.hammer2.debug=0x0040` enabled (which prints "LOST CHILD4" at `hammer2_flush.c:398-402` whenever the retry-loop body fires AT ALL). **Result: zero "LOST CHILD4" events** — the retry-loop body at line 397 never executed during any run. This means a chain's parent never changed while a flush was processing it. The race window (flush_core's unlock at :662 → relock at :665) is only a few lock operations wide, and the hammer2 flush/delete serialization prevented the overlap from occurring in practice. Zero panics on both the unpatched #0 and patched #7 kernels. ## Impact ceiling A NULL deref at a fixed offset (atomic_fetchadd_int at &NULL->refs, or KKASSERT on NULL->refs) is a **pure DoS (kernel panic)** — no memory corruption, no attacker-controlled write target, no escalation path. The panic would crash the system but cannot be leveraged for code execution or privilege escalation. Realistic trigger: an unprivileged user with write access to a mounted hammer2 filesystem doing heavy concurrent modify/unlink + sync operations. The race is probabilistic and very tight; it may require extended churn to win (if winnable at all from pure userspace). ## Fix (fix.diff) The fix adds NULL guards in the retry loop body, making it consistent with the setup (line 382-383) and teardown (line 434-435): ```diff --- a/sys/vfs/hammer2/hammer2_flush.c +++ b/sys/vfs/hammer2/hammer2_flush.c @@ -400,9 +400,11 @@ info.parent, chain, chain->parent); } - hammer2_chain_drop(info.parent); + if (info.parent) + hammer2_chain_drop(info.parent); info.parent = chain->parent; - hammer2_chain_ref(info.parent); + if (info.parent) + hammer2_chain_ref(info.parent); } ``` This is a minimal, targeted defense-in-depth fix: if `chain->parent` becomes NULL during a retry, the loop simply drops the old parent (if any) and sets `info.parent=NULL` without crashing; subsequent `flush_core` and teardown handle a NULL parent correctly (they all guard with `if (parent)` / `if (info.parent)`). ## Fix validation (Phase 8) - **fix.diff applies cleanly**: `git apply --check` and `patch -p1` both succeed. - **Patched kernel compiles**: `make -j6 nativekernel KERNCONF=X86_64_GENERIC` produced `kernel.stripped` (15.7 MB) and `kernel.debug` (119 MB) with zero compile errors. `hammer2_flush.o` rebuilt successfully. - **Patched kernel boots**: `kern.version` = `6.5-DEVELOPMENT #7` (today's ts). - **PoC re-run on patched kernel**: both harnesses ran for ~3 min with no panic, no regression, guest healthy. - **fix_status: not_testable** — since the race did not fire on EITHER the unpatched baseline or the patched kernel, a behavioral before/after comparison is not possible. The fix is validated at the compile + boot + code-inspection level: the NULL-deref path at lines 403/405 is definitively closed. ## PoC changes - Authored `churn_h2.c` (create/write/fsync/unlink/sync churn harness). - Authored `dirstress_h2.c` (directory-topology-stress variant for indirect-block churn). - Authored `setup_h2.sh` / `teardown_h2.sh` (root: create vn-backed hammer2 image, newfs, mount). - Authored `fix.diff` (NULL guards in the retry loop body). ## Kernel references (confirmed during verification) - `sys/vfs/hammer2/hammer2_flush.c:397` — retry loop condition (`info.parent != chain->parent`) - `sys/vfs/hammer2/hammer2_flush.c:403` — unguarded `hammer2_chain_drop(info.parent)` - `sys/vfs/hammer2/hammer2_flush.c:405` — unguarded `hammer2_chain_ref(info.parent)` ← NULL deref - `sys/vfs/hammer2/hammer2_flush.c:382-383` — setup (correctly guarded) - `sys/vfs/hammer2/hammer2_flush.c:434-435` — teardown (correctly guarded) - `sys/vfs/hammer2/hammer2_flush.c:662` — `hammer2_chain_unlock(chain)` (race window open) - `sys/vfs/hammer2/hammer2_flush.c:665` — `hammer2_chain_lock(chain)` (race window close) - `sys/vfs/hammer2/hammer2_flush.c:681-689` — parent-change detection, sets retry=1 - `sys/vfs/hammer2/hammer2_chain.c:3559` — `chain->parent = NULL` in delete_helper - `sys/vfs/hammer2/hammer2_chain.c:260` — `atomic_fetchadd_int(&chain->refs,1)` in chain_ref - `sys/vfs/hammer2/hammer2_chain.c:350` — `KKASSERT(chain->refs > 0)` in chain_drop