hfs_bmap_reserve() calls hfsplus_file_extend() on tree->inode with tree->tree_lock already held. For the extents overflow B-tree's own inode, growing it can call hfsplus_ext_read_extent() -> hfs_find_init() on that same tree, taking tree_lock a second time (lockdep: "possible recursive locking ... &tree->tree_lock/1"). This happens two ways: - the fork already claims more blocks than its eight extents describe (a corrupted on-disk fork), so hfsplus_ext_read_extent() is called immediately to look up the rest; or - the fork's eight extents get exhausted during this call, and inserting a new overflow extent record for the file would need the same lookup. Per the HFS+ format the extents overflow file is fully described by its eight fork extents and can never legitimately have overflow extents of its own, so both cases mean it cannot grow any further. Move the check into hfsplus_ext_read_extent() itself, the one place that actually re-enters hfs_find_init(), rather than duplicating it at each caller, and report -ENOSPC. For the second case, don't allocate blocks on the chance the fork still has room and undo it if not: hfsplus_ext_fork_full() tests the fork first. If it does have a free extent slot, any free space works, same as before. If it's already full, the only way to grow is a contiguous extension of the last extent, so only search for free space starting exactly at the block right after it, and fail with -ENOSPC immediately if that block isn't free -- nothing gets allocated in that case, so there's nothing to undo. The prior allocate-then-free-on-failure code stays at the insert_extent label as a backstop, in case this reasoning has a gap. Reported-by: syzbot+f8ce6c197125ab9d72ce@syzkaller.appspotmail.com Signed-off-by: Nguyen Ngoc Thang Co-Authored-By: Claude Sonnet 5 --- fs/hfsplus/extents.c | 59 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 4 deletions(-) diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c index eb7c11524d18..236f2d9a7a2d 100644 --- a/fs/hfsplus/extents.c +++ b/fs/hfsplus/extents.c @@ -84,6 +84,17 @@ static u32 hfsplus_ext_lastblock(struct hfsplus_extent *ext) return be32_to_cpu(ext->start_block) + be32_to_cpu(ext->block_count); } +/* True if all eight extents of a fork are in use (no free slot left) */ +static bool hfsplus_ext_fork_full(struct hfsplus_extent *ext) +{ + int i; + + for (i = 0; i < 8; ext++, i++) + if (!ext->block_count) + return false; + return true; +} + static int __hfsplus_ext_write_extent(struct inode *inode, struct hfs_find_data *fd) { @@ -217,6 +228,15 @@ static int hfsplus_ext_read_extent(struct inode *inode, u32 block) block < hip->cached_start + hip->cached_blocks) return 0; + /* + * The extents overflow file is fully described by its own fork + * extents; looking up an overflow extent for it would re-enter + * hfs_find_init() on the extents tree, whose tree_lock may already + * be held by the caller. + */ + if (inode->i_ino == HFSPLUS_EXT_CNID) + return -ENOSPC; + res = hfs_find_init(HFSPLUS_SB(inode->i_sb)->ext_tree, &fd); if (!res) { res = __hfsplus_ext_cache_extent(&fd, inode, block); @@ -465,13 +485,30 @@ int hfsplus_file_extend(struct inode *inode, bool zeroout) } len = hip->clump_blocks; - start = hfsplus_block_allocate(sb, sbi->total_blocks, goal, &len); - if (start >= sbi->total_blocks) { - start = hfsplus_block_allocate(sb, goal, 0, &len); - if (start >= goal) { + if (inode->i_ino == HFSPLUS_EXT_CNID && + hip->alloc_blocks == hip->first_blocks && + hfsplus_ext_fork_full(hip->first_extents)) { + /* + * No free slot is left in the fork, and the extents overflow + * file can't record an overflow extent of its own: the only + * way to grow it is a contiguous extension of the last + * extent, so only accept free space starting exactly at + * goal instead of allocating anywhere and having to undo it. + */ + start = hfsplus_block_allocate(sb, goal + 1, goal, &len); + if (start != goal) { res = -ENOSPC; goto out; } + } else { + start = hfsplus_block_allocate(sb, sbi->total_blocks, goal, &len); + if (start >= sbi->total_blocks) { + start = hfsplus_block_allocate(sb, goal, 0, &len); + if (start >= goal) { + res = -ENOSPC; + goto out; + } + } } if (zeroout) { @@ -526,6 +563,20 @@ int hfsplus_file_extend(struct inode *inode, bool zeroout) return res; insert_extent: + /* + * The fork-full precheck above keeps the extents overflow file's + * own inode from ever landing here with blocks already allocated; + * this is a backstop, so still free what was allocated rather + * than leak it. + */ + if (inode->i_ino == HFSPLUS_EXT_CNID) { + if (hfsplus_block_free(sb, start, len)) + pr_err("can't free extent: start %u, count %u\n", + start, len); + res = -ENOSPC; + goto out; + } + hfs_dbg("insert new extent\n"); res = hfsplus_ext_write_extent_locked(inode); if (res) -- 2.43.0