A hfsplus_check_fork() pass over a special file's eight fork extents, called from hfs_btree_open() for the extents, catalog and attributes trees: - block_count == 0 but start_block != 0: garbage left in a slot that should be blank (this is what the syzbot-reported image has in the extents overflow file's fork, slots 3 and 6); - start_block + block_count > sbi->total_blocks: an extent pointing past the end of the volume; - a non-zero extent following a zero one: a hole in the used range. If the first extent itself fails these checks, the b-tree's location on disk is unknown and there is nothing to recover, so hfs_btree_open() fails as it already does for the other structural checks in that function, and the mount fails. If only a later extent is affected, the tree can still be opened (its first extent, and hence its root node, is fine); mark it corrupt and let the caller decide. hfsplus_fill_super() forces the volume read-only in that case, and hfsplus_reconfigure() checks the same per-tree flag on remount instead of re-deriving it, refusing to go back to read-write. attr_tree may be NULL (volumes without an attributes fork), so both checks guard for that. This also gives the previous patch's hfsplus_file_extend() fix a mount-time backstop: a fuzzed or damaged extents overflow fork like the one in the syzbot report is caught here before any write ever reaches it. Reported-by: syzbot+f8ce6c197125ab9d72ce@syzkaller.appspotmail.com Signed-off-by: Nguyen Ngoc Thang Co-Authored-By: Claude Sonnet 5 --- fs/hfsplus/btree.c | 12 ++++++++++++ fs/hfsplus/extents.c | 37 +++++++++++++++++++++++++++++++++++++ fs/hfsplus/hfsplus_fs.h | 4 ++++ fs/hfsplus/super.c | 9 +++++++++ 4 files changed, 62 insertions(+) diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c index 2ea8cd5658e1..0a05ade53070 100644 --- a/fs/hfsplus/btree.c +++ b/fs/hfsplus/btree.c @@ -293,6 +293,18 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id) goto free_inode; } + switch (hfsplus_check_fork(sb, HFSPLUS_I(tree->inode)->first_extents)) { + case -EIO: + pr_err("%s (cnid 0x%x) fork's first extent is corrupt\n", + hfs_btree_name(id), id); + goto free_inode; + case 1: + pr_warn("%s (cnid 0x%x) fork has corrupt extents, forcing read-only.\n", + hfs_btree_name(id), id); + tree->corrupt = true; + break; + } + mapping = tree->inode->i_mapping; page = read_mapping_page(mapping, 0, NULL); if (IS_ERR(page)) diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c index 236f2d9a7a2d..a9303ce5bf8f 100644 --- a/fs/hfsplus/extents.c +++ b/fs/hfsplus/extents.c @@ -95,6 +95,43 @@ static bool hfsplus_ext_fork_full(struct hfsplus_extent *ext) return true; } +/* + * Check a fork's eight extents for the corruption a fuzzed or damaged + * volume header can contain: garbage in a slot that should be unused, + * an extent that runs past the end of the volume, or a used extent + * following an unused one. + * + * Returns 0 if the fork is fully consistent, 1 if only extents after + * the first are affected (the b-tree can still be located, so it's + * safe to mount read-only), or -EIO if the first extent itself is + * unusable. + */ +int hfsplus_check_fork(struct super_block *sb, struct hfsplus_extent *ext) +{ + struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb); + bool seen_hole = false; + int i; + + for (i = 0; i < 8; i++, ext++) { + u32 start = be32_to_cpu(ext->start_block); + u32 count = be32_to_cpu(ext->block_count); + bool bad; + + if (!count) { + bad = start != 0; + seen_hole = true; + } else { + bad = seen_hole || start + count < start || + start + count > sbi->total_blocks; + } + + if (bad) + return i ? 1 : -EIO; + } + + return 0; +} + static int __hfsplus_ext_write_extent(struct inode *inode, struct hfs_find_data *fd) { diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h index 1e5b58e6a13f..8d47219e67d3 100644 --- a/fs/hfsplus/hfsplus_fs.h +++ b/fs/hfsplus/hfsplus_fs.h @@ -56,6 +56,9 @@ struct hfs_btree { unsigned int max_key_len; unsigned int depth; + /* fork extents past the first were found corrupt at open time */ + bool corrupt; + struct mutex tree_lock; unsigned int pages_per_bnode; @@ -440,6 +443,7 @@ 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); /* inode.c */ extern const struct address_space_operations hfsplus_aops; diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c index ff7d6b3336a6..b65edb8ee589 100644 --- a/fs/hfsplus/super.c +++ b/fs/hfsplus/super.c @@ -400,6 +400,11 @@ static int hfsplus_reconfigure(struct fs_context *fc) pr_warn("filesystem is marked journaled, leaving read-only.\n"); sb->s_flags |= SB_RDONLY; fc->sb_flags |= SB_RDONLY; + } else if (sbi->ext_tree->corrupt || sbi->cat_tree->corrupt || + (sbi->attr_tree && sbi->attr_tree->corrupt)) { + pr_warn("a b-tree fork was corrupt at mount time, leaving read-only.\n"); + sb->s_flags |= SB_RDONLY; + fc->sb_flags |= SB_RDONLY; } } return 0; @@ -564,6 +569,10 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc) } sb->s_xattr = hfsplus_xattr_handlers; + if (sbi->ext_tree->corrupt || sbi->cat_tree->corrupt || + (sbi->attr_tree && sbi->attr_tree->corrupt)) + sb->s_flags |= SB_RDONLY; + inode = hfsplus_iget(sb, HFSPLUS_ALLOC_CNID); if (IS_ERR(inode)) { pr_err("failed to load allocation file\n"); -- 2.43.0