Syzkaller reported an issue with corrupted HFS+ images where the b-tree
allocation bitmap indicates that the header node (Node 0) is free. Node 0
must always be allocated as it contains the b-tree header record and the
allocation bitmap itself. Violating this invariant leads to allocator
corruption, which cascades into kernel panics or undefined behavior when
the filesystem attempts to allocate blocks.

Prevent trusting a corrupted allocator state by adding a validation check
during hfs_btree_open(). Introduce the hfs_bmap_test_bit() helper
(utilizing the newly added map-access API) to safely verify that the MSB
of the first bitmap byte (representing Node 0) is marked as allocated.

If corruption is detected (either structurally invalid map records or an
illegally cleared bit), print a warning identifying the specific
corrupted tree and force the filesystem to mount read-only (SB_RDONLY).
This prevents kernel panics from corrupted images while enabling data
recovery.

Reported-by: syzbot+1c8ff72d0cd8a50dfeaa@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=1c8ff72d0cd8a50dfeaa
Link: https://lore.kernel.org/all/20260228122305.1406308-1-shardul.b@mpiricsoftware.com/
Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
---
 fs/hfsplus/btree.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
index 1c6a27e397fb..7c98b5858f99 100644
--- a/fs/hfsplus/btree.c
+++ b/fs/hfsplus/btree.c
@@ -185,6 +185,32 @@ static struct page *hfs_bmap_get_map_page(struct hfs_bnode *node, struct hfs_bma
 	return node->page[ctx->page_idx];
 }
 
