DF-0839 — hammer_ioc_set_version leaks sync_lock and finalize_lock on undo-fail

Summary

hammer_ioc_set_version() (sys/vfs/hammer/hammer_ioctl.c) acquires the mount's finalize_lock (exclusive) and sync_lock (exclusive via hammer_sync_lock_ex), then on the undo-upgrade failure path (goto failed at line 651) jumps past the matching hammer_sync_unlock / hammer_unlock at lines 664–665. Both locks are permanently leaked.

• Severity: Medium (root-only ioctl → local DoS)
• Impact: deadlock/DoS — sync_lock held exclusively forever deadlocks all subsequent HAMMER write/flush operations (20+ sync_lock_sh/ex call sites).
• Root cause: missing unlock on the error cleanup path.

How to reproduce

./build.sh   # cc -o trigger trigger.c
./run.sh     # as root, against a file on a mounted HAMMER v<4 filesystem

The trigger issues HAMMERIOC_SET_VERSION(cur_version=4) on a HAMMER v3 mount. On a valid filesystem the undo upgrade succeeds and locks are released normally (no bug). The lock leak only fires when hammer_upgrade_undo_4() returns an error — which requires hammer_bnew() to fail.

Failure-path reachability analysis

hammer_bnew() with isnew=1 calls hammer_io_new() which calls getblk() and always returns 0. Therefore the undo-upgrade failure path is effectively unreachable on a validly-mounted filesystem under normal conditions. The finding's stated trigger ("memory pressure or crafted UNDO blockmap causing hammer_bnew fail") requires extraordinary conditions:

• Extreme memory pressure (getblk/alloc failures)
• A crafted filesystem image with a corrupted UNDO blockmap whose alloc_offset translates to an out-of-range volume number via hammer_xlate_to_undo() (causing hammer_get_volume() to return NULL → error)

On the default GENERIC kernel (INVARIANTS ON), if the failure were triggered: 1. sync_lock_refs stays at 1 (never decremented) 2. hammer_done_transaction() at hammer_transaction.c:131 trips KKASSERT(trans->sync_lock_refs == 0) → immediate kernel panic

On a production kernel (INVARIANTS OFF): 1. The KKASSERT is a no-op 2. hmp->sync_lock stays exclusively locked forever 3. Every subsequent HAMMER write/flush operation that calls hammer_sync_lock_sh() or hammer_sync_lock_ex() deadlocks permanently

Per the task instructions, since the failure path is too narrow to trigger with a valid image, a deterministic code-level trace is used as the proof.

Expected output

On a valid HAMMER v3 mount (success path — no bug triggered):

[probe] issuing HAMMERIOC_SET_VERSION(cur=4) on /mnt/htest/testfile (fd=3)
[probe] ioctl rc=0 errno=0 head.error=0 head.flags=0x0
[probe] undo-upgrade succeeded (no leak this run)

If the undo-upgrade failure path were hit on GENERIC, expect a kernel panic (KKASSERT in hammer_done_transaction). On production, expect a permanent hang on the next HAMMER sync/flush operation.

DF-0839 — VERDICT

Verdict: REPRODUCED (code-level trace — failure path too narrow for live trigger)

The Bug

hammer_ioc_set_version() in sys/vfs/hammer/hammer_ioctl.c acquires two exclusive locks, then on the undo-upgrade failure path jumps past their release:

hammer_ioctl.c:635   error = hammer_init_cursor(trans, &cursor, NULL, NULL);
hammer_ioctl.c:636   if (error) goto failed;              // OK: before locks
hammer_ioctl.c:638   hammer_lock_ex(&hmp->flusher.finalize_lock);   // ACQUIRE
hammer_ioctl.c:639   hammer_sync_lock_ex(trans);                     // ACQUIRE
hammer_ioctl.c:640   hmp->version = ver->cur_version;
hammer_ioctl.c:646   if (over < 4 && ver->cur_version >= 4) {
hammer_ioctl.c:649       error = hammer_upgrade_undo_4(trans);
hammer_ioctl.c:650       if (error)
hammer_ioctl.c:651           goto failed;              // BUG: skips 664-665
hammer_ioctl.c:652   }
hammer_ioctl.c:657   volume = hammer_get_root_volume(hmp, &error);
hammer_ioctl.c:659   hammer_modify_volume_field(...);
hammer_ioctl.c:664   hammer_sync_unlock(trans);              // SKIPPED on error
hammer_ioctl.c:665   hammer_unlock(&hmp->flusher.finalize_lock);  // SKIPPED
hammer_ioctl.c:666  failed:
hammer_ioctl.c:667   ver->head.error = error;
hammer_ioctl.c:668   hammer_done_cursor(&cursor);
hammer_ioctl.c:669   return(0);

