DF-2644 — NULL-deref of error'd PFS-root chain in hammer2_xop_inode_flush during sync

Site: sys/vfs/hammer2/hammer2_flush.c:1334-1338 (pmp->pfs_iroot_blocksets[clindex] = chain->data->ipdata.u.blockset;)

Reach: unprivileged-triggerable kernel-panic condition on environmental media failure: any PFS whose root-inode chain cannot be re-read from media during a filesystem sync (dying/pulled disk, short read, CRC-failed block evicted from cache, dd-corrupted image) panics the kernel with a NULL-pointer dereference instead of returning an error from sync(2).

Reproduce

Three pieces in this pack:

• inject.diff — environment simulation only (never a fix): sysctl- gated fault injector that fails inode-block reads with EIO (vfs.hammer2.df2644_fail_inode_read). Needed because a real read error cannot be manufactured on the QEMU guest: vnconfig -u is refused while the mount holds the device open (VNIOCDETACH checks disk_getopencount() > 1, sys/dev/disk/vn/vn.c:464-466), and truncating the vn backing file yields short reads without B_ERROR (vnstrategy uses VOP_READ uio, sys/dev/disk/vn/vn.c:342-371).
• fix.diff — the actual fix (guard + error propagation).
• run_df2644_inj.sh — trigger script (mount, dirty PFS root, drop chain data, arm injector, sync).

Kernel A = stock + inject.diff (panic expected). Kernel B = stock + inject.diff + fix.diff (panic must be gone; sync survives; hammer2: chain error during flush messages still present).

# on the guest, with kernel A booted:
sh /root/df2644/run_df2644_inj.sh        # -> panic in hammer2_xop_inode_flush

Expected stock-kernel signature (serial console / vm.sh log): hammer2: chain error during flush messages followed by a kernel page fault cpuid = ...; lapic = ... with backtrace frames hammer2_xop_inode_flush() ... hammer2_flush_core() ... and the faulting instruction in the blockset-copy line (small constant offset deref of a NULL chain->data).

Why the crash is real and not injector-specific

• hammer2_inode_chain(ip, clindex, HAMMER2_RESOLVE_ALWAYS) (hammer2_flush.c:1319) returns the iroot chain locked but error'd when hammer2_chain_load_data() hits an I/O error: hammer2_chain.c:1003-1008 sets chain->error = HAMMER2_ERROR_EIO, releases the dio and returns with chain->data == NULL.
• chain->flags & HAMMER2_CHAIN_FLUSH_MASK (hammer2_flush.c:1322) is satisfied by HAMMER2_CHAIN_ONFLUSH, which any dirty child under the PFS root sets on the iroot chain (hammer2_chain_setflush stops at the inode inflection, hammer2_chain.c:147-166).
• hammer2_flush()/hammer2_flush_core() handle the error'd chain correctly (hammer2_flush.c:671-679: report, accumulate, skip) — the code even prints "hammer2: chain error during flush".
• hammer2_xop_inode_flush() then ignores both the chain error and the flush return value and dereferences chain->data->ipdata.u.blockset at :1336-1337 → NULL-page fault → panic. (The injector only makes the device fail, exactly like failing hardware.)

Stage-1 per-inode flushes in the same sync take the graceful path, which the run log shows as interleaved "unable to fsync inode" messages — demonstrating the contrast between the handled paths and this one.

DF-2644 — VERDICT

Finding: NULL-pointer dereference of an error'd PFS-root chain in hammer2_xop_inode_flush() — sys/vfs/hammer2/hammer2_flush.c:1334-1338 dereferences chain->data->ipdata.u.blockset after a flush that failed because the chain could not be re-read from media.

Classification: REPRODUCED (panic), fix FIXED (site-specific).

1. Root cause

• hammer2_inode_chain(ip, clindex, HAMMER2_RESOLVE_ALWAYS) (hammer2_flush.c:1319) returns the PFS-root (iroot) chain locked; when the underlying inode block cannot be read, hammer2_chain_load_data() (hammer2_chain.c:998-1009) sets chain->error = HAMMER2_ERROR_EIO, releases the dio and returns with chain->data == NULL.
• chain->flags & HAMMER2_CHAIN_FLUSH_MASK (hammer2_flush.c:1322) is satisfied by HAMMER2_CHAIN_ONFLUSH: any dirty child under the PFS root sets ONFLUSH on the iroot chain (hammer2_chain_setflush() stops at the inode inflection, hammer2_chain.c:147-166).
• hammer2_flush()/hammer2_flush_core() handle the error'd chain correctly — hammer2_flush.c:671-679 prints hammer2: chain error during flush, accumulates info->error and skips. flush_recurse handles it too (:1203-1208, :1266-1270 "PARENT ERROR DURING FLUSH LOCK").
• hammer2_xop_inode_flush() then ignores both chain->error and the return value of hammer2_flush() (the call at :1331 discards it) and executes pmp->pfs_iroot_blocksets[clindex] = chain->data->ipdata.u.blockset; at :1336-1337 → NULL-page fault (offset 0x200 = offsetof(ipdata.u.blockset)) → kernel panic.

