Re: [PATCH] hfsplus: fix recursive tree_lock in hfsplus_file_extend()

[Viacheslav Dubeyko]

On Tue, 2026-09-08 at 00:15 +0700, ThangNN99 wrote:
> Hi Slava,
> 
> Thanks for looking at this.
> 
> > Could you please explain the use-case or workload that is trying to
> > claim more blocks that fork can include for Extents Overflow file?
> 
> It is not a normal workload -- it requires a corrupted/adversarial
> on-disk volume, e.g. a loop-mounted image (removable media, a
> downloaded .img/.dmg, or a fuzzer). Per Apple's TN1150 ("HFS Plus
> Volume Format"):

OK. It sounds that you have corrupted fork in volume header for the
case of Extents Overflow file (and, maybe, for other metadata too).
Could you share the dump/content of the Extents Overflow file's fork?
Could we detect the corruption of the fork during the mount phase? If
we can then we need to mount in Read-Only mode the corrupted volume.

Thanks,
Slava.


> 
>   "The extents overflow file also stores additional extents for the
>   special files except for the extents overflow file itself."
> 
> So by design the extents overflow file must always be fully described
> by the eight extents in its own fork record; it can never
> legitimately
> need an overflow extent of its own. The syzbot reproducer mounts an
> image whose volume header sets the Extents File fork's total block
> count higher than what its eight direct extents describe, which puts
> hip->alloc_blocks != hip->first_blocks for HFSPLUS_EXT_CNID -- a
> state
> the volume header alone can force without the extents tree itself
> being touched. hfs_btree_open() doesn't currently validate this fork
> against the invariant above.
> 
> Once mounted, a plain pwritev2() to a regular file (call it FILE_A)
> that already has extents cached from a previous lookup is enough to
> hit it:
> 
> > Could you please share the call trace for the issue?
> 
>   pwritev2
>    -> hfsplus_get_block(FILE_A)
>    -> hfsplus_file_extend(FILE_A)
>    -> hfsplus_ext_read_extent(FILE_A) -> hfs_find_init(ext_tree)  
> [tree_lock acquired]
>    -> __hfsplus_ext_cache_extent(FILE_A): FILE_A's cached extent is
> dirty
>    -> __hfsplus_ext_write_extent(FILE_A): needs to insert a new
> record
>    -> hfs_bmap_reserve(ext_tree): ext_tree itself is out of free
> nodes
>    -> hfsplus_file_extend(ext_tree->inode)          <- now growing
> the tree's own file
>    -> hfsplus_ext_read_extent(ext_tree->inode) ->
> hfs_find_init(ext_tree)  [tree_lock again -> deadlock]
> 
> Full syzbot lockdep report for reference:
> 
> WARNING: possible recursive locking detected
> 6.16.0-rc7-syzkaller-00120-g5f33ebd2018c #0 Not tainted
> --------------------------------------------
> syz-executor310/5840 is trying to acquire lock:
> ffff88807fe920b0 (&tree->tree_lock/1){+.+.}-{4:4}, at:
> hfsplus_find_init+0x15a/0x1d0 fs/hfsplus/bfind.c:28
> 
> but task is already holding lock:
> ffff88807fe920b0 (&tree->tree_lock/1){+.+.}-{4:4}, at:
> hfsplus_find_init+0x15a/0x1d0 fs/hfsplus/bfind.c:28
> 
> 5 locks held by syz-executor310/5840:
>  #0: sb_writers#8, at: vfs_writev+0x288/0x960 fs/read_write.c:1055
>  #1: &sb->s_type->i_mutex_key#14, at:
> generic_file_write_iter+0xe3/0x540 mm/filemap.c:4252
>  #2: &hip->extents_lock, at: hfsplus_file_extend+0x1fc/0x1990
> fs/hfsplus/extents.c:458
>  #3: &tree->tree_lock/1, at: hfsplus_find_init+0x15a/0x1d0
> fs/hfsplus/bfind.c:28
>  #4: &HFSPLUS_I(inode)->extents_lock, at:
> hfsplus_file_extend+0x1fc/0x1990 fs/hfsplus/extents.c:458
> 
> Call Trace:
>  hfsplus_find_init fs/hfsplus/bfind.c:28
>  hfsplus_ext_read_extent fs/hfsplus/extents.c:216 [inline]
>  hfsplus_file_extend+0x416/0x1990 fs/hfsplus/extents.c:462
>  hfsplus_bmap_reserve+0x122/0x500 fs/hfsplus/btree.c:358
>  __hfsplus_ext_write_extent+0x28d/0x5b0 fs/hfsplus/extents.c:104
>  __hfsplus_ext_cache_extent+0x89/0xe30 fs/hfsplus/extents.c:186
>  hfsplus_ext_read_extent fs/hfsplus/extents.c:218 [inline]
>  hfsplus_file_extend+0x444/0x1990 fs/hfsplus/extents.c:462
>  hfsplus_get_block+0x411/0x1530 fs/hfsplus/extents.c:245
>  __block_write_begin_int+0x6b2/0x1900 fs/buffer.c:2151
>  block_write_begin fs/buffer.c:2262 [inline]
>  cont_write_begin+0x789/0xb50 fs/buffer.c:2601
>  hfsplus_write_begin+0x66/0xb0 fs/hfsplus/inode.c:46
>  generic_perform_write+0x2c4/0x910 mm/filemap.c:4112
>  generic_file_write_iter+0x10f/0x540 mm/filemap.c:4255
>  do_iter_readv_writev+0x56b/0x7f0 fs/read_write.c:-1
>  vfs_writev+0x31a/0x960 fs/read_write.c:1057
>  do_pwritev fs/read_write.c:1153 [inline]
>  __se_sys_pwritev2+0x179/0x290 fs/read_write.c:1202
> 
> (full report:
> https://syzkaller.appspot.com/text?tag=CrashReport&x=172748a2580000)
> 
> hfsplus_get_block() already refuses HFSPLUS_EXT_CNID for the lookup
> direction (extents.c:261: "if (inode->i_ino == HFSPLUS_EXT_CNID)
> return -EIO;"). This patch adds the same refusal on the grow path in
> hfsplus_file_extend(), which is the one hfs_bmap_reserve() can reach
> with tree_lock already held. It doesn't fix the underlying
> corruption,
> just stops it from self-deadlocking the tree_lock; a hfs_btree_open()
> check that rejects such a volume outright at mount time would close
> the hole earlier and I'm happy to send that as a follow-up if you'd
> rather validate it there instead.
> 
> I have not personally observed the crash on current mainline: I
> rebuilt the syzbot C reproducer, and hfs_btree_open(HFSPLUS_CAT_CNID)
> now rejects the image at mount (silently) -- it's a 2022-era fuzzed
> image and mainline has since gained catalog b-tree validation
> (node-size sanity check, record-offset table validation, etc.) that
> this image no longer passes. The analysis above is derived from
> source
> plus the syzbot-provided trace, not from a reproduced local crash.
> 
> Thanks,
> Thang