DF-2661 — hammer2 DIO error path leaves stale dio->bp with DIO_GOOD cleared → state-machine violation, panic on concurrent accessor

• File: sys/vfs/hammer2/hammer2_io.c (pass-2 audit of this file)
• Root cause: _hammer2_io_getblk() error path (io.c:350-373 vs io.c:254)
• Severity: Medium (kernel panic / local DoS on device read error)
• Bucket: hammer2
• Related: DF-2662 (second EIO-path panic unmasked by this fix — different file, hammer2_iocom.c)

Kernels used (all built in-guest with make -j6 nativekernel)

• A (baseline) = stock + inject.diff (build.sh; log lost to the mandatory post-panic vm.sh reset with-src — procedure identical to kernel B's minus fix_b.diff)
• B (fix 1) = stock + inject.diff + fix_b.diff (build.log / buildB.log) — DF-2661's assert is GONE; the same storm then exposes the unrelated DF-2662 NULL deref (hammer2_update_spans)
• C (fix 1+2) = kernel B source + DF-2662's fix.diff (fix_build.log / DF-2662/buildC.log)

Media prep (once per fresh guest)

sh /root/df2661/trigger_df2661.sh golden

512MB vn(4)-backed hammer2 volume, 40 inodes in one directory (one 64KB DIO window), golden image materialized via dd if=/dev/vn2 while attached. NEVER use vnconfig -T on prepared images (it is O_TRUNC).

Run (per kernel)

sh /root/df2661/trigger_df2661.sh 5        # ARM=1 (default): EIO storm
ARM=0 sh /root/df2661/trigger_df2661.sh 5  # control: no EIO

Expected / observed

• Kernel A: panic assertion "dio->bp == NULL" failed in _hammer2_io_getblk at hammer2_io.c:264 (= stock :254 + 10 injector lines) — guest wedged in ddb (panic.txt, run_baseline.txt).
• Kernel B + ARM=0 control: 5/5 ROUND_SURVIVED (crash is EIO-specific).
• Kernel B + ARM=1: no dio assert anymore; crashes in DF-2662's hammer2_update_spans instead (see DF-2662 pack).
• Kernel C + ARM=1: 5/5 ROUND_SURVIVED + ALL_ROUNDS_SURVIVED, RC=0 (fix_run.log, run twice).

DF-2661 VERDICT — hammer2 DIO error path leaves stale dio->bp, panics concurrent accessor

Status: REPRODUCED (panic, kernel A) / FIXED (kernel C)

The fix

fix.diff (git-apply-able against pristine sys/), after dio->error = error; (io.c:356):

if (error && dio->bp) {
    brelse(dio->bp);
    dio->bp = NULL;
}

This restores the invariant "GOOD clear && INPROG clear => bp == NULL" that io.c:254 asserts and that _hammer2_io_putblk()'s errored disposal path (else if (bp) brelse(bp)) now simply never sees.

The bug

_hammer2_io_getblk() (sys/vfs/hammer2/hammer2_io.c) implements the DIO state machine with flag bits in dio->refs: DIO_INPROG (one thread owns buffer instantiation), DIO_GOOD (dio->bp is stable and usable).

On a FAILED read the completion loop (io.c:361-373) clears INPROG, sets GOOD only if error == 0, and leaves dio->bp pointing at the errored buffer. The buffer is only disposed on the refs 1->0 "lastdrop" transition in _hammer2_io_putblk() (io.c:458-512).

That breaks the state machine's own invariant, asserted at io.c:254:

KKASSERT(dio->bp == NULL);  /* we just acquired INPROG, GOOD clear */

Any second thread that took a DIO ref while the first thread was inside the INPROG section (e.g. a concurrent load of a different inode in the same 64KB window — hammer2 packs ~128 512-byte inodes per DIO) wakes from its tsleep, acquires INPROG, and trips the assert -> panic.

Stack (serial console, kernel A):

_hammer2_io_bread()
hammer2_chain_load_data()
hammer2_chain_lock()
hammer2_inode_chain()

The garbled console line in panic.txt is two CPUs printing simultaneously: CPU1 emitting hammer2_chain_load_data: I/O error (chain.c:1001) while CPU2 emits the panic — the two-actor race caught in the act.

Reproduction (kernel A = stock + sysctl-gated EIO injector)

• Injector (inject.diff, technique from DF-2644): forces error = EIO exactly where a real media error lands — after bp is instantiated, before the GOOD decision. Gated to INODE brefs on a 512MB test volume; guest root fs (different size) is untouched.
• Media: newfs_hammer2 on vn(4) over a file; 40 inodes created in one directory (one 64KB window); golden image materialized with dd if=/dev/vn2 while still attached (serves dirty buffer content).
• Per round: fresh clone of golden (NEVER vnconfig -T — it is O_TRUNC and zeroes the backing file), attach WITHOUT -T, mount (cold chains + cold device buffers -> real strategy I/O inside INPROG), ls to warm the path walk without statting file inodes, arm injector, then 8 concurrent cat loops on 8 distinct cold inodes in the same window.
• Result: panic assertion "dio->bp == NULL" failed in _hammer2_io_getblk at /usr/src/sys/vfs/hammer2/hammer2_io.c:264 (= stock line 254; +10 injector lines), guest wedged in ddb. See panic.txt / panic_full.txt.

Non-INVARIANTS behavior (not separately built, code-traced)

With the assert compiled out, the second accessor re-runs breadnx(), which reuses the passed *bpp (vfs_bio.c:905-907) — the stale bp is re-strategied and the read self-heals; but a DOP_NEW accessor takes the dio->bp = getblk(...) assignment (io.c:267), dropping the held bp pointer without brelse, orphaning the buffer lock (subsequent getblk on that (dev,loffset) blocks). Same root cause, same fix.

Fix validation

Three kernels were built in-guest (see README.md):

• A (stock + injector): baseline panic at the dio->bp assert (panic.txt) — DF-2661 reproduced.
• B (stock + injector + fix.diff): the assert is GONE. With the storm armed the box now dies later and elsewhere — in hammer2_update_spans (hammer2_iocom.c), a second, independent, pre-existing EIO-path NULL deref that DF-2661's earlier panic had been masking. Filed separately as DF-2662 (its own pack has the crash, disassembly and fix). A control run with the injector disarmed (ARM=0) survived 5/5 rounds on kernel B, proving the fix did not introduce a generic mount/umount failure.
• C (kernel B + DF-2662's fix): the identical 5-round EIO storm survives completely — ROUND_SURVIVED x5, ALL_ROUNDS_SURVIVED, RC=0 (fix_run.log; run twice for good measure). EIOs surface as ordinary open() failures.

Threat model

Unprivileged users cannot cause a device read error, but device read errors are routine environmental events (dying disks, cable/host resets on external storage, dm-crypt/VPN-backed block devices hiccuping, virtual disk backends under memory pressure). When one occurs, any multi-threaded metadata workload touching the affected 64KB window (parallel ls/stat/open on one directory is enough) panics the system on any kernel built with INVARIANTS (X86_64_GENERIC default). Impact: local DoS / availability. No memory corruption, no privilege boundary crossed: severity Medium.

Kernel references

• sys/vfs/hammer2/hammer2_io.c:254 (assert), :350-356 (error kept, bp left), :361-373 (GOOD gated on error==0), :458-512 (bp only disposed at lastdrop)
• sys/vfs/hammer2/hammer2_chain.c:985-1006 (concurrent loaders of chains sharing one DIO; error path bqrelse while refs>1 leaves bp set)
• sys/kern/vfs_bio.c:905-907 (breadnx reuses non-NULL *bpp)