DF-2568 — hammer2_flush retry-loop NULL-deref panic

Finding

hammer2_flush retry loop at sys/vfs/hammer2/hammer2_flush.c:405 calls hammer2_chain_ref(info.parent) without checking for NULL, unlike the initial setup at :382-383 and cleanup at :434-435 which guard. When a concurrent unlink deletes the chain's parent during the flush's unlock/relock window (flush_core:662-665), chain->parent becomes NULL, and the retry's hammer2_chain_ref(NULL) dereferences a NULL pointer → kernel panic.

PoC files

• race_flush.c — v1: workers create+write+fsync+unlink, syncers (basic)
• race_flush_v2.c — v2: shared dir + rename churn
• race_flush_v3.c — v3: population churn + dir churn
• race_flush_v5.c — v5: focused fsync-vs-unlink + rename
• race_flush_v6.c — v6: concurrent dirtier/unlinker shared pool (PRIMARY)
• race_flush_v7.c — v7: 512KB files for indirect chains
• setup.sh — root: create vnode-backed hammer2 fs at /h2mnt
• build.sh — build the PoC
• run.sh — run the PoC as unprivileged user

Build

./build.sh    # or: cc -O2 -pipe -o race_flush race_flush.c

Setup (as root)

./setup.sh    # creates 2G vnode-backed hammer2 fs, mounts at /h2mnt, chowns /h2mnt/race to maxx

Run (as unprivileged user)

./run.sh      # races concurrent create+write+fsync+unlink+sync on /h2mnt/race

Or run a specific variant:

cc -O2 -pipe -o race_flush_v7 race_flush_v7.c
./race_flush_v7 -d /h2mnt/race -D 12 -U 12 -s 4 -p 16 -f 512 -t 300

Expected result

On an unpatched kernel IF the race is won: kernel panic (NULL deref in hammer2_chain_ref) — fatal trap 12: page fault while in kernel mode, fault address near 0.

In practice: The race is extremely narrow. After 45+ minutes of stress testing with 7 PoC variants, the race was never triggered (zero "LOST CHILD" messages with vfs.hammer2.debug=0x40). The code bug is real but the race window is too narrow for userspace concurrency to hit reliably.

Fix

See fix.diff — adds if (info.parent != NULL) guard before hammer2_chain_ref(info.parent) at flush.c:405, matching the pattern at :382-383 and :434-435.

DF-2568 — hammer2_flush retry-loop NULL-deref (verification verdict)

Verdict: NOT REPRODUCED (race too narrow) — CODE BUG CONFIRMED, fix.diff validated

Summary

The code bug is real and confirmed: hammer2_flush retry loop at sys/vfs/hammer2/hammer2_flush.c:405 calls hammer2_chain_ref(info.parent) WITHOUT a NULL check, unlike the initial setup at :382-383 (if ((info.parent = chain->parent) != NULL)) and the cleanup at :434-435 (if (info.parent) hammer2_chain_drop(info.parent)). If info.parent is NULL at :405, hammer2_chain_ref(NULL) dereferences &NULL->refs → kernel page-fault panic.

