DF-0819 — blkmap_lock leaked on hammer_bnew error paths

Verdict: REPRODUCED (code-level) / LATENT — fix VALIDATED

The bug is a real, confirmed code defect: two goto failed statements in hammer_blockmap_alloc() and hammer_blockmap_reserve() jump past the hammer_unlock(&hmp->blkmap_lock) call, permanently leaking the lock on the error path. If that error path is ever reached, the entire HAMMER filesystem deadlocks permanently (every subsequent metadata operation blocks forever in hammer_lock_ex() on blkmap_lock, unkillable D-state).

However, the error path is effectively dead code under normal operation: hammer_bnew()/hammer_bnew_ext() call through to hammer_io_new() which always returns 0 (getblk either succeeds or panics). Runtime testing on a mounted HAMMER v1 filesystem confirmed the path cannot be triggered — the filesystem returns clean ENOSPC (handled upstream by _hammer_checkspace) and never deadlocks.

Classification: latent lock-leak bug — the code pattern is objectively wrong, the deadlock impact is certain IF the path is reached, but the path requires filesystem corruption / hardware I/O error / crafted image to trigger (it cannot be reached from a normally-functioning filesystem).

Mechanism (confirmed by code trace)

Bug 1: hammer_blockmap_alloc() (sys/vfs/hammer/hammer_blockmap.c:95)

285:  hammer_lock_ex(&hmp->blkmap_lock);        // LOCK ACQUIRED
...
372:  if ((next_offset & HAMMER_BUFMASK) == 0) {
373:      hammer_bnew_ext(trans->hmp, next_offset, bytes,
374:                      errorp, &buffer3);
375:      if (*errorp) {
376:          result_offset = 0;
377:          goto failed;                        // <<<< JUMPS PAST UNLOCK
378:      }
379:  }
...
392:  hammer_unlock(&hmp->blkmap_lock);          // UNLOCK (skipped by goto)
393: failed:                                    // label is AFTER the unlock

goto failed at line 377 targets label at line 393, which is AFTER the unlock at line 392. The lock acquired at line 285 is NEVER released on this path.

Bug 2: hammer_blockmap_reserve() (sys/vfs/hammer/hammer_blockmap.c:419)

546:  hammer_lock_ex(&hmp->blkmap_lock);        // LOCK ACQUIRED
...
602:  if (bytes < HAMMER_BUFSIZE && (next_offset & HAMMER_BUFMASK) == 0) {
603:      if (!vm_paging_min_dnc(HAMMER_BUFSIZE / PAGE_SIZE)) {
604:          hammer_bnew(hmp, next_offset, errorp, &buffer3);
605:          if (*errorp)
606:              goto failed;                    // <<<< JUMPS PAST UNLOCK
607:      }
608:  }
...
611:  hammer_unlock(&hmp->blkmap_lock);          // UNLOCK (skipped by goto)
613: failed:                                    // label is AFTER the unlock

Same pattern: goto failed at line 606 targets line 613, past the unlock at 611.

Impact if triggered

blkmap_lock serializes ALL HAMMER blockmap (metadata + data) allocation. A leaked lock means every subsequent hammer_blockmap_alloc/reserve/finalize/ free/dedup call blocks forever in hammer_lock_ex(). This is an unkillable D-state deadlock affecting the entire mounted HAMMER filesystem.

Trigger reachability analysis

The error path requires hammer_bnew()/hammer_bnew_ext() to set *errorp. Tracing the call chain:

hammer_bnew_ext (hammer_ondisk.c:1178)
  → _hammer_bread (hammer_ondisk.c:1116)
    → hammer_get_buffer (hammer_ondisk.c:696)
      → hammer_load_buffer(buffer, isnew=1) (hammer_ondisk.c:881)
        → hammer_io_new() (hammer_io.c:437)
          → getblk(devvp, ...) → return(0)  // ALWAYS returns 0

hammer_io_new() at hammer_io.c:437 always returns 0 (line 460). It calls getblk() which either succeeds or panics (NULL deref at bp->b_ops on line 444 if getblk returns NULL). So the isnew=1 path through hammer_load_buffer cannot produce a non-zero error.

hammer_get_buffer() CAN set *errorp via hammer_get_volume() (returns ENOENT if zone2_offset decodes to a non-existent vol_no — filesystem corruption) or hammer_load_volume() (I/O error on backing device). These are realistic on a corrupted/worn device or crafted image but NOT on a normally-functioning filesystem.

Runtime confirmation: mounted a 12GB HAMMER v1 filesystem, wrote 68,760 16K-aligned blocks under memory pressure (3GB mmap+memset), then filled to 100% ENOSPC and continued writes. The filesystem returned clean ENOSPC errors in every case — never deadlocked. The error path was never reached.

Fix

Add hammer_unlock(&hmp->blkmap_lock) before each goto failed that occurs after the lock is acquired. See fix.diff.

• Line 377 (hammer_blockmap_alloc): added unlock before goto failed
• Line 606 (hammer_blockmap_reserve): added braces + unlock before goto failed

The fix is a pure safety addition: it only adds an hammer_unlock call on an error path that was previously missing it. On the success path (which is the only path reachable in practice), nothing changes.

Fix validation

• Unpatched baseline (#0): code trace confirms goto failed at lines 377/606 skip hammer_unlock at lines 392/611. Runtime: filesystem works correctly (error path is dead code).
• Patched kernel (#1): hammer_unlock(&hmp->blkmap_lock) now present at lines 377 and 607 (error paths). Built single-fix kernel, booted, verified HAMMER v1 filesystem still works correctly (newfs, mount, write, read, stress, ENOSPC — all clean, no regression).
• Result: fix is correct and introduces no regression. The lock is now released on all paths including the previously-leaking error path.

PoC changes

Created two files: - blkmap_lock_trace.c — code-level analysis harness that prints the exact line ranges proving the lock-acquire-without-matching-release. - hammer_blkmap_trigger.c — runtime trigger that mounts a HAMMER fs, fills it, applies memory pressure, and checks for deadlock (confirms the error path is dead code at runtime).