[PATCH v4] hfs: port HFS+ b-tree bitmap corruption check

Aditya Prakash Srivastava posted 1 patch 1 week, 4 days ago
There is a newer version of this series
fs/hfs/bnode.c             | 42 -------------------
fs/hfs/btree.c             | 86 ++++++++++++++++++++++++++++++++++++++
fs/hfs/btree.h             | 40 ++++++++++++++++++
include/linux/hfs_common.h |  5 +++
4 files changed, 131 insertions(+), 42 deletions(-)
[PATCH v4] hfs: port HFS+ b-tree bitmap corruption check
Posted by Aditya Prakash Srivastava 1 week, 4 days ago
In HFS+ filesystems, during b-tree open (hfs_btree_open()), the code
verifies that the allocation map bit for the tree header (node 0) is
set. If not, it indicates a corrupted map record/bitmap and mounts
the volume as read-only (SB_RDONLY) to prevent further damage.

HFS filesystems share the same b-tree structure but currently lack
this corruption detection check.

Port this check to HFS, aligning its implementation with HFS+ to
maintain consistent b-tree logic across both filesystems:
1. Define struct hfs_bmap_ctx, and define HFS_TREE_HEAD and the relevant
   map record indices in include/linux/hfs_common.h.
2. Implement static hfs_bmap_get_map_page() in fs/hfs/btree.c, and port
   the necessary offset and length validation helpers to fs/hfs/btree.h
   as static inline functions with robust null pointer checks.
3. Implement static hfs_bmap_test_bit() in fs/hfs/btree.c to inspect
   the B-tree bitmap using the new get_map_page helper.
4. In hfs_btree_open(), retrieve the header node via hfs_bnode_find(),
   test its allocation bit with hfs_bmap_test_bit(), and release it
   using hfs_bnode_put().

Suggested-by: Viacheslav Dubeyko <slava@dubeyko.com>
Link: https://lore.kernel.org/all/6a36101b.be22b350.2a3e9.0001.GAE@google.com/T/#r446d0fed2a2900bd805534bbcb799d86619ae2ea
Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
---
 fs/hfs/bnode.c             | 42 -------------------
 fs/hfs/btree.c             | 86 ++++++++++++++++++++++++++++++++++++++
 fs/hfs/btree.h             | 40 ++++++++++++++++++
 include/linux/hfs_common.h |  5 +++
 4 files changed, 131 insertions(+), 42 deletions(-)

diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
index da8e5342c91c..1b331108d9c0 100644
--- a/fs/hfs/bnode.c
+++ b/fs/hfs/bnode.c
@@ -15,48 +15,6 @@
 
 #include "btree.h"
 
-static inline
-bool is_bnode_offset_valid(struct hfs_bnode *node, u32 off)
-{
-	bool is_valid = off < node->tree->node_size;
-
-	if (!is_valid) {
-		pr_err("requested invalid offset: "
-		       "NODE: id %u, type %#x, height %u, "
-		       "node_size %u, offset %u\n",
-		       node->this, node->type, node->height,
-		       node->tree->node_size, off);
-	}
-
-	return is_valid;
-}
-
-static inline
-u32 check_and_correct_requested_length(struct hfs_bnode *node, u32 off, u32 len)
-{
-	unsigned int node_size;
-
-	if (!is_bnode_offset_valid(node, off))
-		return 0;
-
-	node_size = node->tree->node_size;
-
-	if ((u64)off + len > node_size) {
-		u32 new_len = node_size - off;
-
-		pr_err("requested length has been corrected: "
-		       "NODE: id %u, type %#x, height %u, "
-		       "node_size %u, offset %u, "
-		       "requested_len %u, corrected_len %u\n",
-		       node->this, node->type, node->height,
-		       node->tree->node_size, off, len, new_len);
-
-		return new_len;
-	}
-
-	return len;
-}
-
 void hfs_bnode_read(struct hfs_bnode *node, void *buf, u32 off, u32 len)
 {
 	struct page *page;
diff --git a/fs/hfs/btree.c b/fs/hfs/btree.c
index 2eb37a2f64e8..2c841221c45c 100644
--- a/fs/hfs/btree.c
+++ b/fs/hfs/btree.c
@@ -15,6 +15,8 @@
 
 #include "btree.h"
 
+static bool hfs_bmap_test_bit(struct hfs_bnode *node, u32 node_bit_idx);
+
 /* Get a reference to a B*Tree and do some initial checks */
 struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id, btree_keycmp keycmp)
 {
@@ -23,6 +25,7 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id, btree_keycmp ke
 	struct address_space *mapping;
 	struct folio *folio;
 	struct buffer_head *bh;
+	struct hfs_bnode *node;
 	unsigned int size;
 	u16 dblock;
 	sector_t start_block;
@@ -155,6 +158,20 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id, btree_keycmp ke
 	kunmap_local(head);
 	folio_unlock(folio);
 	folio_put(folio);
+
+	node = hfs_bnode_find(tree, HFS_TREE_HEAD);
+	if (IS_ERR(node))
+		goto free_inode;
+
+	if (!hfs_bmap_test_bit(node, HFS_TREE_HEAD)) {
+		pr_warn("(%s): %s (cnid 0x%x) bitmap corrupted, forcing rdonly\n",
+			sb->s_id, id == HFS_EXT_CNID ? "extents" : "catalog", id);
+		pr_warn("Run fsck.hfs to repair.\n");
+		sb->s_flags |= SB_RDONLY;
+	}
+
+	hfs_bnode_put(node);
+
 	return tree;
 
 fail_folio:
@@ -356,6 +373,75 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
 	}
 }
 