However, the race required to trigger the NULL condition was not reproducible in 45+ minutes of aggressive stress testing across 7 PoC variants. Zero "LOST CHILD" messages (the kernel's own debug indicator that chain->parent changed during a flush unlock/relock window) were observed, meaning the race window at flush_core:681 was never reached.

Mechanism (theoretical)

The race requires chain->parent to change from non-NULL to NULL during the flush_core unlock/relock window at hammer2_flush.c:662-665:

661: hammer2_chain_ref_hold(chain);
662: hammer2_chain_unlock(chain);              // ← chain unlocked
663: if (parent)
664:     hammer2_chain_lock(parent, ...);       // ← parent locked
665: hammer2_chain_lock(chain, ...);           // ← chain relocked

Between :662 (chain unlock) and :664 (parent lock), both chain and parent are unlocked. A concurrent unlink xop (hammer2_xop_unlink in hammer2_xops.c:334) that has already locked the parent directory and found the chain in its rbtree can:

1. Lock the chain (at hammer2_chain_lookup → hammer2_chain_lock)
2. Call hammer2_chain_delete(parent, chain, ...) → _hammer2_chain_delete_helper at hammer2_chain.c:3556-3559 sets chain->parent = NULL
3. The flush relocks chain at :665, sees chain->parent != parent at :681 → retry=1
4. Back in hammer2_flush retry loop at :397: info.parent != chain->parent → enters the block, :403 drops old parent, :404 sets info.parent = NULL, :405 calls hammer2_chain_ref(NULL) → NULL deref panic

The flush xop (hammer2_xop_inode_flush at hammer2_flush.c:1292) and unlink xop (hammer2_xop_unlink) are dispatched to different xop thread groups (different inode hashes: file inode vs parent directory inode), so they CAN run concurrently on different CPUs.

Why the race was NOT triggered

Despite exhaustive testing, the race was never hit. Key factors:

1. Extremely narrow timing window: The gap between :662 (chain unlock) and :664 (parent lock) is just a few function calls. The concurrent unlink xop must have already locked the parent AND found the chain in the rbtree AND be blocking on the chain lock at the precise moment the flush releases it at :662.

2. XOP dispatch overhead: The unlink xop must be dispatched, picked up by an xop worker thread, lock the parent, call hammer2_chain_lookup (which scans the rbtree), and attempt to lock the child — all before the flush completes the downward recursion and reaches :662. For simple file inodes with few children, the downward recursion is very fast.

3. No LOST CHILD observed: With vfs.hammer2.debug=0x40 enabled (which prints all 4 "LOST CHILD" variants when chain->parent changes during flush), zero messages appeared across all runs. This means the race at flush_core:681 was never reached, not even partially.

Testing performed

Total: ~45 minutes of stress across vnode-backed and root hammer2 filesystems, with spread_workers=0 and =1, debug=0x40, 12-30 concurrent processes.

Configurations tested: - File sizes: 48B to 512KB (to vary flush tree depth) - Pool sizes: 8 to 64 files (to vary collision probability) - Worker counts: 12-30 processes - vfs.hammer2.spread_workers: 0 and 1 - vfs.hammer2.debug: 0x40 (LOST CHILD messages enabled) - Filesystems: vnode-backed hammer2 (/h2mnt) and root hammer2 (/)

Fix

The fix adds a NULL check before hammer2_chain_ref(info.parent) at :405, matching the pattern at :382-383 and :434-435:

--- a/sys/vfs/hammer2/hammer2_flush.c
+++ b/sys/vfs/hammer2/hammer2_flush.c
@@ -402,7 +402,8 @@
            }
            hammer2_chain_drop(info.parent);
            info.parent = chain->parent;
-           hammer2_chain_ref(info.parent);
+           if (info.parent != NULL)
+               hammer2_chain_ref(info.parent);
        }

This is a trivially-correct defensive fix: the existing code at :382-383 already handles info.parent == NULL by conditionally referencing, and the cleanup at :434-435 conditionally drops. The retry path at :405 was the only place missing this guard. With the fix, if chain->parent becomes NULL during the flush (as the race theoretically allows), the retry loop simply skips the ref and continues with info.parent = NULL, which flush_core handles correctly (parent is allowed to be NULL per the comment at :504).

Fix validation

The fix.diff was applied to the in-guest source, compiled into a single-fix kernel (make -j6 nativekernel, rc=0), installed as /boot/kernel/kernel, and booted as 6.5-DEVELOPMENT #1. The patched kernel boots correctly and handles the stress workload without issues.

Since the race did not trigger on the unpatched kernel (no panic to compare against), the fix cannot be validated via before/after panic comparison. Instead, the fix is validated by: 1. Compiles cleanly (rc=0, no warnings) 2. Boots correctly (#1 kernel, all services start) 3. Handles stress workload (240s stress run, no panic, guest stays up) 4. Code correctness is obvious (matches existing guarded patterns)

fix_status: not_testable — the race can't be triggered to produce a panic on the unpatched kernel, so no before/after panic comparison is possible. The fix.diff applies, compiles, and boots; the code change is trivially correct.