From nobody Thu Sep 24 15:10:57 2026 Received: from mail-pj2-f12.google.com (mail-pj2-f12.google.com [74.125.227.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 52FE45650EF for ; Tue, 22 Sep 2026 16:22:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.227.140 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790094127; cv=none; b=pkBRvxrfoD57Zu8i3q/TMtbupb5nQ4lCD5kZwo7DXPGpENnrJzsTWWHgLtoeGdqqiPw95H8HtI4AA320al5TqeuEWo1RTxmsJQepodYUGhW+M8Xt16F3Bz6V9FvzmP0BBBra6Tw//jRTQW7+Iti+kE2WE4WippGFBDZqr4e+TOo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790094127; c=relaxed/simple; bh=QPY/m7J0NLKJtwiEhBrXif7X5tOPvx88yF0XqLBp0Is=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=Xfom5BcZUf+AYTpslhc8flrg6XxHEaVecOajLeFOFfPpZQxUWMp38jYM7XkfvCoxC8HZE2G07x9TGtjbrLcwtcwq5FM4/zcSBxYhS5MxtIId2MtZ860ElzxQzbY8oTGJmP6djgXFOjgi8+eAEOgbJQkgzOX8C22R1+2dNAC6wrg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=Jl8fseOq; arc=none smtp.client-ip=74.125.227.140 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Jl8fseOq" Received: by mail-pj2-f12.google.com with SMTP id 98e67ed59e1d1-396ccd4f99cso107956a91.0 for ; Tue, 22 Sep 2026 09:22:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1790094123; x=1790698923; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to:content-type; bh=Zntf4umr1eS7Ny1yGhjgKRrHwdgFRxoOc6Ks5T0x52k=; b=Jl8fseOqKW/4NK9TTf5xbqHfR3/dAlP88GKQ/R90HsIyZk1P3mlDoqiamR9tASMXqR +0SJOxMbIhr9h1ttIP6IOS1jZM2OLH84aG/or/uoet6C8bbKyzZoPWXhnq30P+G3Jzd6 G+xtEiQY6Jrbd6pniaFqrh7XrwUY+K1YAYsAmDhQhJUa555dZIwqmk6Mwqra4zGEtf3m PwdthRZpZJK6VdkM1Trh7O6CPNj8N4N9m04NHqlYpkfdBZN9ByPGBeDMgXbIUN2FoP78 jAk5hqirknaGodGf4fCcI1rLzXmog5/MzonoMIYaC/AtmZAn6BUlCrMAdT0zLl4i5spL 5KKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1790094123; x=1790698923; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=Zntf4umr1eS7Ny1yGhjgKRrHwdgFRxoOc6Ks5T0x52k=; b=ei/fbzLG7T/qlZO5wk28XwDiW5hZHpEj174hso5h4aTl9hOPDZdx/Hlu+BYht5SdRi nzr2uj+2eq6P+d8GNPwWvjbzW+8xckQQIAdYsMa1G4emdugcafJqyXNMZplrtvsoC3E7 rOiJV1jWA+3KonETlplEh2Mk4zTgGTXi9alJO3HaEGet5QYZa8fBP8QEHTq51H9aH+HC O2lLqqXfMlpfo/Xs3THJcQnWNrGgq0qGpEBMYEUgVcmtwCqaeR0CzltCKHTpYrEYSN1h 4UwIt5eALUoxmwS8OWRzGnbjrDAeda5Ns0fgqBNp+Vtg5F/GxcpRmh9evx7/L7q46GIC Mwqg== X-Forwarded-Encrypted: i=1; AKwUvBwTcOKTT6lIBSUztO8FCAFIlTpckrmxtRenpxqQutkvgsyz0zrlE+ERQAdqdKOKlR0c7ygDlSmB6R2v71U=@vger.kernel.org X-Gm-Message-State: AFuF++k8zQkyK4HGzqDyD4q9QFh0JvRiEK/wH46Qjn6BkPDYOnGwzKim c98zJq6fCr45PqZwGzZx0jSWFwI+uoZ7zyhBd/XZ4mKVY2HcNIjt4z+l X-Gm-Gg: AYBFou3vjLWIq4QjVf2F6n8O5s5N3PIQhGn01Vncb2vcQGgrAtTlZh/DBwXkF+NZh+M WRR07OWutN8f7XzWd0gkF/6sC8H+1QM9AJDpbdiVB0P+X8LMP2hQjyO2JOR8ZJ4NEleC8SWRXRr N4G/t5OGD8SljyaGdr/Ct+Tkyng0NQm3irTPPxeYOcQDcN6tXcgvmjMevPPYDKZcI3RKL5/rRiO UeCyYskcOXs05gxV97GRIDLcOpfgSoRlb680He6nd/mYdpPA2UqEDc/uHrIXq9FqkNt3nRMIm/h OitlB3nFUiA20RI1p+pQjQzgJMDe/KqsjjRSXTKbPL3q+RMobX0dQ8jnxynqrVyq0PblEFlilG2 DmCem47schq9of4x+NdcXPnAEUgJ8UfN7MHXjAnKkZqxQr9dQ9FE8whoj90Tj7eLbllJKnVWJ75 Gonw73+cTihvZB3c3gulu/qZIpV3I/9dCEnKxdn1IQ986C7lmlxC5IuELMIiQdyafi1Iwu8n/FR ciqISE7xNOJfkoO X-Received: by 2002:a17:90b:4b44:b0:39e:14db:437c with SMTP id 98e67ed59e1d1-3a07e4f35e3mr2213a91.4.1790094123309; Tue, 22 Sep 2026 09:22:03 -0700 (PDT) Received: from thangnn-ASUS.. ([2405:4802:1d4a:e90:b024:bd81:d1dd:b7df]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-3a07ddae723sm128296a91.5.2026.09.22.09.22.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 22 Sep 2026 09:22:02 -0700 (PDT) From: Nguyen Ngoc Thang To: Viacheslav Dubeyko Cc: John Paul Adrian Glaubitz , Yangtao Li , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, syzbot+f8ce6c197125ab9d72ce@syzkaller.appspotmail.com, Nguyen Ngoc Thang , Claude Sonnet 5 Subject: [PATCH v7] hfsplus: fix recursive tree_lock and validate b-tree fork extents Date: Tue, 22 Sep 2026 23:21:57 +0700 Message-ID: <20260922162157.31987-1-ngocthang2710.1999@gmail.com> X-Mailer: git-send-email 2.43.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Fix a self-deadlock in the extents-overflow B-tree and validate b-tree fork extents at mount time to catch on-disk corruption early. hfs_bmap_reserve() can call hfsplus_file_extend() on tree->inode while tree->tree_lock is already held. When the tree is the extents overflow B-tree itself and its fork already claims more blocks than its eight direct extents describe, hfsplus_file_extend() calls hfsplus_ext_read_extent() -> hfs_find_init() on that same tree, taking tree_lock a second time. Per the HFS+ format the extents overflow file can never have overflow extents of its own, so this state only arises from a corrupted image; refuse to grow the file in that case instead of re-entering the lock. Separately, validate each system b-tree's fork (extents, total_blocks, total_size, clump_size) once at mount time in hfs_btree_open(), and mount read-only if a fork is corrupt but still usable, or fail the mount if the first extent itself is corrupt. Thanks to Viacheslav Dubeyko for the thorough v6 review that caught the missing lock fix and the other issues below. v7: - Restore the tree_lock fix that was mistakenly dropped in v6 (I rebased off the wrong base and lost it; sorry for the noise). - Move HFSPLUS_EXTENT_LAST_IDX into hfsplus_fs.h. - Move fork validation out of hfsplus_inode_read_fork() and into hfs_btree_open(), where the error is actually checked and acted on. - Only set SB_RDONLY from hfsplus_fill_super()/hfsplus_reconfigure(), never from the fork checker itself. - Also validate total_size against the fork's block count, and reject a clump_size larger than the volume. Reported-by: syzbot+f8ce6c197125ab9d72ce@syzkaller.appspotmail.com Signed-off-by: Nguyen Ngoc Thang Co-Authored-By: Claude Sonnet 5 --- fs/hfsplus/btree.c | 21 +++++++ fs/hfsplus/extents.c | 122 ++++++++++++++++++++++++++++++++++++++-- fs/hfsplus/hfsplus_fs.h | 10 ++++ fs/hfsplus/super.c | 13 +++++ 4 files changed, 160 insertions(+), 6 deletions(-) diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c index 2ea8cd5658e1..a07ca8a477f7 100644 --- a/fs/hfsplus/btree.c +++ b/fs/hfsplus/btree.c @@ -274,6 +274,7 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb= , u32 id) struct inode *inode; struct page *page; unsigned int size; + int res; =20 tree =3D kzalloc_obj(*tree); if (!tree) @@ -293,6 +294,26 @@ struct hfs_btree *hfs_btree_open(struct super_block *s= b, u32 id) goto free_inode; } =20 + res =3D hfsplus_check_fork(sb, HFSPLUS_I(tree->inode)->first_extents, + HFSPLUS_I(tree->inode)->alloc_blocks, + tree->inode->i_size); + if (res =3D=3D -EIO) { + pr_err("%s (cnid 0x%x) fork's first extent is corrupt\n", + hfs_btree_name(id), id); + goto free_inode; + } else if (res) { + pr_warn("%s (cnid 0x%x) fork has corrupt extents, forcing read-only.\n", + hfs_btree_name(id), id); + set_bit(HFSPLUS_I_CORRUPT_TREE, &HFSPLUS_I(tree->inode)->flags); + } else if (HFSPLUS_I(tree->inode)->clump_blocks > HFSPLUS_SB(sb)->total_b= locks) { + /* clump_size is only ever a growth hint, but a value this + * large can only come from a corrupt fork + */ + pr_warn("%s (cnid 0x%x) fork has bogus clump size, forcing read-only.\n", + hfs_btree_name(id), id); + set_bit(HFSPLUS_I_CORRUPT_TREE, &HFSPLUS_I(tree->inode)->flags); + } + mapping =3D tree->inode->i_mapping; page =3D read_mapping_page(mapping, 0, NULL); if (IS_ERR(page)) diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c index eb7c11524d18..925bbc3b36dc 100644 --- a/fs/hfsplus/extents.c +++ b/fs/hfsplus/extents.c @@ -77,13 +77,68 @@ static u32 hfsplus_ext_lastblock(struct hfsplus_extent = *ext) { int i; =20 - ext +=3D 7; - for (i =3D 0; i < 7; ext--, i++) + ext +=3D HFSPLUS_EXTENT_LAST_IDX; + for (i =3D 0; i < HFSPLUS_EXTENT_LAST_IDX; ext--, i++) if (ext->block_count) break; return be32_to_cpu(ext->start_block) + be32_to_cpu(ext->block_count); } =20 +/* True for the extents overflow file's own inode */ +static inline bool is_extents_btree(struct inode *inode) +{ + return inode->i_ino =3D=3D HFSPLUS_EXT_CNID; +} + +static bool hfsplus_extent_valid(struct hfsplus_extent *ext, u32 volume_bl= ocks) +{ + u32 start =3D be32_to_cpu(ext->start_block); + u32 count =3D be32_to_cpu(ext->block_count); + + if (!count) + return start =3D=3D 0; + + return start + count > start && start + count <=3D volume_blocks; +} + +/* 0 if the fork is consistent, -EUCLEAN if fixable, -EIO if unusable */ +int hfsplus_check_fork(struct super_block *sb, struct hfsplus_extent *ext, + u32 fork_blocks, u64 fork_size) +{ + struct hfsplus_sb_info *sbi =3D HFSPLUS_SB(sb); + bool seen_hole =3D false; + u32 used_blocks =3D 0; + loff_t max_size, min_size; + int i; + + for (i =3D 0; i < HFSPLUS_EXTENT_COUNT; i++, ext++) { + u32 count =3D be32_to_cpu(ext->block_count); + + if (!hfsplus_extent_valid(ext, sbi->total_blocks) || + (seen_hole && count)) + return i ? -EUCLEAN : -EIO; + + if (count) + used_blocks +=3D count; + else + seen_hole =3D true; + } + + if (!used_blocks) + return -EIO; + + if (used_blocks !=3D fork_blocks) + return -EUCLEAN; + + /* fork_size must need exactly fork_blocks allocation blocks */ + max_size =3D (loff_t)fork_blocks << sbi->alloc_blksz_shift; + min_size =3D max_size - (1 << sbi->alloc_blksz_shift); + if (fork_size <=3D min_size || fork_size > max_size) + return -EUCLEAN; + + return 0; +} + static int __hfsplus_ext_write_extent(struct inode *inode, struct hfs_find_data *fd) { @@ -217,6 +272,10 @@ static int hfsplus_ext_read_extent(struct inode *inode= , u32 block) block < hip->cached_start + hip->cached_blocks) return 0; =20 + /* The extents overflow file can't have overflow extents of its own */ + if (is_extents_btree(inode)) + return -ENOSPC; + res =3D hfs_find_init(HFSPLUS_SB(inode->i_sb)->ext_tree, &fd); if (!res) { res =3D __hfsplus_ext_cache_extent(&fd, inode, block); @@ -392,6 +451,35 @@ static int hfsplus_free_extents(struct super_block *sb, } } =20 +/* True when all 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 =3D 0; i < HFSPLUS_EXTENT_COUNT; ext++, i++) + if (!ext->block_count) + return false; + return true; +} + +/* True when the extents overflow file's fork has no free slot left */ +static bool hfsplus_ext_file_needs_contig_grow(struct inode *inode, + struct hfsplus_inode_info *hip) +{ + return is_extents_btree(inode) && + hip->alloc_blocks =3D=3D hip->first_blocks && + hfsplus_ext_fork_full(hip->first_extents); +} + +/* + * Fork is full: only a contiguous extension of the last extent works. + * Search for up to *len free blocks starting exactly at goal. + */ +static u32 hfsplus_ext_file_grow(struct super_block *sb, u32 goal, u32 *le= n) +{ + return hfsplus_block_allocate(sb, goal + *len, goal, len); +} + int hfsplus_free_fork(struct super_block *sb, u32 cnid, struct hfsplus_fork_raw *fork, int type) { @@ -458,6 +546,11 @@ int hfsplus_file_extend(struct inode *inode, bool zero= out) if (hip->alloc_blocks =3D=3D hip->first_blocks) goal =3D hfsplus_ext_lastblock(hip->first_extents); else { + /* Would re-enter ext_tree->tree_lock; corrupt fork */ + if (is_extents_btree(inode)) { + res =3D -EIO; + goto out; + } res =3D hfsplus_ext_read_extent(inode, hip->alloc_blocks); if (res) goto out; @@ -465,13 +558,21 @@ int hfsplus_file_extend(struct inode *inode, bool zer= oout) } =20 len =3D hip->clump_blocks; - start =3D hfsplus_block_allocate(sb, sbi->total_blocks, goal, &len); - if (start >=3D sbi->total_blocks) { - start =3D hfsplus_block_allocate(sb, goal, 0, &len); - if (start >=3D goal) { + if (hfsplus_ext_file_needs_contig_grow(inode, hip)) { + start =3D hfsplus_ext_file_grow(sb, goal, &len); + if (start !=3D goal) { res =3D -ENOSPC; goto out; } + } else { + start =3D hfsplus_block_allocate(sb, sbi->total_blocks, goal, &len); + if (start >=3D sbi->total_blocks) { + start =3D hfsplus_block_allocate(sb, goal, 0, &len); + if (start >=3D goal) { + res =3D -ENOSPC; + goto out; + } + } } =20 if (zeroout) { @@ -526,6 +627,15 @@ int hfsplus_file_extend(struct inode *inode, bool zero= out) return res; =20 insert_extent: + /* Can't happen: the fork-full branch above rules this out */ + if (WARN_ON_ONCE(is_extents_btree(inode))) { + if (hfsplus_block_free(sb, start, len)) + pr_err("can't free extent: start %u, count %u\n", + start, len); + res =3D -ENOSPC; + goto out; + } + hfs_dbg("insert new extent\n"); res =3D hfsplus_ext_write_extent_locked(inode); if (res) diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h index 1e5b58e6a13f..df636f2dbf1d 100644 --- a/fs/hfsplus/hfsplus_fs.h +++ b/fs/hfsplus/hfsplus_fs.h @@ -222,15 +222,23 @@ struct hfsplus_inode_info { #define HFSPLUS_EXT_DIRTY 0x0001 #define HFSPLUS_EXT_NEW 0x0002 =20 +/* Number of extent slots in a fork's hfsplus_extent_rec (hfs_common.h) */ +#define HFSPLUS_EXTENT_COUNT 8 +#define HFSPLUS_EXTENT_LAST_IDX (HFSPLUS_EXTENT_COUNT - 1) + #define HFSPLUS_I_RSRC 0 /* represents a resource fork */ #define HFSPLUS_I_CAT_DIRTY 1 /* has changes in the catalog tree */ #define HFSPLUS_I_EXT_DIRTY 2 /* has changes in the extent tree */ #define HFSPLUS_I_ALLOC_DIRTY 3 /* has changes in the allocation file */ #define HFSPLUS_I_ATTR_DIRTY 4 /* has changes in the attributes tree */ +#define HFSPLUS_I_CORRUPT_TREE 5 /* tree's fork had corrupt extents at ope= n time */ =20 #define HFSPLUS_IS_RSRC(inode) \ test_bit(HFSPLUS_I_RSRC, &HFSPLUS_I(inode)->flags) =20 +#define HFSPLUS_TREE_IS_CORRUPT(tree) \ + test_bit(HFSPLUS_I_CORRUPT_TREE, &HFSPLUS_I((tree)->inode)->flags) + static inline struct hfsplus_inode_info *HFSPLUS_I(struct inode *inode) { return container_of(inode, struct hfsplus_inode_info, vfs_inode); @@ -440,6 +448,8 @@ int hfsplus_free_fork(struct super_block *sb, u32 cnid, struct hfsplus_fork_raw *fork, int type); int hfsplus_file_extend(struct inode *inode, bool zeroout); void hfsplus_file_truncate(struct inode *inode); +int hfsplus_check_fork(struct super_block *sb, struct hfsplus_extent *ext, + u32 fork_blocks, u64 fork_size); =20 /* inode.c */ extern const struct address_space_operations hfsplus_aops; diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c index ff7d6b3336a6..b6a85c153cf3 100644 --- a/fs/hfsplus/super.c +++ b/fs/hfsplus/super.c @@ -400,6 +400,14 @@ static int hfsplus_reconfigure(struct fs_context *fc) pr_warn("filesystem is marked journaled, leaving read-only.\n"); sb->s_flags |=3D SB_RDONLY; fc->sb_flags |=3D SB_RDONLY; + } else if (HFSPLUS_TREE_IS_CORRUPT(sbi->ext_tree) || + HFSPLUS_TREE_IS_CORRUPT(sbi->cat_tree) || + (sbi->attr_tree && + HFSPLUS_TREE_IS_CORRUPT(sbi->attr_tree))) { + /* Corruption is only ever detected at mount, in hfs_btree_open() */ + pr_warn("a b-tree fork was corrupt at mount time, leaving read-only.\n"= ); + sb->s_flags |=3D SB_RDONLY; + fc->sb_flags |=3D SB_RDONLY; } } return 0; @@ -564,6 +572,11 @@ static int hfsplus_fill_super(struct super_block *sb, = struct fs_context *fc) } sb->s_xattr =3D hfsplus_xattr_handlers; =20 + if (HFSPLUS_TREE_IS_CORRUPT(sbi->ext_tree) || + HFSPLUS_TREE_IS_CORRUPT(sbi->cat_tree) || + (sbi->attr_tree && HFSPLUS_TREE_IS_CORRUPT(sbi->attr_tree))) + sb->s_flags |=3D SB_RDONLY; + inode =3D hfsplus_iget(sb, HFSPLUS_ALLOC_CNID); if (IS_ERR(inode)) { pr_err("failed to load allocation file\n"); --=20 2.43.0