# 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).