DF-0800 — Missing NULL check on hammer2_inode_chain in truncate reset path

Finding: sys/vfs/hammer2/hammer2_xops.c:1603-1607 — hammer2_xop_inode_chain_sync() re-fetches the inode chain after the truncate-down delete loop but does NOT check the return value for NULL before dereferencing it in kprintf(..., parent->data->ipdata.filename). Every other xop in this file (and the first fetch in this very function at :1547) tests for NULL; this is the sole exception.

Severity: Low (CVSS 3.1 AV:L/AC:H/PR:L/UI:N/S:U/C:N/I:N/A:L, CWE-476).

Files

Reproduce

The PoC runs as the unprivileged user on the HAMMER2 root filesystem:

./build.sh
./run.sh

Expected output

Bug present (unpatched #0):

DF-0800: completed 8 grow/shrink/fsync cycles
DF-0800: done.
RUN_EXIT=0
# dmesg: 0 (no "TRUNCATE RESET" — reset block is dead code, see VERDICT.md)

Fixed kernel:

DF-0800: completed 8 grow/shrink/fsync cycles
DF-0800: done.
RUN_EXIT=0
# dmesg: 0 (identical — no behavioral change since the path was unreachable)

The vulnerable reset block at hammer2_xops.c:1600-1608 is dead code in the current codebase: the hammer2_chain_lookup/hammer2_chain_next iteration always returns parent to the inode (type HAMMER2_BREF_TYPE_INODE) because the upward-recursion loop in hammer2_chain_lookup (:2429-2439) walks parent back to the inode whenever key_end = HAMMER2_KEY_MAX exceeds any indirect block's range. The NULL check is therefore a defense-in-depth fix that makes the code safe if the iteration logic ever changes. See VERDICT.md for the full chain-iteration trace.

DF-0800 — VERDICT

Verdict: NOT REPRODUCED (latent/dead-code bug) — fix validated as defense-in-depth

The missing NULL check at hammer2_xops.c:1603-1607 is real — it is the sole xop in hammer2_xops.c that does not test the return of hammer2_inode_chain() for NULL. However, the vulnerable code block (the "reset" at :1600-1608) is dead code in the current codebase: the chain-iteration logic in hammer2_chain_lookup/hammer2_chain_next always leaves parent pointing at the inode after the truncate-down delete loop, so the parent->bref.type != HAMMER2_BREF_TYPE_INODE guard at :1600 is always false and the block never executes.

The bug (code-level)

hammer2_xop_inode_chain_sync() (hammer2_xops.c:1536) handles the HAMMER2_INODE_RESIZED truncate-down case (:1558-1609). After deleting data chains beyond EOF via the hammer2_chain_lookup/hammer2_chain_next loop (:1570-1595), it optionally re-fetches the inode chain:

// hammer2_xops.c:1600-1608
if (parent->bref.type != HAMMER2_BREF_TYPE_INODE) {
    hammer2_chain_unlock(parent);
    hammer2_chain_drop(parent);
    parent = hammer2_inode_chain(xop->head.ip1,
                                 clindex,
                                 HAMMER2_RESOLVE_ALWAYS);
    kprintf("xop_inode_chain_sync: TRUNCATE RESET on '%s'\n",
            parent->data->ipdata.filename);  // ← NULL deref if parent==NULL
}

hammer2_inode_chain() (hammer2_inode.c:408) returns NULL when: - clindex >= cluster->nchains, or - cluster->array[clindex].chain == NULL (gapped/degraded slot, or slot cleared by a concurrent hammer2_inode_repoint()).

Every other xop checks: xop_readdir (:204 area), xop_nresolve (:263), xop_unlink (:352), xop_bmap (:1656), and the first fetch in this very function at :1547-1550. This is the sole exception.

Why the reset block is dead code (chain-iteration trace)

The reset block only fires when parent->bref.type != HAMMER2_BREF_TYPE_INODE after the delete loop. Tracing why it is always HAMMER2_BREF_TYPE_INODE:

The delete loop uses key_end = HAMMER2_KEY_MAX (0xFFFFFFFFFFFFFFFF):

// hammer2_xops.c:1570-1595
chain = hammer2_chain_lookup(&parent, &key_next,
                             lbase, HAMMER2_KEY_MAX, ...);
while (chain) {
    ...
    chain = hammer2_chain_next(&parent, chain, &key_next,
                               key_next, HAMMER2_KEY_MAX, ...);
}

hammer2_chain_next() (hammer2_chain.c:2726) with a non-NULL chain computes key_beg = chain->bref.key + (1 << chain->bref.keybits) and calls hammer2_chain_lookup(). Inside hammer2_chain_lookup(), the very first thing is an upward-recursion loop (hammer2_chain.c:2429-2439):

while (parent->bref.type == HAMMER2_BREF_TYPE_INDIRECT ||
       parent->bref.type == HAMMER2_BREF_TYPE_FREEMAP_NODE) {
    scan_beg = parent->bref.key;
    scan_end = scan_beg + ((hammer2_key_t)1 << parent->bref.keybits) - 1;
    if ((parent->flags & HAMMER2_CHAIN_DELETED) == 0) {
        if (key_beg >= scan_beg && key_end <= scan_end)
            break;
    }
    parent = hammer2_chain_repparent(parentp, how_maybe);
}

Because key_end = HAMMER2_KEY_MAX = 0xFFFFFFFFFFFFFFFF, the condition key_end <= scan_end can only be true if scan_end >= KEY_MAX, which requires scan_beg + (1 << keybits) to overflow past 2^64 — impossible for any real indirect block (whose keybits < 64). Therefore the loop unconditionally walks parent up via hammer2_chain_repparent() until parent->bref.type == HAMMER2_BREF_TYPE_INODE (the inode, which breaks the while condition). After this, parent is the inode.

If the subsequent search in the inode's blockset finds no more chains, the lookup returns NULL and the while loop in hammer2_xop_inode_chain_sync exits with parent == inode.

hammer2_chain_repparent() (hammer2_chain.c:2201) cannot return NULL — it panic("hammer2_chain_repparent: no parent") if chain->parent is NULL (:2216). So the upward recursion always reaches the inode.

Conclusion: After the delete loop, parent->bref.type is always HAMMER2_BREF_TYPE_INODE. The reset block at :1600 never fires. The missing NULL check is in dead code. The kprintf("TRUNCATE RESET...") message never appears in dmesg — confirmed across the baseline run.

PoC results (baseline #0 kernel)

DF-0800: wrote 1048576 bytes and fsync'd
DF-0800: truncated to 65536 bytes
DF-0800: fsync after truncate — drives the inode_chain_sync xop
DF-0800: completed 8 grow/shrink/fsync cycles
RUN_EXIT=0
dmesg TRUNCATE count: 0

No panic, no crash, no "TRUNCATE RESET" message. The PoC exercises the truncate path but the vulnerable reset block is not reached.

When COULD the path become reachable?

The reset block would fire if: 1. The chain iteration ever leaves parent at an indirect block (e.g., a future change to hammer2_chain_next semantics, or a key_end less than HAMMER2_KEY_MAX). 2. A concurrent hammer2_inode_repoint() clears the cluster chain slot during the unlock+drop+reacquire window at :1601-1605 (on a multi-node HAMMER2 cluster with slave synchronization). On this guest's single-node root fs, vfs.usermount=0 and there is only one chain slot, so this race cannot be triggered from userspace.

The fix

fix.diff adds two changes: 1. NULL check (if (parent == NULL) { error = HAMMER2_ERROR_EIO; goto done; }) matching the pattern at :1547 and every other xop. 2. Bounded %.*s using parent->data->ipdata.meta.name_len instead of unbounded %s on the 256-byte filename[] buffer (which is not guaranteed NUL-terminated on a crafted image — secondary hardening).

Fix validation

• Baseline (#0, unpatched): PoC exits 0, no panic, 0 "TRUNCATE RESET" in dmesg. The reset block does not fire (dead code).
• Fixed kernel (#0 rebuilt Jul 13 22:33, sha256 7e84f51c...): PoC exits 0, no panic, 0 "TRUNCATE RESET" in dmesg. Identical behavior — no regression. The fix compiles cleanly and is present in the kernel (strings /boot/kernel/kernel shows '%.*s' format, not '%s').

Since the vulnerable path is dead code, there is no behavioral before/after difference. The fix is defense-in-depth: it protects against the path becoming reachable through future code changes or on crafted/multi-node HAMMER2 images, and brings consistency with every other xop in the file.

fix_status: fixed — fix compiles, boots, no regression; the missing NULL check is now present.