Lock mechanics

• hammer_sync_lock_ex(trans) at hammer_subs.c:747: ++trans->sync_lock_refs (→1) + hammer_lock_ex(&hmp->sync_lock) (exclusive)
• hammer_sync_unlock(trans) at hammer_subs.c:772: --trans->sync_lock_refs (→0) + hammer_unlock(&hmp->sync_lock)
• hammer_done_transaction() at hammer_transaction.c:124: KKASSERT(trans->sync_lock_refs == 0) at line 131

Impact chain

On GENERIC (INVARIANTS ON): 1. goto failed at :651 → sync_lock_refs stays at 1, hmp->sync_lock stays exclusively held, finalize_lock stays exclusively held 2. hammer_done_transaction at :131 → KKASSERT(sync_lock_refs == 0) → panic

On production (INVARIANTS OFF): 1. KKASSERT is a no-op → locks silently leaked 2. hmp->sync_lock exclusively held forever 3. Next hammer_sync_lock_sh()/hammer_sync_lock_ex() call (from flusher, write, inode sync, prune, reblock, rebalance, dedup, mirror, etc.) → permanent deadlock

Why a code-level trace is acceptable

The failure path requires hammer_upgrade_undo_4() to return an error. This function's only failure mode is hammer_bnew() returning an error. However, hammer_bnew() with isnew=1 calls hammer_io_new() (hammer_ondisk.c:908) which calls getblk() and always returns 0. Therefore:

• On a validly-mounted HAMMER filesystem, the failure path is effectively unreachable under normal conditions
• Triggering it requires either extreme memory pressure (system-wide OOM during getblk) or a deliberately corrupted filesystem image whose UNDO blockmap translates to an out-of-range volume number

Per the task instructions: "if too narrow, a deterministic code-level trace proving the lock-acquire-without-matching-release is acceptable."

Ioctl reachability — PROVEN

A live test confirmed the ioctl path is reachable. Using a 12 GB vnode-backed HAMMER v3 image (newfs_hammer -V 3), mounted read-write, the clean upgrade to v4 succeeded as root:

[probe] ioctl rc=0 errno=0 head.error=0 head.flags=0x0
[probe] undo-upgrade succeeded (no leak this run)

The success path acquires and properly releases both locks (lines 638-639 acquire, 664-665 release). The bug is exclusively on the error path.

Privilege boundary

hammer_ioctl() at hammer_ioctl.c:72 performs caps_priv_check(cred, SYSCAP_NOVFS_IOCTL) before any ioctl dispatch. This requires root. An unprivileged user (maxx, uid 1001) receives EPERM:

[probe] ioctl rc=-1 errno=1 (Operation not permitted)

Therefore the realistic threat is root self-DoS: a root user (or a process with SYSCAP_NOVFS_IOCTL capability) triggers the undo-upgrade failure and permanently deadlocks all HAMMER write/flush operations. Medium severity is appropriate.

Exploit chain

None — this is a lock-leak → deadlock (DoS). No memory corruption, no privilege escalation primitive.

The Fix

fix.diff adds hmp->version = over (restore the in-memory version, since line 640 speculatively modified it before the can-fail undo upgrade) plus hammer_sync_unlock(trans) and hammer_unlock(&hmp->flusher.finalize_lock) before the goto failed at line 651.

Disassembly verification (patched kernel #2)

The compiler merged the unlock sequences from both the error and success paths into a shared code block:

Error path (undo upgrade failed):
  1827: mov %r15d,0x144(%r13)     # hmp->version = over (restore)
  182e: jmpq 13df                  # → shared unlock

Shared unlock (both error and success paths):
  13df: lea  trans
  13e6: callq hammer_sync_unlock   # release sync_lock
  13f2: callq hammer_unlock        # release finalize_lock
  13fd: jmpq 642                   # → failed: label (cleanup)

Both paths now release both locks before reaching the failed: cleanup label.

Fix validation

• Baseline (#0 unpatched): clean upgrade v3→v4 succeeds, locks properly released (success path works). The error-path lock leak is confirmed by code inspection.
• Patched (#2 single-fix kernel): clean upgrade v3→v4 succeeds, guest stays up. Disassembly confirms the error path now restores version + releases both locks before jumping to cleanup.
• fix_status: fixed — the fix eliminates the lock leak on the error path.