⬢ DragonFlyBSD Kernel Audit
← triage · dashboard
DF-2659

NULL column chain during sync: hammer2_inode_chain_and_parent() cannot return NULL (panics at chain->parent first) and synchro.c evaluates chain->bref.modify_tid BEFORE its own if (chain) guard

Field Value
ID DF-2659
Status new
Severity Low
CVSS 3.1 CVSS:3.1/AV:L/AC:H/PR:H/UI:N/S:U/C:N/I:N/A:H
CWE CWE-476 NULL Pointer Dereference (dead defensive path)
File sys/vfs/hammer2/hammer2_synchro.c
Lines 417-422, 687-692 (helper defect inode.c:453,461,468)
Area vfs
Confidence speculative
Discovered 2026-08-29
Pass 2 (GLM 5.3 second pass)
Bucket hammer2
Reported pending
Known CVE none
CVE match novel

Summary

Both sync-thread call sites expect hammer2_inode_chain_and_parent() to return NULL for a missing local column chain (the if (chain) blocks at synchro.c:422-431 and 697-700 exist for it), but (a) the helper dereferences parent = chain->parent unconditionally at inode.c:453 (and again at :461, plus hammer2_chain_unlock(NULL) at :468) so it panics before any NULL return, and (b) synchro.c:421 evaluates chain->bref.modify_tid BEFORE the if (chain) check at :422, so even a fixed helper would crash at the call site. Within this file's own logic the NULL state is prevented (iroot elements are mount-discovered; deferred inodes receive their idx element at defer time), so reachability is speculative — plausible via error-path element NULLing (hammer2_inode_repoint, inode.c:1430-1438) or element-removal idx shifts (overlaps the known teardown family).

--- a/sys/vfs/hammer2/hammer2_inode.c
+++ b/sys/vfs/hammer2/hammer2_inode.c
@@ -450,6 +450,10 @@
+   if (chain == NULL) {
+       *parentp = NULL;
+       return NULL;
+   }

    /*
     * Get parent, lock order must be (parent, chain).
     */
    parent = chain->parent;
--- a/sys/vfs/hammer2/hammer2_synchro.c
+++ b/sys/vfs/hammer2/hammer2_synchro.c
@@ -418,7 +418,6 @@
-           want_update = (chain->bref.modify_tid != sync_tid);
            if (chain) {
+               want_update = (chain->bref.modify_tid != sync_tid);
                hammer2_chain_unlock(chain);

Timeline

  • 2026-08-29 Discovered during pass-2 audit of hammer2_synchro.c (GLM 5.3).

Discussion (0)

No comments yet.

PoC verification

Evidence pack

findings/poc/DF-2659 · 2 files
FileTypeDescriptionSize
README.md file 2.9 KB ↓ raw
verdict.json file 2.5 KB view raw
README.md file
↓ download raw

DF-2659 — sync of an inode whose local column chain is NULL: guaranteed

NULL deref (hammer2_inode_chain_and_parent cannot return NULL; and

synchro.c dereferences chain->bref BEFORE its own NULL check)

sys/vfs/hammer2/hammer2_synchro.c:417-431 and 687-700 calling sys/vfs/hammer2/hammer2_inode.c:429-478.

Low / hammer2 bucket. Code defect: certain. Reachability: speculative (no demonstrated trigger on this guest — see below).

Root cause (path:line)

hammer2_inode_chain_and_parent() documents and implements a NULL chain path, but never returns NULL — it dereferences first:

sys/vfs/hammer2/hammer2_inode.c:436-453:
    for (;;) {
        ...
        chain = ip->cluster.array[clindex].chain;
        if (chain) { ...lock... } else { ...unlock spin only... }
        parent = chain->parent;            /* <= NULL deref (453) */
        ...
        if (ip->cluster.array[clindex].chain == chain &&
            chain->parent == parent)       /* <= NULL deref (461) */
            break;
        hammer2_chain_unlock(chain);       /* <= unlock(NULL) (468) */

Both sync-thread call sites then contain the illusion of a NULL guard:

sys/vfs/hammer2/hammer2_synchro.c:417-422:
    chain = hammer2_inode_chain_and_parent(ip, idx, &parent, ...);
    want_update = (chain->bref.modify_tid != sync_tid);  /* BEFORE */
    if (chain) {                                           /* the check */
        hammer2_chain_unlock(chain);
        ...

(Second site: synchro.c:687-697, same pattern feeding hammer2_sync_replace + unconditional KKASSERT(parent != NULL) at :692.)

The author clearly expected NULL to be possible (the if (chain) blocks exist) — the evaluation order bug in synchro.c and the impossible NULL return in inode.c are two halves of the same dead defensive path.

Trigger conditions (why speculative)

The crash needs a synced inode (pmp->iroot or a deferred-list inode) whose ip->cluster.array[idx].chain is NULL at pass time. Within this file's own logic that is prevented: iroot's elements are set from the mount-discovered chains (vfsops.c:481-516), and deferred inodes get their idx element set at defer time (synchro.c:601-605 inode_get/repoint_one). Plausible real-world paths that were NOT ruled out and need a live diverged cluster to test: a column whose chain was dropped by error handling (hammer2_inode_repoint NULLing elements, inode.c:1430-1438), or pfsdealloc element removal shifting idx mid-run (overlaps the known DF-0823/2620/2654 teardown family, not re-reported here).

Fix

  • inode.c: return NULL (with *parentp = NULL) when the element is NULL — the loop body must guard chain before touching chain->parent.
  • synchro.c:421: move the want_update evaluation inside if (chain) (skip sync with merror = HAMMER2_ERROR_EIO or ENOENT when NULL).

PoC status

untested — see verdict.json.

Fix verification

not_testable
per-fix-DF-2659

Confirmed kernel references

Detail

Evidence (decisive lines)

findings/poc/DF-2659/README.md (root cause with path:line, trigger analysis, fix)

PoC changes

n/a (no seed PoC existed; audit-time discovery)

Verified recommended fix

Make hammer2_inode_chain_and_parent() return NULL with *parentp=NULL for a NULL element, and move the want_update evaluation (synchro.c:421) inside the existing if (chain) guard.

Verdict

Not verified on the guest (status untested): reaching the crash requires a synced inode whose local cluster column chain is NULL at sync time, and within hammer2_synchro.c's own logic that state is prevented (iroot elements are mount-discovered; deferred inodes receive their idx element at defer time, synchro.c:601-605). The code defect is certain from line-accurate reading: hammer2_inode_chain_and_parent() dereferences chain->parent at inode.c:453/461 before any NULL return is possible (its else-branch only drops the spinlock), and both synchro.c call sites evaluate chain->bref.modify_tid BEFORE their own if (chain) guards (synchro.c:421 vs 422, and 687-697 with KKASSERT(parent != NULL) at 692) - i.e. the NULL path the author wrote defenses for would panic before any defense runs. Real-world triggers plausibly exist via error-path element NULLing (hammer2_inode_repoint, inode.c:1430-1438) or element removal shifting idx mid-run, but demonstrating one needs a live diverged/failed-column cluster and was blocked on this guest by the DF-2620-family teardown UAF (DF-2657 follow-up).