Environmental trigger: failing/pulled/yanked media, or a corrupted (short/CRC-bad) inode block that was cache-evicted between the dirtying operation and the sync. Result: a recoverable I/O condition becomes an unconditional kernel crash during sync.

2. Reproduction (kernel A = stock + inject.diff, this run)

A real EIO cannot be manufactured safely on the guest (vnconfig -u is refused while the mount holds the device open — VNIOCDETACH checks disk_getopencount() > 1, sys/dev/disk/vn/vn.c:464; truncating the vn backing file yields short reads without B_ERROR — vnstrategy uses VOP_READ uio, sys/dev/disk/vn/vn.c:342-371), so the environment is simulated with inject.diff: a sysctl-gated hook in _hammer2_io_getblk() that forces error = EIO for inode-block reads on the 512MB test volume only — byte-exact real-media semantics (the error is injected exactly where breadnx would report it, after the DIO is fully set up; dio->error propagates through hammer2_io_bread identically to a failed device read).

Fresh hammer2 image (vn2 over /root file), mount, mkdir+touch (ONFLUSH on iroot chain), settle 5 s, sysctl vfs.hammer2.df2644_fail_inode_read armed, sync:

hammer2_chain_load_data: I/O error 000000000240080a: 5
hammer2: chain error during flush            <- the DESIGNED handler works
...
Fatal trap 12: page fault while in kernel mode
fault virtual address   = 0x200              <- NULL chain->data + 0x200
Stopped at      hammer2_xop_inode_flush+0x39c:  repe movsq (%rsi),%es:(%rdi)

(panic.txt; the repe movsq IS the blockset structure copy at hammer2_flush.c:1336-1337. Full serial in panic.txt.)

3. Fix (fix.diff, validated)

Guard the dereference and propagate the flush error (previously the return value of hammer2_flush() was discarded entirely at :1331):

    flush_error |= hammer2_flush(chain, xflags);
    ...
    if (ip == pmp->iroot && pmp != hmp->spmp &&
        chain->error == 0 && chain->data != NULL) {
        ... blockset copy ...
    }

flush_error already gates the volume-header synchronization at :1480, so a failed PFS-root flush no longer writes a new volume header either.

Kernel B (stock + inject.diff + fix.diff, only delta = fix.diff): the identical workload and error-message sequence completes the stage-2 iroot flush with no fault at hammer2_xop_inode_flush (fix_run.log / fix_run_partial_3112.txt: on kernel A the sequence hammer2: chain error during flush is immediately followed by the Fatal trap 12 at hammer2_xop_inode_flush+0x39c; on kernel B the same sequence continues through further handled loads and completes).

4. Residual (out of scope, documented for the chain.c audit)

With media errors persisting past the flush, the guest still panics later at an unrelated pre-existing unconditional assert:

panic: assertion "parent->error == 0" failed in hammer2_chain_create
       at /usr/src/sys/vfs/hammer2/hammer2_chain.c:3112
    via hammer2_xop_inode_create_ins   (fsync-retry index re-insert)
     or via hammer2_assign_physical ← hammer2_xop_strategy_write

(kernel A hit the same 3112 assert via the write path before the injector was volume-gated — see buildA-first-run transcripts.) This is a distinct defect class ("frontend create/write paths KKASSERT on error'd parents instead of returning errors") and belongs to hammer2_chain.c; it should be filed/triaged there. It is not addressed by this finding's fix.diff, by design (no blind cross-file error-handling rewrite).

5. Impact ceiling

Kernel NULL-deref panic (DoS) from an environmental media-failure condition during routine sync — no data-only unprivileged trigger exists (the media must actually fail), hence severity Medium. The bug class is "kernel crashes instead of returning EIO", same family as the gracefully-handled paths surrounding it.