โฌข DragonFlyBSD Kernel Audit
DF-0819 / run.log
โ† back to finding โ†“ download raw
=================================================================
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.
=================================================================