[PATCH v4 2/2] hfsplus: validate b-tree fork extents at mount time

[Nguyen Ngoc Thang]

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@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@xxxxxxxxx>
Co-Authored-By: Claude Sonnet 5 <noreply@xxxxxxxxxxxxx>
---
 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