+/* Context for iterating b-tree map pages
+ * @page_idx: The index of the page within the b-node's page array
+ * @off: The byte offset within the mapped page
+ * @len: The remaining length of the map record
+ */
+struct hfs_bmap_ctx {
+	unsigned int page_idx;
+	unsigned int off;
+	u16 len;
+};
+
+static struct page *hfs_bmap_get_map_page(struct hfs_bnode *node,
+					  struct hfs_bmap_ctx *ctx,
+					  u32 byte_offset)
+{
+	u16 rec_idx, off16;
+	unsigned int page_off;
+
+	if (node->this == HFS_TREE_HEAD) {
+		if (node->type != HFS_NODE_HEADER) {
+			pr_err("hfs: invalid btree header node\n");
+			return ERR_PTR(-EIO);
+		}
+		rec_idx = HFS_BTREE_HDR_MAP_REC_INDEX;
+	} else {
+		if (node->type != HFS_NODE_MAP) {
+			pr_err("hfs: invalid btree map node\n");
+			return ERR_PTR(-EIO);
+		}
+		rec_idx = HFS_BTREE_MAP_NODE_REC_INDEX;
+	}
+
+	ctx->len = hfs_brec_lenoff(node, rec_idx, &off16);
+	if (!ctx->len)
+		return ERR_PTR(-ENOENT);
+
+	if (!is_bnode_offset_valid(node, off16))
+		return ERR_PTR(-EIO);
+
+	ctx->len = check_and_correct_requested_length(node, off16, ctx->len);
+
+	if (byte_offset >= ctx->len)
+		return ERR_PTR(-EINVAL);
+
+	page_off = (u32)off16 + node->page_offset + byte_offset;
+	ctx->page_idx = page_off >> PAGE_SHIFT;
+	ctx->off = page_off & ~PAGE_MASK;
+
+	return node->page[ctx->page_idx];
+}
+
+static bool 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 false;
+
+	bmap = kmap_local_page(page);
+	byte = bmap[ctx.off];
+	kunmap_local(bmap);
+
+	mask = 1 << (7 - (node_bit_idx % BITS_PER_BYTE));
+	return (byte & mask) != 0;
+}
+
 void hfs_bmap_free(struct hfs_bnode *node)
 {
 	struct hfs_btree *tree;
diff --git a/fs/hfs/btree.h b/fs/hfs/btree.h
index 99be858b2446..f8afa33f709a 100644
--- a/fs/hfs/btree.h
+++ b/fs/hfs/btree.h
@@ -129,3 +129,43 @@ extern int __hfs_brec_find(struct hfs_bnode *bnode, struct hfs_find_data *fd);
 extern int hfs_brec_find(struct hfs_find_data *fd);
 extern int hfs_brec_read(struct hfs_find_data *fd, void *rec, u32 rec_len);
 extern int hfs_brec_goto(struct hfs_find_data *fd, int cnt);
+
+static inline bool is_bnode_offset_valid(struct hfs_bnode *node, u32 off)
+{
+	bool is_valid;
+
+	if (!node || !node->tree)
+		return false;
+
+	is_valid = off < node->tree->node_size;
+
+	if (!is_valid) {
+		pr_err("invalid offset: id %u, type %x, h %u, sz %u, off %u\n",
+		       node->this, node->type, node->height,
+		       node->tree->node_size, off);
+	}
+
+	return is_valid;
+}
+
+static inline u32 check_and_correct_requested_length(struct hfs_bnode *node, u32 off, u32 len)
+{
+	unsigned int node_size;
+
+	if (!is_bnode_offset_valid(node, off))
+		return 0;
+
+	node_size = node->tree->node_size;
+
+	if ((u64)off + len > node_size) {
+		u32 new_len = node_size - off;
+
+		pr_err("corrected len: id %u, type %x, h %u, sz %u, off %u, len %u->%u\n",
+		       node->this, node->type, node->height,
+		       node_size, off, len, new_len);
+
+		return new_len;
+	}
+
+	return len;
+}
diff --git a/include/linux/hfs_common.h b/include/linux/hfs_common.h
index 45fb4c9ff9f5..8dc10e0bfa7e 100644
--- a/include/linux/hfs_common.h
+++ b/include/linux/hfs_common.h
@@ -510,6 +510,11 @@ struct hfs_btree_header_rec {
 						   b-tree but not in extents
 						   b-tree (hfsplus). */
 
+/* HFS BTree misc info */
+#define HFS_TREE_HEAD				0
+#define HFS_BTREE_HDR_MAP_REC_INDEX		2	/* Map (bitmap) record in Header node */
+#define HFS_BTREE_MAP_NODE_REC_INDEX		0	/* Map record in Map Node */
+
 /* HFS+ BTree misc info */
 #define HFSPLUS_TREE_HEAD			0
 #define HFSPLUS_NODE_MXSZ			32768
-- 
2.47.3
Re: [PATCH v4] hfs: port HFS+ b-tree bitmap corruption check
Posted by Viacheslav Dubeyko 1 week, 3 days ago
On Tue, 2026-07-14 at 09:27 +0000, Aditya Prakash Srivastava wrote:
> In HFS+ filesystems, during b-tree open (hfs_btree_open()), the code
> verifies that the allocation map bit for the tree header (node 0) is
> set. If not, it indicates a corrupted map record/bitmap and mounts
> the volume as read-only (SB_RDONLY) to prevent further damage.
> 
> HFS filesystems share the same b-tree structure but currently lack
> this corruption detection check.
> 
> Port this check to HFS, aligning its implementation with HFS+ to
> maintain consistent b-tree logic across both filesystems:
> 1. Define struct hfs_bmap_ctx, and define HFS_TREE_HEAD and the
> relevant
>    map record indices in include/linux/hfs_common.h.
> 2. Implement static hfs_bmap_get_map_page() in fs/hfs/btree.c, and
> port
>    the necessary offset and length validation helpers to
> fs/hfs/btree.h
>    as static inline functions with robust null pointer checks.
> 3. Implement static hfs_bmap_test_bit() in fs/hfs/btree.c to inspect
>    the B-tree bitmap using the new get_map_page helper.
> 4. In hfs_btree_open(), retrieve the header node via
> hfs_bnode_find(),
>    test its allocation bit with hfs_bmap_test_bit(), and release it
>    using hfs_bnode_put().
> 
> Suggested-by: Viacheslav Dubeyko <slava@dubeyko.com>
> Link:
> https://lore.kernel.org/all/6a36101b.be22b350.2a3e9.0001.GAE@google.com/T/#r446d0fed2a2900bd805534bbcb799d86619ae2ea
> Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
> ---
>  fs/hfs/bnode.c             | 42 -------------------
>  fs/hfs/btree.c             | 86
> ++++++++++++++++++++++++++++++++++++++
>  fs/hfs/btree.h             | 40 ++++++++++++++++++
>  include/linux/hfs_common.h |  5 +++
>  4 files changed, 131 insertions(+), 42 deletions(-)
> 
> diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
> index da8e5342c91c..1b331108d9c0 100644
> --- a/fs/hfs/bnode.c
> +++ b/fs/hfs/bnode.c
> @@ -15,48 +15,6 @@
>  
>  #include "btree.h"
>  
> -static inline
> -bool is_bnode_offset_valid(struct hfs_bnode *node, u32 off)
> -{
> -	bool is_valid = off < node->tree->node_size;
> -
> -	if (!is_valid) {
> -		pr_err("requested invalid offset: "
> -		       "NODE: id %u, type %#x, height %u, "
> -		       "node_size %u, offset %u\n",
> -		       node->this, node->type, node->height,
> -		       node->tree->node_size, off);
> -	}
> -
> -	return is_valid;
> -}
> -
> -static inline
> -u32 check_and_correct_requested_length(struct hfs_bnode *node, u32
> off, u32 len)
> -{
> -	unsigned int node_size;
> -
> -	if (!is_bnode_offset_valid(node, off))
> -		return 0;
> -
> -	node_size = node->tree->node_size;
> -
> -	if ((u64)off + len > node_size) {
> -		u32 new_len = node_size - off;
> -
> -		pr_err("requested length has been corrected: "
> -		       "NODE: id %u, type %#x, height %u, "
> -		       "node_size %u, offset %u, "
> -		       "requested_len %u, corrected_len %u\n",
> -		       node->this, node->type, node->height,
> -		       node->tree->node_size, off, len, new_len);
> -
> -		return new_len;
> -	}
> -
> -	return len;
> -}
> -
>  void hfs_bnode_read(struct hfs_bnode *node, void *buf, u32 off, u32
> len)
>  {
>  	struct page *page;
> diff --git a/fs/hfs/btree.c b/fs/hfs/btree.c
> index 2eb37a2f64e8..2c841221c45c 100644
> --- a/fs/hfs/btree.c
> +++ b/fs/hfs/btree.c
> @@ -15,6 +15,8 @@
>  
>  #include "btree.h"
>  
> +static bool hfs_bmap_test_bit(struct hfs_bnode *node, u32
> node_bit_idx);

Logically, implementation should be here but not the forward
declaration. :) Could you please move the implementation?

> +
>  /* Get a reference to a B*Tree and do some initial checks */
>  struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id,
> btree_keycmp keycmp)
>  {
> @@ -23,6 +25,7 @@ struct hfs_btree *hfs_btree_open(struct super_block
> *sb, u32 id, btree_keycmp ke
>  	struct address_space *mapping;
>  	struct folio *folio;
>  	struct buffer_head *bh;
> +	struct hfs_bnode *node;
>  	unsigned int size;
>  	u16 dblock;
>  	sector_t start_block;
> @@ -155,6 +158,20 @@ struct hfs_btree *hfs_btree_open(struct
> super_block *sb, u32 id, btree_keycmp ke
>  	kunmap_local(head);
>  	folio_unlock(folio);
>  	folio_put(folio);
> +
> +	node = hfs_bnode_find(tree, HFS_TREE_HEAD);
> +	if (IS_ERR(node))
> +		goto free_inode;
> +
> +	if (!hfs_bmap_test_bit(node, HFS_TREE_HEAD)) {
> +		pr_warn("(%s): %s (cnid 0x%x) bitmap corrupted,
> forcing rdonly\n",
> +			sb->s_id, id == HFS_EXT_CNID ? "extents" :
> "catalog", id);
> +		pr_warn("Run fsck.hfs to repair.\n");
> +		sb->s_flags |= SB_RDONLY;
> +	}
> +
> +	hfs_bnode_put(node);
> +
>  	return tree;
>  
>  fail_folio:
> @@ -356,6 +373,75 @@ struct hfs_bnode *hfs_bmap_alloc(struct
> hfs_btree *tree)
>  	}
>  }
>  
> +/* Context for iterating b-tree map pages
> + * @page_idx: The index of the page within the b-node's page array
> + * @off: The byte offset within the mapped page
> + * @len: The remaining length of the map record
> + */
> +struct hfs_bmap_ctx {
> +	unsigned int page_idx;
> +	unsigned int off;
> +	u16 len;
> +};
> +
> +static struct page *hfs_bmap_get_map_page(struct hfs_bnode *node,
> +					  struct hfs_bmap_ctx *ctx,
> +					  u32 byte_offset)
> +{
> +	u16 rec_idx, off16;
> +	unsigned int page_off;
> +
> +	if (node->this == HFS_TREE_HEAD) {
> +		if (node->type != HFS_NODE_HEADER) {
> +			pr_err("hfs: invalid btree header node\n");
> +			return ERR_PTR(-EIO);
> +		}
> +		rec_idx = HFS_BTREE_HDR_MAP_REC_INDEX;
> +	} else {
> +		if (node->type != HFS_NODE_MAP) {
> +			pr_err("hfs: invalid btree map node\n");
> +			return ERR_PTR(-EIO);
> +		}
> +		rec_idx = HFS_BTREE_MAP_NODE_REC_INDEX;
> +	}
> +
> +	ctx->len = hfs_brec_lenoff(node, rec_idx, &off16);
> +	if (!ctx->len)
> +		return ERR_PTR(-ENOENT);
> +
> +	if (!is_bnode_offset_valid(node, off16))
> +		return ERR_PTR(-EIO);
> +
> +	ctx->len = check_and_correct_requested_length(node, off16,
> ctx->len);
> +
> +	if (byte_offset >= ctx->len)
> +		return ERR_PTR(-EINVAL);
> +
> +	page_off = (u32)off16 + node->page_offset + byte_offset;
> +	ctx->page_idx = page_off >> PAGE_SHIFT;
> +	ctx->off = page_off & ~PAGE_MASK;
> +
> +	return node->page[ctx->page_idx];
> +}
> +
> +static bool 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 false;
> +
> +	bmap = kmap_local_page(page);
> +	byte = bmap[ctx.off];
> +	kunmap_local(bmap);
> +
> +	mask = 1 << (7 - (node_bit_idx % BITS_PER_BYTE));
> +	return (byte & mask) != 0;
> +}
> +
>  void hfs_bmap_free(struct hfs_bnode *node)
>  {
>  	struct hfs_btree *tree;
> diff --git a/fs/hfs/btree.h b/fs/hfs/btree.h
> index 99be858b2446..f8afa33f709a 100644
> --- a/fs/hfs/btree.h
> +++ b/fs/hfs/btree.h
> @@ -129,3 +129,43 @@ extern int __hfs_brec_find(struct hfs_bnode
> *bnode, struct hfs_find_data *fd);
>  extern int hfs_brec_find(struct hfs_find_data *fd);
>  extern int hfs_brec_read(struct hfs_find_data *fd, void *rec, u32
> rec_len);
>  extern int hfs_brec_goto(struct hfs_find_data *fd, int cnt);
> +
> +static inline bool is_bnode_offset_valid(struct hfs_bnode *node, u32
> off)
> +{
> +	bool is_valid;
> +
> +	if (!node || !node->tree)
> +		return false;
> +
> +	is_valid = off < node->tree->node_size;
> +
> +	if (!is_valid) {
> +		pr_err("invalid offset: id %u, type %x, h %u, sz %u,
> off %u\n",
> +		       node->this, node->type, node->height,
> +		       node->tree->node_size, off);
> +	}
> +
> +	return is_valid;
> +}
> +
> +static inline u32 check_and_correct_requested_length(struct
> hfs_bnode *node, u32 off, u32 len)
> +{
> +	unsigned int node_size;
> +
> +	if (!is_bnode_offset_valid(node, off))
> +		return 0;
> +
> +	node_size = node->tree->node_size;
> +
> +	if ((u64)off + len > node_size) {
> +		u32 new_len = node_size - off;
> +
> +		pr_err("corrected len: id %u, type %x, h %u, sz %u,
> off %u, len %u->%u\n",
> +		       node->this, node->type, node->height,
> +		       node_size, off, len, new_len);
> +
> +		return new_len;
> +	}
> +
> +	return len;
> +}
> diff --git a/include/linux/hfs_common.h b/include/linux/hfs_common.h
> index 45fb4c9ff9f5..8dc10e0bfa7e 100644
> --- a/include/linux/hfs_common.h
> +++ b/include/linux/hfs_common.h
> @@ -510,6 +510,11 @@ struct hfs_btree_header_rec {
>  						   b-tree but not in
> extents
>  						   b-tree (hfsplus).
> */
>  
> +/* HFS BTree misc info */
> +#define HFS_TREE_HEAD				0
> +#define HFS_BTREE_HDR_MAP_REC_INDEX		2	/* Map
> (bitmap) record in Header node */
> +#define HFS_BTREE_MAP_NODE_REC_INDEX		0	/* Map
> record in Map Node */

Probably, we need to re-use HFS declarations for the case of HFS+. I
suggest this way for all three constants:

#define HFSPLUS_TREE_HEAD          HFS_TREE_HEAD

Thanks,
Slava.

> +
>  /* HFS+ BTree misc info */
>  #define HFSPLUS_TREE_HEAD			0
>  #define HFSPLUS_NODE_MXSZ			32768