Re: hfsplus: possible recursive locking on extents_lock during unlink/truncate

[Viacheslav Dubeyko <slava@dubeyko.com>]

On Thu, 2026-10-01 at 02:58 +0400, Kullanılmıyor Pasif wrote:
> # hfsplus: lockdep "possible recursive locking" on extents_lock
> during unlink/truncate
> 
> **Reporter:** Noroxi (Mahmut Emin Kurhan <guvenlik@noroxi.com>)
> **Subsystem:** fs/hfsplus
> **Class:** lockdep false-positive (possible recursive locking) — CWE-
> 667-ish
> **Severity:** low — lockdep splat while mounting/using a crafted HFS+
> image. Analysis
> below indicates this is a missing lock-nesting annotation, NOT a real
> runtime
> deadlock (the two locks are different inodes of the same lock class,
> acquired in a
> fixed order). No memory corruption.
> **Status:** analysis from source; observed under syzkaller
> (kmemleak/lockdep) fuzzing
> of HFS+ image mounts. No standalone reproducer extracted yet.
> 
> ## lockdep report
> 
> ```
> WARNING: possible recursive locking detected
> syz.3.60/2437 is trying to acquire lock:
>   (&HFSPLUS_I(inode)->extents_lock){+.+.}, at: hfsplus_get_block+...
> fs/hfsplus/extents.c:264
> but task is already holding lock:
>   (&HFSPLUS_I(inode)->extents_lock){+.+.}, at:
> hfsplus_file_truncate+... fs/hfsplus/extents.c:579
>  May be due to missing lock nesting notation
> ```
> 
> ## Call chain (from the report stack)
> 
> ```
> hfsplus_unlink                 fs/hfsplus/dir.c:420
>  hfsplus_delete_inode          fs/hfsplus/inode.c:553
>   hfsplus_file_truncate        fs/hfsplus/extents.c:579   <--
> mutex_lock(inode A ->extents_lock)
>    hfsplus_free_extents        fs/hfsplus/extents.c:364
>     hfsplus_block_free         fs/hfsplus/bitmap.c:185     <-- reads
> the allocation bitmap file
>      block_read_full_folio     fs/buffer.c
>       hfsplus_get_block        fs/hfsplus/extents.c:264   <--
> mutex_lock(inode B ->extents_lock)
> ```
> 
> ## Analysis (why this is annotation, not a real deadlock)
> 
> - **inode A** = the regular file being unlinked/truncated.
> - **inode B** = the **allocation bitmap file** (`sbi->alloc_file`):
> `hfsplus_block_free()`
>   reads a page of the allocation file, which goes through `-
> >get_block` and locks the
>   allocation file's own `extents_lock`.
> - The two lock addresses in the report differ, so these are two
> *different* inode
>   instances of the *same* lock class (`extents_lock`). lockdep flags
> this as
>   "recursive" and itself notes "May be due to missing lock nesting
> notation".
> - The acquisition order is fixed: a regular file's `extents_lock` is
> taken first (in
>   truncate/extend), then the allocation file's `extents_lock` while
> freeing/allocating
>   blocks. There is no reverse path (the allocation file's block I/O
> does not take a
>   regular file's `extents_lock`), so no real ABBA/self-deadlock
> exists.
> 
> ## Suggested fix direction
> 
> hfsplus already solves the identical problem for the B-tree
> `tree_lock` via
> `hfsplus_btree_lock_class()` (fs/hfsplus/hfsplus_fs.h), which returns
> a distinct
> lockdep subclass per tree CNID and is used with
> `mutex_lock_nested()`.
> 
> The same pattern applied to `extents_lock` would resolve this: assign
> a distinct
> lockdep subclass to the special-inode `extents_lock` (at least the
> allocation file,
> and for completeness the attributes/catalog files) and take it with
> `mutex_lock_nested(&hip->extents_lock,
> hfsplus_extents_lock_class(inode))` at the five
> lock sites (extents.c:153,264,457,579 and xattr.c:267). Regular files
> keep the default
> subclass, so a genuine regular-file-vs-regular-file recursion would
> still be detected.
> 
> We can provide a candidate patch if useful; deferring the exact
> subclass scheme to the
> maintainers since the lock design is yours.
> 
> ## Disclosure
> Normal fs bug report (not a security embargo). To:
> linux-fsdevel@vger.kernel.org;
> Cc: Viacheslav Dubeyko, John Paul Adrian Glaubitz, Yangtao Li, linux-
> kernel.
> 
> ## Credit
> Found by Noroxi via coverage-guided fuzzing (syzkaller) of HFS+ image
> mounts.

Please, send the formal patch and we can start the regular review and
discussion.

Thanks,
Slava.