+/**
+ * hfs_bmap_test_bit - test a bit in the b-tree map
+ * @node: the b-tree node containing the map record
+ * @node_bit_idx: the relative bit index within the node's map record
+ *
+ * Returns 1 if set, 0 if clear, or a negative error code on failure.
+ */
+static int hfs_bmap_test_bit(struct hfs_bnode *node, u32 node_bit_idx)
+{
+	struct hfs_bmap_ctx ctx;
+	struct page *page;
+	u8 *bmap, byte, mask;
+
+	page = hfs_bmap_get_map_page(node, &ctx, node_bit_idx / BITS_PER_BYTE);
+	if (IS_ERR(page))
+		return PTR_ERR(page);
+
+	bmap = kmap_local_page(page);
+	byte = bmap[ctx.off];
+	kunmap_local(bmap);
+
+	mask = 1 << (7 - (node_bit_idx % BITS_PER_BYTE));
+	return (byte & mask) ? 1 : 0;
+}
+
+
 /**
  * hfs_bmap_clear_bit - clear a bit in the b-tree map
  * @node: the b-tree node containing the map record
@@ -218,15 +244,36 @@ static int hfs_bmap_clear_bit(struct hfs_bnode *node, u32 node_bit_idx)
 	return 0;
 }
 
+#define HFS_EXTENT_TREE_NAME  "Extents"
+#define HFS_CATALOG_TREE_NAME "Catalog"
+#define HFS_ATTR_TREE_NAME    "Attributes"
+#define HFS_UNKNOWN_TREE_NAME "Unknown"
+
+static const char *hfs_btree_name(u32 cnid)
+{
+	switch (cnid) {
+	case HFSPLUS_EXT_CNID:
+		return HFS_EXTENT_TREE_NAME;
+	case HFSPLUS_CAT_CNID:
+		return HFS_CATALOG_TREE_NAME;
+	case HFSPLUS_ATTR_CNID:
+		return HFS_ATTR_TREE_NAME;
+	default:
+		return HFS_UNKNOWN_TREE_NAME;
+	}
+}
+
 /* Get a reference to a B*Tree and do some initial checks */
 struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
 {
 	struct hfs_btree *tree;
 	struct hfs_btree_header_rec *head;
 	struct address_space *mapping;
+	struct hfs_bnode *node;
 	struct inode *inode;
 	struct page *page;
 	unsigned int size;
+	int res;
 
 	tree = kzalloc_obj(*tree);
 	if (!tree)
@@ -331,6 +378,26 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
 
 	kunmap_local(head);
 	put_page(page);
+
+	node = hfs_bnode_find(tree, HFSPLUS_TREE_HEAD);
+	if (IS_ERR(node))
+		goto free_inode;
+
+	res = hfs_bmap_test_bit(node, 0);
+	if (res < 0) {
+		pr_warn("(%s): %s Btree (cnid 0x%x) map record invalid/corrupted, forcing read-only.\n",
+				sb->s_id, hfs_btree_name(id), id);
+		pr_warn("Run fsck.hfsplus to repair.\n");
+		sb->s_flags |= SB_RDONLY;
+	} else if (res == 0) {
+		pr_warn("(%s): %s Btree (cnid 0x%x) bitmap corruption detected, forcing read-only.\n",
+				sb->s_id, hfs_btree_name(id), id);
+		pr_warn("Run fsck.hfsplus to repair.\n");
+		sb->s_flags |= SB_RDONLY;
+	}
+
+	hfs_bnode_put(node);
+
 	return tree;
 
  fail_page:
-- 
2.34.1

On Sun, 2026-03-15 at 22:50 +0530, Shardul Bankar wrote:
> Syzkaller reported an issue with corrupted HFS+ images where the b-tree
> allocation bitmap indicates that the header node (Node 0) is free. Node 0
> must always be allocated as it contains the b-tree header record and the
> allocation bitmap itself. Violating this invariant leads to allocator
> corruption, which cascades into kernel panics or undefined behavior when
> the filesystem attempts to allocate blocks.
> 
> Prevent trusting a corrupted allocator state by adding a validation check
> during hfs_btree_open(). Introduce the hfs_bmap_test_bit() helper
> (utilizing the newly added map-access API) to safely verify that the MSB
> of the first bitmap byte (representing Node 0) is marked as allocated.
> 
> If corruption is detected (either structurally invalid map records or an
> illegally cleared bit), print a warning identifying the specific
> corrupted tree and force the filesystem to mount read-only (SB_RDONLY).
> This prevents kernel panics from corrupted images while enabling data
> recovery.
> 
> Reported-by: syzbot+1c8ff72d0cd8a50dfeaa@syzkaller.appspotmail.com
> Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__syzkaller.appspot.com_bug-3Fextid-3D1c8ff72d0cd8a50dfeaa&d=DwIDAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=7dlIxjaWFhoMAoS7ynWJQTY1_vifSFvOUWF3arXEWP24YytxvUupuv7_gWKWUUu1&s=ySMGzbg10Br2OVgCYK-CRCdGleeuQlfw4PenzGgbfsY&e= 
> Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_20260228122305.1406308-2D1-2Dshardul.b-40mpiricsoftware.com_&d=DwIDAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=7dlIxjaWFhoMAoS7ynWJQTY1_vifSFvOUWF3arXEWP24YytxvUupuv7_gWKWUUu1&s=Jz2TFxYLe-GFT6dlKhzUs44DoackTNwb0yGFtrUEU0Q&e= 
> Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
> ---
>  fs/hfsplus/btree.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
> index 1c6a27e397fb..7c98b5858f99 100644
> --- a/fs/hfsplus/btree.c
> +++ b/fs/hfsplus/btree.c
> @@ -185,6 +185,32 @@ static struct page *hfs_bmap_get_map_page(struct hfs_bnode *node, struct hfs_bma
>  	return node->page[ctx->page_idx];
>  }
>  
> +/**
> + * hfs_bmap_test_bit - test a bit in the b-tree map
> + * @node: the b-tree node containing the map record
> + * @node_bit_idx: the relative bit index within the node's map record
> + *
> + * Returns 1 if set, 0 if clear, or a negative error code on failure.
> + */
> +static int hfs_bmap_test_bit(struct hfs_bnode *node, u32 node_bit_idx)

Why not return bool data type?

> +{
> +	struct hfs_bmap_ctx ctx;
> +	struct page *page;
> +	u8 *bmap, byte, mask;
> +
> +	page = hfs_bmap_get_map_page(node, &ctx, node_bit_idx / BITS_PER_BYTE);
> +	if (IS_ERR(page))
> +		return PTR_ERR(page);

We can return false for the case of error.

> +
> +	bmap = kmap_local_page(page);
> +	byte = bmap[ctx.off];
> +	kunmap_local(bmap);
> +
> +	mask = 1 << (7 - (node_bit_idx % BITS_PER_BYTE));
> +	return (byte & mask) ? 1 : 0;

This is why I would like to see bool data type. :)

> +}
> +
> +
>  /**
>   * hfs_bmap_clear_bit - clear a bit in the b-tree map
>   * @node: the b-tree node containing the map record
> @@ -218,15 +244,36 @@ static int hfs_bmap_clear_bit(struct hfs_bnode *node, u32 node_bit_idx)
>  	return 0;
>  }
>  
> +#define HFS_EXTENT_TREE_NAME  "Extents"

Maybe we need to have Extents Overflow File (or B-tree), Catalog file,
Attributes file?

> +#define HFS_CATALOG_TREE_NAME "Catalog"
> +#define HFS_ATTR_TREE_NAME    "Attributes"
> +#define HFS_UNKNOWN_TREE_NAME "Unknown"
> +
> +static const char *hfs_btree_name(u32 cnid)
> +{
> +	switch (cnid) {
> +	case HFSPLUS_EXT_CNID:
> +		return HFS_EXTENT_TREE_NAME;
> +	case HFSPLUS_CAT_CNID:
> +		return HFS_CATALOG_TREE_NAME;
> +	case HFSPLUS_ATTR_CNID:
> +		return HFS_ATTR_TREE_NAME;
> +	default:
> +		return HFS_UNKNOWN_TREE_NAME;
> +	}
> +}
> +
>  /* Get a reference to a B*Tree and do some initial checks */
>  struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
>  {
>  	struct hfs_btree *tree;
>  	struct hfs_btree_header_rec *head;
>  	struct address_space *mapping;
> +	struct hfs_bnode *node;
>  	struct inode *inode;
>  	struct page *page;
>  	unsigned int size;
> +	int res;
>  
>  	tree = kzalloc_obj(*tree);
>  	if (!tree)
> @@ -331,6 +378,26 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
>  
>  	kunmap_local(head);
>  	put_page(page);
> +
> +	node = hfs_bnode_find(tree, HFSPLUS_TREE_HEAD);
> +	if (IS_ERR(node))
> +		goto free_inode;
> +
> +	res = hfs_bmap_test_bit(node, 0);

We definitely can return false for both cases.

Thanks,
Slava.

> +	if (res < 0) {
> +		pr_warn("(%s): %s Btree (cnid 0x%x) map record invalid/corrupted, forcing read-only.\n",
> +				sb->s_id, hfs_btree_name(id), id);
> +		pr_warn("Run fsck.hfsplus to repair.\n");
> +		sb->s_flags |= SB_RDONLY;
> +	} else if (res == 0) {
> +		pr_warn("(%s): %s Btree (cnid 0x%x) bitmap corruption detected, forcing read-only.\n",
> +				sb->s_id, hfs_btree_name(id), id);
> +		pr_warn("Run fsck.hfsplus to repair.\n");
> +		sb->s_flags |= SB_RDONLY;
> +	}
> +
> +	hfs_bnode_put(node);
> +
>  	return tree;
>  
>   fail_page:

On Mon, 2026-03-16 at 22:35 +0000, Viacheslav Dubeyko wrote:
> On Sun, 2026-03-15 at 22:50 +0530, Shardul Bankar wrote:
> > 
> > +/**
> > + * hfs_bmap_test_bit - test a bit in the b-tree map
> > + * @node: the b-tree node containing the map record
> > + * @node_bit_idx: the relative bit index within the node's map
> > record
> > + *
> > + * Returns 1 if set, 0 if clear, or a negative error code on
> > failure.
> > + */
> > +static int hfs_bmap_test_bit(struct hfs_bnode *node, u32
> > node_bit_idx)
> 
> Why not return bool data type?
> 
> > +{
> > +       struct hfs_bmap_ctx ctx;
> > +       struct page *page;
> > +       u8 *bmap, byte, mask;
> > +
> > +       page = hfs_bmap_get_map_page(node, &ctx, node_bit_idx /
> > BITS_PER_BYTE);
> > +       if (IS_ERR(page))
> > +               return PTR_ERR(page);
> 
> We can return false for the case of error.
> 
> > +
> > +       bmap = kmap_local_page(page);
> > +       byte = bmap[ctx.off];
> > +       kunmap_local(bmap);
> > +
> > +       mask = 1 << (7 - (node_bit_idx % BITS_PER_BYTE));
> > +       return (byte & mask) ? 1 : 0;
> 
> This is why I would like to see bool data type. :)
> 

I completely agree. Changing the return type to `bool` and returning
`false` on both an IO error and a cleared bit vastly simplifies the
caller. I will update `hfs_bmap_test_bit()` to return a `bool`, and I
will collapse the two `res < 0` and `res == 0` validation checks in
`hfs_btree_open()` into a single `if (!hfs_bmap_test_bit(node, 0))`
block in v7.

> > +}
> > +
> > +
> >  /**
> >   * hfs_bmap_clear_bit - clear a bit in the b-tree map
> >   * @node: the b-tree node containing the map record
> > @@ -218,15 +244,36 @@ static int hfs_bmap_clear_bit(struct
> > hfs_bnode *node, u32 node_bit_idx)
> >         return 0;
> >  }
> >  
> > +#define HFS_EXTENT_TREE_NAME  "Extents"
> 
> Maybe we need to have Extents Overflow File (or B-tree), Catalog
> file,
> Attributes file?
> 

Good point, those are the proper structural names. For v7, I will
update the macros to "Extents Overflow File", "Catalog File", and
"Attributes File", and I will slightly tweak the `pr_warn` format
string to accommodate the new names gracefully.

> > +#define HFS_CATALOG_TREE_NAME "Catalog"
> > +#define HFS_ATTR_TREE_NAME    "Attributes"
> > +#define HFS_UNKNOWN_TREE_NAME "Unknown"
> > +
> > +static const char *hfs_btree_name(u32 cnid)
> > +{
> > +       switch (cnid) {
> > +       case HFSPLUS_EXT_CNID:
> > +               return HFS_EXTENT_TREE_NAME;
> > +       case HFSPLUS_CAT_CNID:
> > +               return HFS_CATALOG_TREE_NAME;
> > +       case HFSPLUS_ATTR_CNID:
> > +               return HFS_ATTR_TREE_NAME;
> > +       default:
> > +               return HFS_UNKNOWN_TREE_NAME;
> > +       }
> > +}
> > +
> >  /* Get a reference to a B*Tree and do some initial checks */
> >  struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
> >  {
> >         struct hfs_btree *tree;
> >         struct hfs_btree_header_rec *head;
> >         struct address_space *mapping;
> > +       struct hfs_bnode *node;
> >         struct inode *inode;
> >         struct page *page;
> >         unsigned int size;
> > +       int res;
> >  
> >         tree = kzalloc_obj(*tree);
> >         if (!tree)
> > @@ -331,6 +378,26 @@ struct hfs_btree *hfs_btree_open(struct
> > super_block *sb, u32 id)
> >  
> >         kunmap_local(head);
> >         put_page(page);
> > +
> > +       node = hfs_bnode_find(tree, HFSPLUS_TREE_HEAD);
> > +       if (IS_ERR(node))
> > +               goto free_inode;
> > +
> > +       res = hfs_bmap_test_bit(node, 0);
> 
> We definitely can return false for both cases.
> 

Ack'ed

I will prepare the v7 patchset with these final stylistic polishings. 

Thanks for the guidance throughout this series!
Shardul