DF-0819 / run.log
=================================================================
DF-0819: blkmap_lock LEAK on hammer_bnew error paths
=================================================================
FILE: sys/vfs/hammer/hammer_blockmap.c
--- BUG 1: hammer_blockmap_alloc() [line 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 PAST UNLOCK
398: if (buffer1) hammer_rel_buffer(buffer1, 0);
...
405: return(result_offset);
PROOF: line 377 `goto failed` 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() [line 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 PAST UNLOCK
...
623: return(resv);
PROOF: line 606 `goto failed` targets label at line 613,
which is AFTER the unlock at line 611.
The lock acquired at line 546 is NEVER released on this path.
--- IMPACT ---
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() on
blkmap_lock. This is an unkillable D-state deadlock affecting the
entire mounted HAMMER filesystem. Impact: permanent DoS of the
filesystem; all processes touching it hang in D-state.
--- TRIGGER REACHABILITY ---
The error path requires hammer_bnew()/hammer_bnew_ext() to return
a non-zero *errorp. Tracing the call chain:
hammer_bnew_ext -> _hammer_bread -> hammer_get_buffer ->
hammer_load_buffer(isnew=1) -> hammer_io_new() -> return(0)
hammer_io_new() (hammer_io.c:437) ALWAYS returns 0 โ it calls
getblk() which either succeeds or panics (NULL deref at bp->b_ops).
So under normal operation the error path is dead code.
However, hammer_get_buffer() can set *errorp via hammer_get_volume()
(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.
CLASSIFICATION: latent bug โ real code defect, lock-leak pattern is
objectively present. Would deadlock the filesystem if the error path
is ever reached (corruption / I/O error / crafted image / future
code changes adding real error returns to hammer_io_new).
--- FIX ---
Add hammer_unlock(&hmp->blkmap_lock) before each `goto failed` that
occurs after the lock is acquired (lines 377 and 606). See fix.diff.
=================================================================