[PATCH v4 0/2] hfsplus: validate btree bitmap during mount and handle corruption gracefully

[PATCH v4 0/2] hfsplus: validate btree bitmap during mount and handle corruption gracefully

[Shardul Bankar]

Hi,

syzbot 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 when the filesystem
attempts to allocate blocks.

This series prevents the kernel from trusting a corrupted state by
validating the Node 0 bitmap at mount time, allowing the filesystem to
safely fall back to read-only mode for data recovery.

Patch 1 is a preparatory cleanup. It extracts the map-record traversal
logic from hfs_bmap_alloc() into a generic helper. This deduplicates
the code, introduces strict node-type validation to prevent misinterpreting
corrupted nodes, and provides the abstraction needed for the mount-time check.

Patch 2 implements the actual Syzkaller fix. It uses the new helper during
hfs_btree_open() to verify that Node 0 is marked allocated. If it isn't, or
if the map record itself is structurally invalid, it forces the superblock
to SB_RDONLY.

Link: https://lore.kernel.org/all/54dc9336b514fb10547e27c7d6e1b8b967ee2eda.camel@ibm.com/

v4:
 - Split the changes into a 2-patch series (Refactoring + Bug Fix).
 - Extracted map node traversal into a generic helper (hfs_bmap_get_map_page)
   as per Slava's feedback, replacing manual offset/page management.
 - Added node-type validation (HFS_NODE_HEADER vs HFS_NODE_MAP) inside the
   helper to defend against structurally corrupted linkages.
 - Replaced hardcoded values with named macros (HFSPLUS_BTREE_NODE0_BIT, etc).
 - Handled invalid map offsets/lengths as corruption, continuing the mount
   as SB_RDONLY instead of failing it completely to preserve data recovery.

v3:
  - Moved validation logic inline into hfs_btree_open() to allow
    reporting the specific corrupted tree ID.
  - Replaced custom offset calculations with existing hfs_bnode_find()
    and hfs_brec_lenoff() infrastructure to handle node sizes and
    page boundaries correctly.
  - Removed temporary 'btree_bitmap_corrupted' superblock flag; setup
    SB_RDONLY directly upon detection.
  - Moved logging to hfs_btree_open() to include the specific tree ID in
    the warning message
  - Used explicit bitwise check (&) instead of test_bit() to ensure
    portability. test_bit() bit-numbering is architecture-dependent
    (e.g., bit 0 vs bit 7 can swap meanings on BE vs LE), whereas
    masking 0x80 consistently targets the MSB required by the HFS+
    on-disk format.
v2:
  - Fix compiler warning about comparing u16 bitmap_off with PAGE_SIZE which
can exceed u16 maximum on some architectures
  - Cast bitmap_off to unsigned int for the PAGE_SIZE comparison to avoid
tautological constant-out-of-range comparison warning.
  - Link: https://lore.kernel.org/oe-kbuild-all/202601251011.kJUhBF3P-lkp@intel.com/

Shardul Bankar (2):
  hfsplus: refactor b-tree map page access and add node-type validation
  hfsplus: validate b-tree node 0 bitmap at mount time

 fs/hfsplus/btree.c         | 123 ++++++++++++++++++++++++++++++-------
 include/linux/hfs_common.h |   3 +
 2 files changed, 104 insertions(+), 22 deletions(-)

-- 
2.34.1

[PATCH v4 1/2] hfsplus: refactor b-tree map page access and add node-type validation

[Shardul Bankar]

In HFS+ b-trees, the node allocation bitmap is stored across multiple
records. The first chunk resides in the b-tree Header Node at record
index 2, while all subsequent chunks are stored in dedicated Map Nodes
at record index 0.

This structural quirk forces callers like hfs_bmap_alloc() to duplicate
boilerplate code to validate offsets, correct lengths, and map the
underlying pages via kmap_local_page() for both the initial header node
and the subsequent map nodes in the chain.

Introduce a generic helper, hfs_bmap_get_map_page(), to encapsulate
the map record access. This helper:
1. Automatically validates the node->type against HFS_NODE_HEADER and
   HFS_NODE_MAP to prevent misinterpreting corrupted nodes.
2. Infers the correct record index (2 or 0) based on the node type.
3. Handles the offset calculation, length validation, and page mapping.

Refactor hfs_bmap_alloc() to utilize this helper, stripping out the
redundant setup blocks. As part of this cleanup, the double pointer
iterator (struct page **pagep) is replaced with a simpler unsigned int
index (page_idx) for cleaner page boundary crossing.

This deduplicates the allocator logic, hardens the map traversal against
fuzzed/corrupted images, and provides a generic map-access abstraction
that will be utilized by upcoming mount-time validation checks.

Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
---
 fs/hfsplus/btree.c         | 78 +++++++++++++++++++++++++++-----------
 include/linux/hfs_common.h |  3 ++
 2 files changed, 59 insertions(+), 22 deletions(-)

diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
index 1220a2f22737..22efd6517ef4 100644
--- a/fs/hfsplus/btree.c
+++ b/fs/hfsplus/btree.c
@@ -129,6 +129,47 @@ u32 hfsplus_calc_btree_clump_size(u32 block_size, u32 node_size,
 	return clump_size;
 }
 
+/*
+ * Maps the page containing the b-tree map record and calculates offsets.
+ * Automatically handles the difference between header and map nodes.
+ * Returns the mapped data pointer, or an ERR_PTR on failure.
+ * Note: The caller is responsible for calling kunmap_local(data).
+ */
+static u8 *hfs_bmap_get_map_page(struct hfs_bnode *node, u16 *off, u16 *len,
+				unsigned int *page_idx)
+{
+	u16 rec_idx, off16;
+
+	if (node->this == HFSPLUS_TREE_HEAD) {
+		if (node->type != HFS_NODE_HEADER) {
+			pr_err("hfsplus: invalid btree header node\n");
+			return ERR_PTR(-EIO);
+		}
+		rec_idx = HFSPLUS_BTREE_HDR_MAP_REC_INDEX;
+	} else {
+		if (node->type != HFS_NODE_MAP) {
+			pr_err("hfsplus: invalid btree map node\n");
+			return ERR_PTR(-EIO);
+		}
+		rec_idx = HFSPLUS_BTREE_MAP_NODE_REC_INDEX;
+	}
+
+	*len = hfs_brec_lenoff(node, rec_idx, &off16);
+	if (!*len)
+		return ERR_PTR(-ENOENT);
+
+	if (!is_bnode_offset_valid(node, off16))
+		return ERR_PTR(-EIO);
+
+	*len = check_and_correct_requested_length(node, off16, *len);
+
+	off16 += node->page_offset;
+	*page_idx = off16 >> PAGE_SHIFT;
+	*off = off16 & ~PAGE_MASK;
+
+	return kmap_local_page(node->page[*page_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)
 {
@@ -374,10 +415,9 @@ int hfs_bmap_reserve(struct hfs_btree *tree, u32 rsvd_nodes)
 struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
 {
 	struct hfs_bnode *node, *next_node;
-	struct page **pagep;
+	unsigned int page_idx;
 	u32 nidx, idx;
-	unsigned off;
-	u16 off16;
+	u16 off;
 	u16 len;
 	u8 *data, byte, m;
 	int i, res;
@@ -390,30 +430,24 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
 	node = hfs_bnode_find(tree, nidx);
 	if (IS_ERR(node))
 		return node;
-	len = hfs_brec_lenoff(node, 2, &off16);
-	off = off16;
-
-	if (!is_bnode_offset_valid(node, off)) {
+	data = hfs_bmap_get_map_page(node, &off, &len, &page_idx);
+	if (IS_ERR(data)) {
+		res = PTR_ERR(data);
 		hfs_bnode_put(node);
-		return ERR_PTR(-EIO);
+		return ERR_PTR(res);
 	}
-	len = check_and_correct_requested_length(node, off, len);
 
-	off += node->page_offset;
-	pagep = node->page + (off >> PAGE_SHIFT);
-	data = kmap_local_page(*pagep);
-	off &= ~PAGE_MASK;
 	idx = 0;
 
 	for (;;) {
 		while (len) {
 			byte = data[off];
 			if (byte != 0xff) {
-				for (m = 0x80, i = 0; i < 8; m >>= 1, i++) {
+				for (m = HFSPLUS_BTREE_NODE0_BIT, i = 0; i < 8; m >>= 1, i++) {
 					if (!(byte & m)) {
 						idx += i;
 						data[off] |= m;
-						set_page_dirty(*pagep);
+						set_page_dirty(node->page[page_idx]);
 						kunmap_local(data);
 						tree->free_nodes--;
 						mark_inode_dirty(tree->inode);
@@ -425,7 +459,7 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
 			}
 			if (++off >= PAGE_SIZE) {
 				kunmap_local(data);
-				data = kmap_local_page(*++pagep);
+				data = kmap_local_page(node->page[++page_idx]);
 				off = 0;
 			}
 			idx += 8;
@@ -443,12 +477,12 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
 			return next_node;
 		node = next_node;
 
-		len = hfs_brec_lenoff(node, 0, &off16);
-		off = off16;
-		off += node->page_offset;
-		pagep = node->page + (off >> PAGE_SHIFT);
-		data = kmap_local_page(*pagep);
-		off &= ~PAGE_MASK;
+		data = hfs_bmap_get_map_page(node, &off, &len, &page_idx);
+		if (IS_ERR(data)) {
+			res = PTR_ERR(data);
+			hfs_bnode_put(node);
+			return ERR_PTR(res);
+		}
 	}
 }
 
diff --git a/include/linux/hfs_common.h b/include/linux/hfs_common.h
index dadb5e0aa8a3..8238f55dd1d3 100644
--- a/include/linux/hfs_common.h
+++ b/include/linux/hfs_common.h
@@ -510,7 +510,10 @@ struct hfs_btree_header_rec {
 #define HFSPLUS_NODE_MXSZ			32768
 #define HFSPLUS_ATTR_TREE_NODE_SIZE		8192
 #define HFSPLUS_BTREE_HDR_NODE_RECS_COUNT	3
+#define HFSPLUS_BTREE_HDR_MAP_REC_INDEX		2	/* Map (bitmap) record in Header node */
+#define HFSPLUS_BTREE_MAP_NODE_REC_INDEX	0	/* Map record in Map Node */
 #define HFSPLUS_BTREE_HDR_USER_BYTES		128
+#define HFSPLUS_BTREE_NODE0_BIT			(1 << 7)
 
 /* btree key type */
 #define HFSPLUS_KEY_CASEFOLDING		0xCF	/* case-insensitive */
-- 
2.34.1

[PATCH v4 2/2] hfsplus: validate b-tree node 0 bitmap at mount time

[Shardul Bankar]

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 can cascade 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(). Using the newly introduced map-access helper,
verify that the MSB of the first bitmap byte (representing Node 0) is
marked as allocated. Additionally, catch any errors if the map record
itself is structurally invalid.

If corruption is detected, print a warning identifying the specific
corrupted tree (Extents, Catalog, or Attributes) and force the
filesystem to mount read-only (SB_RDONLY). This prevents kernel panics
from corrupted images while enabling data recovery by allowing the mount
to proceed in a safe, read-only mode rather than failing completely.

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

diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
index 22efd6517ef4..e34716cd661b 100644
--- a/fs/hfsplus/btree.c
+++ b/fs/hfsplus/btree.c
@@ -176,9 +176,14 @@ 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;
+	const char *tree_name;
+	unsigned int page_idx;
 	struct inode *inode;
 	struct page *page;
 	unsigned int size;
+	u16 bitmap_off, len;
+	u8 *map_page;
 
 	tree = kzalloc_obj(*tree);
 	if (!tree)
@@ -283,6 +288,46 @@ 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;
+
+	switch (id) {
+	case HFSPLUS_EXT_CNID:
+		tree_name = "Extents";
+		break;
+	case HFSPLUS_CAT_CNID:
+		tree_name = "Catalog";
+		break;
+	case HFSPLUS_ATTR_CNID:
+		tree_name = "Attributes";
+		break;
+	default:
+		tree_name = "Unknown";
+		break;
+	}
+
+	map_page = hfs_bmap_get_map_page(node, &bitmap_off, &len, &page_idx);
+
+	if (IS_ERR(map_page)) {
+		pr_warn("(%s): %s Btree (cnid 0x%x) map record invalid/corrupted, forcing read-only.\n",
+				sb->s_id, tree_name, id);
+		pr_warn("Run fsck.hfsplus to repair.\n");
+		sb->s_flags |= SB_RDONLY;
+		hfs_bnode_put(node);
+		return tree;
+	}
+
+	if (!(map_page[bitmap_off] & HFSPLUS_BTREE_NODE0_BIT)) {
+		pr_warn("(%s): %s Btree (cnid 0x%x) bitmap corruption detected, forcing read-only.\n",
+				sb->s_id, tree_name, id);
+		pr_warn("Run fsck.hfsplus to repair.\n");
+		sb->s_flags |= SB_RDONLY;
+	}
+	kunmap_local(map_page);
+	hfs_bnode_put(node);
+
 	return tree;
 
  fail_page:
-- 
2.34.1

Re: [PATCH v4 2/2] hfsplus: validate b-tree node 0 bitmap at mount time

[Viacheslav Dubeyko]

On Thu, 2026-02-26 at 14:42 +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 can cascade 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(). Using the newly introduced map-access helper,
> verify that the MSB of the first bitmap byte (representing Node 0) is
> marked as allocated. Additionally, catch any errors if the map record
> itself is structurally invalid.
> 
> If corruption is detected, print a warning identifying the specific
> corrupted tree (Extents, Catalog, or Attributes) and force the
> filesystem to mount read-only (SB_RDONLY). This prevents kernel panics
> from corrupted images while enabling data recovery by allowing the mount
> to proceed in a safe, read-only mode rather than failing completely.
> 
> 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=V6ouPNIE325L1Uhl0gL5R48hhzA9Rw4PSf7m7HPyLgNqtuBbiZmizg1krgnu0p47&s=83lgWM491j3f76U6MdBkt3ON-Qi37Pu4KURHxIkDlD0&e= 
> Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_54dc9336b514fb10547e27c7d6e1b8b967ee2eda.camel-40ibm.com_&d=DwIDAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=V6ouPNIE325L1Uhl0gL5R48hhzA9Rw4PSf7m7HPyLgNqtuBbiZmizg1krgnu0p47&s=CEFAhZ2iv4JVGNj2KbN19b0wzKEUx9kZsZIpMdGom8A&e= 
> Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
> ---
>  fs/hfsplus/btree.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
> index 22efd6517ef4..e34716cd661b 100644
> --- a/fs/hfsplus/btree.c
> +++ b/fs/hfsplus/btree.c
> @@ -176,9 +176,14 @@ 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;
> +	const char *tree_name;
> +	unsigned int page_idx;
>  	struct inode *inode;
>  	struct page *page;
>  	unsigned int size;
> +	u16 bitmap_off, len;
> +	u8 *map_page;
>  
>  	tree = kzalloc_obj(*tree);
>  	if (!tree)
> @@ -283,6 +288,46 @@ 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;
> +
> +	switch (id) {
> +	case HFSPLUS_EXT_CNID:
> +		tree_name = "Extents";
> +		break;
> +	case HFSPLUS_CAT_CNID:
> +		tree_name = "Catalog";
> +		break;
> +	case HFSPLUS_ATTR_CNID:
> +		tree_name = "Attributes";
> +		break;
> +	default:
> +		tree_name = "Unknown";
> +		break;
> +	}

Frankly speaking, it could be enough to share only cnid. But if you would like
to be really nice and to share the tree's name, then I prefer to see an array of
constant strings where you can use cnid as an index. And macro or static inline
method that can check cnid as a input argument. At minimum, simply move this
code into the static inline method. But, array of constant strings could be much
compact and elegant solution for my taste. Because, art of programming is to
represent everything as arrays of something and to apply the generalized loops.
:)

> +
> +	map_page = hfs_bmap_get_map_page(node, &bitmap_off, &len, &page_idx);

I will talk about bitmap_off, len, page_idx at the place of function definition.

You are safe in hfs_btree_open() method because nobody yet will try to access
the bitmap. But you need to use lock_page()/unlock_page() for other logic.

I prefer not to have the obligation of using this asynchronous paradigm of
kmap_local()/kunmap_local(). It will be great to keep this inside of
hfs_bmap_get_map_<something>() method.

I prefer not to keep the whole page/folio for complete operation locked. And,
frankly speaking, you don't need in the whole page because you need a byte or
unsigned long portion of bitmap. So, we can consider likewise interface:

u8 hfs_bmap_get_map_byte(struct hfs_bnode *node, u32 bit_index);

Here, you simply need to check the state of bit in byte (READ-ONLY operation).
So, you can atomically copy the state of the byte in local variable and to check
the bit state in local variable.

If you need to allocate and go change the state of the bit, then you need to use
lock for the whole operation:

<lock_bitmap>
hfs_bmap_get_map_byte();
<check and change byte state in local variable>
hfs_bmap_set_map_byte(); <-- set byte and make memory page dirty
<unlock_bitmap>

Thanks,
Slava.

> +
> +	if (IS_ERR(map_page)) {
> +		pr_warn("(%s): %s Btree (cnid 0x%x) map record invalid/corrupted, forcing read-only.\n",
> +				sb->s_id, tree_name, id);
> +		pr_warn("Run fsck.hfsplus to repair.\n");
> +		sb->s_flags |= SB_RDONLY;
> +		hfs_bnode_put(node);
> +		return tree;
> +	}
> +
> +	if (!(map_page[bitmap_off] & HFSPLUS_BTREE_NODE0_BIT)) {
> +		pr_warn("(%s): %s Btree (cnid 0x%x) bitmap corruption detected, forcing read-only.\n",
> +				sb->s_id, tree_name, id);
> +		pr_warn("Run fsck.hfsplus to repair.\n");
> +		sb->s_flags |= SB_RDONLY;
> +	}
> +	kunmap_local(map_page);
> +	hfs_bnode_put(node);
> +
>  	return tree;
>  
>   fail_page:

Re: [PATCH v4 1/2] hfsplus: refactor b-tree map page access and add node-type validation

[Viacheslav Dubeyko]

On Thu, 2026-02-26 at 14:42 +0530, Shardul Bankar wrote:
> In HFS+ b-trees, the node allocation bitmap is stored across multiple
> records. The first chunk resides in the b-tree Header Node at record
> index 2, while all subsequent chunks are stored in dedicated Map Nodes
> at record index 0.
> 
> This structural quirk forces callers like hfs_bmap_alloc() to duplicate
> boilerplate code to validate offsets, correct lengths, and map the
> underlying pages via kmap_local_page() for both the initial header node
> and the subsequent map nodes in the chain.
> 
> Introduce a generic helper, hfs_bmap_get_map_page(), to encapsulate
> the map record access. This helper:
> 1. Automatically validates the node->type against HFS_NODE_HEADER and
>    HFS_NODE_MAP to prevent misinterpreting corrupted nodes.
> 2. Infers the correct record index (2 or 0) based on the node type.
> 3. Handles the offset calculation, length validation, and page mapping.
> 
> Refactor hfs_bmap_alloc() to utilize this helper, stripping out the
> redundant setup blocks. As part of this cleanup, the double pointer
> iterator (struct page **pagep) is replaced with a simpler unsigned int
> index (page_idx) for cleaner page boundary crossing.
> 
> This deduplicates the allocator logic, hardens the map traversal against
> fuzzed/corrupted images, and provides a generic map-access abstraction
> that will be utilized by upcoming mount-time validation checks.
> 
> Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
> ---
>  fs/hfsplus/btree.c         | 78 +++++++++++++++++++++++++++-----------
>  include/linux/hfs_common.h |  3 ++
>  2 files changed, 59 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
> index 1220a2f22737..22efd6517ef4 100644
> --- a/fs/hfsplus/btree.c
> +++ b/fs/hfsplus/btree.c
> @@ -129,6 +129,47 @@ u32 hfsplus_calc_btree_clump_size(u32 block_size, u32 node_size,
>  	return clump_size;
>  }
>  
> +/*
> + * Maps the page containing the b-tree map record and calculates offsets.
> + * Automatically handles the difference between header and map nodes.
> + * Returns the mapped data pointer, or an ERR_PTR on failure.
> + * Note: The caller is responsible for calling kunmap_local(data).
> + */
> +static u8 *hfs_bmap_get_map_page(struct hfs_bnode *node, u16 *off, u16 *len,
> +				unsigned int *page_idx)

I think we don't need in off, len, page_idx arguments here. I suggest slightly
different interface:

u8 hfs_bmap_get_map_byte(struct hfs_bnode *node, u32 bit_index);
int hfs_bmap_set_map_byte(struct hfs_bnode *node, u32 bit_index, u8 byte);

In this case memory operations will be atomic ones and all
kmap_local()/kunmap_local() will be hidden inside these methods. However, I am
slightly worried that I don't see any locking mechanisms in hfs_bmap_alloc(). At
minimum, I believe we can use lock_page()/unlock_page() here. However, it will
be not enough. It is good for accessing only one page. But we need some lock for
the whole bitmap. Maybe, I am missing something. But if I am not, then we have a
huge room for race conditions in b-tree operations.

Probably, you could need something like hfs_bmap_get_map_page(). But you don't
need all of these arguments (off, len, page_idx) with suggested interface.
Because, bit_index should be enough to identify the proper memory page/folio and
get/set bytes there.

> +{
> +	u16 rec_idx, off16;
> +
> +	if (node->this == HFSPLUS_TREE_HEAD) {
> +		if (node->type != HFS_NODE_HEADER) {
> +			pr_err("hfsplus: invalid btree header node\n");
> +			return ERR_PTR(-EIO);
> +		}
> +		rec_idx = HFSPLUS_BTREE_HDR_MAP_REC_INDEX;
> +	} else {
> +		if (node->type != HFS_NODE_MAP) {
> +			pr_err("hfsplus: invalid btree map node\n");
> +			return ERR_PTR(-EIO);
> +		}
> +		rec_idx = HFSPLUS_BTREE_MAP_NODE_REC_INDEX;
> +	}
> +
> +	*len = hfs_brec_lenoff(node, rec_idx, &off16);
> +	if (!*len)
> +		return ERR_PTR(-ENOENT);
> +
> +	if (!is_bnode_offset_valid(node, off16))
> +		return ERR_PTR(-EIO);
> +
> +	*len = check_and_correct_requested_length(node, off16, *len);
> +
> +	off16 += node->page_offset;
> +	*page_idx = off16 >> PAGE_SHIFT;
> +	*off = off16 & ~PAGE_MASK;
> +
> +	return kmap_local_page(node->page[*page_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)
>  {
> @@ -374,10 +415,9 @@ int hfs_bmap_reserve(struct hfs_btree *tree, u32 rsvd_nodes)
>  struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
>  {
>  	struct hfs_bnode *node, *next_node;
> -	struct page **pagep;
> +	unsigned int page_idx;
>  	u32 nidx, idx;
> -	unsigned off;
> -	u16 off16;
> +	u16 off;
>  	u16 len;
>  	u8 *data, byte, m;
>  	int i, res;
> @@ -390,30 +430,24 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
>  	node = hfs_bnode_find(tree, nidx);
>  	if (IS_ERR(node))
>  		return node;
> -	len = hfs_brec_lenoff(node, 2, &off16);
> -	off = off16;
> -
> -	if (!is_bnode_offset_valid(node, off)) {
> +	data = hfs_bmap_get_map_page(node, &off, &len, &page_idx);
> +	if (IS_ERR(data)) {
> +		res = PTR_ERR(data);
>  		hfs_bnode_put(node);
> -		return ERR_PTR(-EIO);
> +		return ERR_PTR(res);
>  	}
> -	len = check_and_correct_requested_length(node, off, len);
>  
> -	off += node->page_offset;
> -	pagep = node->page + (off >> PAGE_SHIFT);
> -	data = kmap_local_page(*pagep);
> -	off &= ~PAGE_MASK;
>  	idx = 0;
>  
>  	for (;;) {
>  		while (len) {
>  			byte = data[off];
>  			if (byte != 0xff) {
> -				for (m = 0x80, i = 0; i < 8; m >>= 1, i++) {
> +				for (m = HFSPLUS_BTREE_NODE0_BIT, i = 0; i < 8; m >>= 1, i++) {

You are not right here. The 0x80 is simply pattern and it's not
HFSPLUS_BTREE_NODE0_BIT. Because, it could be any byte of the map.

Thanks,
Slava.

>  					if (!(byte & m)) {
>  						idx += i;
>  						data[off] |= m;
> -						set_page_dirty(*pagep);
> +						set_page_dirty(node->page[page_idx]);
>  						kunmap_local(data);
>  						tree->free_nodes--;
>  						mark_inode_dirty(tree->inode);
> @@ -425,7 +459,7 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
>  			}
>  			if (++off >= PAGE_SIZE) {
>  				kunmap_local(data);
> -				data = kmap_local_page(*++pagep);
> +				data = kmap_local_page(node->page[++page_idx]);
>  				off = 0;
>  			}
>  			idx += 8;
> @@ -443,12 +477,12 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
>  			return next_node;
>  		node = next_node;
>  
> -		len = hfs_brec_lenoff(node, 0, &off16);
> -		off = off16;
> -		off += node->page_offset;
> -		pagep = node->page + (off >> PAGE_SHIFT);
> -		data = kmap_local_page(*pagep);
> -		off &= ~PAGE_MASK;
> +		data = hfs_bmap_get_map_page(node, &off, &len, &page_idx);
> +		if (IS_ERR(data)) {
> +			res = PTR_ERR(data);
> +			hfs_bnode_put(node);
> +			return ERR_PTR(res);
> +		}
>  	}
>  }
>  
> diff --git a/include/linux/hfs_common.h b/include/linux/hfs_common.h
> index dadb5e0aa8a3..8238f55dd1d3 100644
> --- a/include/linux/hfs_common.h
> +++ b/include/linux/hfs_common.h
> @@ -510,7 +510,10 @@ struct hfs_btree_header_rec {
>  #define HFSPLUS_NODE_MXSZ			32768
>  #define HFSPLUS_ATTR_TREE_NODE_SIZE		8192
>  #define HFSPLUS_BTREE_HDR_NODE_RECS_COUNT	3
> +#define HFSPLUS_BTREE_HDR_MAP_REC_INDEX		2	/* Map (bitmap) record in Header node */
> +#define HFSPLUS_BTREE_MAP_NODE_REC_INDEX	0	/* Map record in Map Node */
>  #define HFSPLUS_BTREE_HDR_USER_BYTES		128
> +#define HFSPLUS_BTREE_NODE0_BIT			(1 << 7)
>  
>  /* btree key type */
>  #define HFSPLUS_KEY_CASEFOLDING		0xCF	/* case-insensitive */

Re: [PATCH v4 1/2] hfsplus: refactor b-tree map page access and add node-type validation

[Shardul Bankar]

On Thu, 2026-02-26 at 23:50 +0000, Viacheslav Dubeyko wrote:
> On Thu, 2026-02-26 at 14:42 +0530, Shardul Bankar wrote:
> > 
> > +/*
> > + * Maps the page containing the b-tree map record and calculates
> > offsets.
> > + * Automatically handles the difference between header and map
> > nodes.
> > + * Returns the mapped data pointer, or an ERR_PTR on failure.
> > + * Note: The caller is responsible for calling kunmap_local(data).
> > + */
> > +static u8 *hfs_bmap_get_map_page(struct hfs_bnode *node, u16 *off,
> > u16 *len,
> > +                               unsigned int *page_idx)
> 
> I think we don't need in off, len, page_idx arguments here. I suggest
> slightly
> different interface:
> 
> u8 hfs_bmap_get_map_byte(struct hfs_bnode *node, u32 bit_index);
> int hfs_bmap_set_map_byte(struct hfs_bnode *node, u32 bit_index, u8
> byte);
> 
> In this case memory operations will be atomic ones and all
> kmap_local()/kunmap_local() will be hidden inside these methods. 

Hi Slava,

Regarding the get_map_byte/set_map_byte interface: there would be a
severe performance regression if we force
kmap_local_page()/kunmap_local() on a per-byte basis inside the
hfs_bmap_alloc() linear scan. I am providing a detailed breakdown of
this overhead and a proposed alternative in my reply to your review on
Patch 2/2.


> However, I am
> slightly worried that I don't see any locking mechanisms in
> hfs_bmap_alloc(). At
> minimum, I believe we can use lock_page()/unlock_page() here.
> However, it will
> be not enough. It is good for accessing only one page. But we need
> some lock for
> the whole bitmap. Maybe, I am missing something. But if I am not,
> then we have a
> huge room for race conditions in b-tree operations.

Regarding the locking, concurrent access to the allocator is already
prevented by the per-tree tree->tree_lock mutex. Operations that reach
hfs_bmap_alloc() (e.g., node splits via hfs_brec_insert) are executed
within a search context initialized by hfs_find_init(), which holds
mutex_lock(&tree->tree_lock). Therefore, the map nodes are safely
serialized without needing individual lock_page() calls during the
scan.

> 
> >         for (;;) {
> >                 while (len) {
> >                         byte = data[off];
> >                         if (byte != 0xff) {
> > -                               for (m = 0x80, i = 0; i < 8; m >>=
> > 1, i++) {
> > +                               for (m = HFSPLUS_BTREE_NODE0_BIT, i
> > = 0; i < 8; m >>= 1, i++) {
> 
> You are not right here. The 0x80 is simply pattern and it's not
> HFSPLUS_BTREE_NODE0_BIT. Because, it could be any byte of the map.
> 

Ack'ed. Good catch, I will retain the original 0x80 pattern in the
allocation loop.

Thanks,
Shardul

Re: [PATCH v4 2/2] hfsplus: validate b-tree node 0 bitmap at mount time

[Shardul Bankar]

On Thu, 2026-02-26 at 23:29 +0000, Viacheslav Dubeyko wrote:
> On Thu, 2026-02-26 at 14:42 +0530, Shardul Bankar wrote:
> > +
> > +       switch (id) {
> > +       case HFSPLUS_EXT_CNID:
> > +               tree_name = "Extents";
> > +               break;
> > +       case HFSPLUS_CAT_CNID:
> > +               tree_name = "Catalog";
> > +               break;
> > +       case HFSPLUS_ATTR_CNID:
> > +               tree_name = "Attributes";
> > +               break;
> > +       default:
> > +               tree_name = "Unknown";
> > +               break;
> > +       }
> 
> Frankly speaking, it could be enough to share only cnid. But if you
> would like
> to be really nice and to share the tree's name, then I prefer to see
> an array of
> constant strings where you can use cnid as an index. And macro or
> static inline
> method that can check cnid as a input argument. At minimum, simply
> move this
> code into the static inline method. But, array of constant strings
> could be much
> compact and elegant solution for my taste. Because, art of
> programming is to
> represent everything as arrays of something and to apply the
> generalized loops.
> :)
> 

Hi Slava,

Sounds good. :) I will implement an array of constant strings indexed
by cnid in v5.

> I prefer not to have the obligation of using this asynchronous
> paradigm of
> kmap_local()/kunmap_local(). It will be great to keep this inside of
> hfs_bmap_get_map_<something>() method.
> 
> I prefer not to keep the whole page/folio for complete operation
> locked. And,
> frankly speaking, you don't need in the whole page because you need a
> byte or
> unsigned long portion of bitmap. So, we can consider likewise
> interface:
> 
> u8 hfs_bmap_get_map_byte(struct hfs_bnode *node, u32 bit_index);
> 
> Here, you simply need to check the state of bit in byte (READ-ONLY
> operation).
> So, you can atomically copy the state of the byte in local variable
> and to check
> the bit state in local variable.
> 

While this byte-level interface is perfect for the mount-time
validation in hfs_btree_open() where we only need to check a single
bit, using it inside hfs_bmap_alloc() introduces a significant
performance regression.

Because hfs_bmap_alloc() performs a linear scan to find a free node,
using hfs_bmap_get_map_byte() inside the while (len) loop would force
the kernel to execute kmap_local_page() and kunmap_local() for every
single byte evaluated (potentially thousands of times per page). The
current logic maps the page once, scans memory linearly, and only
unmaps when crossing a PAGE_SIZE boundary.

To address your request for a generalized map access method without
sacrificing the allocator's O(N) scanning performance, how about this
for v5?

    -We introduce the hfs_bmap_get_map_byte() specifically for single-
bit reads (like the mount-time check). This can internally call
hfs_bmap_get_map_page() from Patch 1/2 to avoid duplicating the offset
math.

    -We retain the page-level helper (hfs_bmap_get_map_page) for
hfs_bmap_alloc() to preserve its fast linear scanning.

Let me know if this dual-helper approach sounds acceptable, and I will
prepare v5.

Thanks,
Shardul

RE: [PATCH v4 2/2] hfsplus: validate b-tree node 0 bitmap at mount time

[Viacheslav Dubeyko]

On Fri, 2026-02-27 at 22:34 +0530, Shardul Bankar wrote:
> On Thu, 2026-02-26 at 23:29 +0000, Viacheslav Dubeyko wrote:
> > On Thu, 2026-02-26 at 14:42 +0530, Shardul Bankar wrote:
> > > +
> > > +       switch (id) {
> > > +       case HFSPLUS_EXT_CNID:
> > > +               tree_name = "Extents";
> > > +               break;
> > > +       case HFSPLUS_CAT_CNID:
> > > +               tree_name = "Catalog";
> > > +               break;
> > > +       case HFSPLUS_ATTR_CNID:
> > > +               tree_name = "Attributes";
> > > +               break;
> > > +       default:
> > > +               tree_name = "Unknown";
> > > +               break;
> > > +       }
> > 
> > Frankly speaking, it could be enough to share only cnid. But if you
> > would like
> > to be really nice and to share the tree's name, then I prefer to see
> > an array of
> > constant strings where you can use cnid as an index. And macro or
> > static inline
> > method that can check cnid as a input argument. At minimum, simply
> > move this
> > code into the static inline method. But, array of constant strings
> > could be much
> > compact and elegant solution for my taste. Because, art of
> > programming is to
> > represent everything as arrays of something and to apply the
> > generalized loops.
> > :)
> > 
> 
> Hi Slava,
> 
> Sounds good. :) I will implement an array of constant strings indexed
> by cnid in v5.
> 
> > I prefer not to have the obligation of using this asynchronous
> > paradigm of
> > kmap_local()/kunmap_local(). It will be great to keep this inside of
> > hfs_bmap_get_map_<something>() method.
> > 
> > I prefer not to keep the whole page/folio for complete operation
> > locked. And,
> > frankly speaking, you don't need in the whole page because you need a
> > byte or
> > unsigned long portion of bitmap. So, we can consider likewise
> > interface:
> > 
> > u8 hfs_bmap_get_map_byte(struct hfs_bnode *node, u32 bit_index);
> > 
> > Here, you simply need to check the state of bit in byte (READ-ONLY
> > operation).
> > So, you can atomically copy the state of the byte in local variable
> > and to check
> > the bit state in local variable.
> > 
> 
> While this byte-level interface is perfect for the mount-time
> validation in hfs_btree_open() where we only need to check a single
> bit, using it inside hfs_bmap_alloc() introduces a significant
> performance regression.
> 
> Because hfs_bmap_alloc() performs a linear scan to find a free node,
> using hfs_bmap_get_map_byte() inside the while (len) loop would force
> the kernel to execute kmap_local_page() and kunmap_local() for every
> single byte evaluated (potentially thousands of times per page). The
> current logic maps the page once, scans memory linearly, and only
> unmaps when crossing a PAGE_SIZE boundary.
> 
> To address your request for a generalized map access method without
> sacrificing the allocator's O(N) scanning performance, how about this
> for v5?
> 
>     -We introduce the hfs_bmap_get_map_byte() specifically for single-
> bit reads (like the mount-time check). This can internally call
> hfs_bmap_get_map_page() from Patch 1/2 to avoid duplicating the offset
> math.
> 
>     -We retain the page-level helper (hfs_bmap_get_map_page) for
> hfs_bmap_alloc() to preserve its fast linear scanning.
> 
> Let me know if this dual-helper approach sounds acceptable, and I will
> prepare v5.
> 
> 

I think your point makes sense. I missed this. However, we need to keep the
methods simple and understandable. First of all, if we need to return multiple
items from the method, then we definitely need some structure declarations that
can be used.

As far as I can see, we never had method for bit state check in the b-tree map
before. However, we have hfs_bmap_free() method that is one bit change
operation. So, we could have one bit check (hfs_bmap_test_bit()) and one bit
change (hfs_bmap_set_bit()) pair of methods that could hide all of these memory
pages operations.

However, hfs_bmap_alloc() is slightly special one. Probably, we could not make
significant changes in core logic of this method. However, your vision of
auxiliary method can be useful here. Yes, we need to execute kmap_local_page()
for the page, then do the search/allocation, and execute kunmap_local(). You are
right here. But, for my taste, the whole logic of linear search looks like not
very efficient. Do you see any ways of optimizations here? Could we employ tree-
>node_count? Or, maybe, introduce some in-core variable(s) that will keep
knowledge about last allocation/free? And we can use this knowledge to start
from the most beneficial region of search?

Thanks,
Slava.

Re: [PATCH v4 2/2] hfsplus: validate b-tree node 0 bitmap at mount time

[Shardul Bankar]

On Fri, 2026-02-27 at 20:11 +0000, Viacheslav Dubeyko wrote:
> On Fri, 2026-02-27 at 22:34 +0530, Shardul Bankar wrote:
> > On Thu, 2026-02-26 at 23:29 +0000, Viacheslav Dubeyko wrote:
> > > On Thu, 2026-02-26 at 14:42 +0530, Shardul Bankar wrote:
> > > 
> > > 
> > 
> > While this byte-level interface is perfect for the mount-time
> > validation in hfs_btree_open() where we only need to check a single
> > bit, using it inside hfs_bmap_alloc() introduces a significant
> > performance regression.
> > 
> > Because hfs_bmap_alloc() performs a linear scan to find a free
> > node,
> > using hfs_bmap_get_map_byte() inside the while (len) loop would
> > force
> > the kernel to execute kmap_local_page() and kunmap_local() for
> > every
> > single byte evaluated (potentially thousands of times per page).
> > The
> > current logic maps the page once, scans memory linearly, and only
> > unmaps when crossing a PAGE_SIZE boundary.
> > 
> > To address your request for a generalized map access method without
> > sacrificing the allocator's O(N) scanning performance, how about
> > this
> > for v5?
> > 
> >     -We introduce the hfs_bmap_get_map_byte() specifically for
> > single-
> > bit reads (like the mount-time check). This can internally call
> > hfs_bmap_get_map_page() from Patch 1/2 to avoid duplicating the
> > offset
> > math.
> > 
> >     -We retain the page-level helper (hfs_bmap_get_map_page) for
> > hfs_bmap_alloc() to preserve its fast linear scanning.
> > 
> > Let me know if this dual-helper approach sounds acceptable, and I
> > will
> > prepare v5.
> > 
> > 
> 
> I think your point makes sense. I missed this. However, we need to
> keep the
> methods simple and understandable. First of all, if we need to return
> multiple
> items from the method, then we definitely need some structure
> declarations that
> can be used.
> 

Agreed. To clean up the method signature for hfs_bmap_get_map_page(), I
will introduce a small structure (e.g., struct hfs_bmap_loc) to hold
the off, len, and page_idx variables instead of passing multiple
pointers.

> As far as I can see, we never had method for bit state check in the
> b-tree map
> before. However, we have hfs_bmap_free() method that is one bit
> change
> operation. So, we could have one bit check (hfs_bmap_test_bit()) and
> one bit
> change (hfs_bmap_set_bit()) pair of methods that could hide all of
> these memory
> pages operations.

This sounds like a good API improvement. I will introduce
hfs_bmap_test_bit() for the mount-time Node 0 check in v5. It can
internally call hfs_bmap_get_map_page() to avoid duplicating the offset
math, while safely encapsulating the kmap_local/kunmap_local for
single-bit reads.

> 
> However, hfs_bmap_alloc() is slightly special one. Probably, we could
> not make
> significant changes in core logic of this method. However, your
> vision of
> auxiliary method can be useful here. Yes, we need to execute
> kmap_local_page()
> for the page, then do the search/allocation, and execute
> kunmap_local(). You are
> right here. But, for my taste, the whole logic of linear search looks
> like not
> very efficient. Do you see any ways of optimizations here? Could we
> employ tree-
> > node_count? Or, maybe, introduce some in-core variable(s) that will
> > keep
> knowledge about last allocation/free? And we can use this knowledge
> to start
> from the most beneficial region of search?
> 

I like the idea of introducing an in-core allocation hint (a roving
pointer) to struct hfs_btree to convert this into a next-fit allocator,
and reusing the map-chain seek logic currently in hfs_bmap_free() to
jump directly to the beneficial region. Bounding the inner scan loop
with tree->node_count also seems like a good correctness optimization
to avoid scanning padding bytes.

However, the current patch series is targeted at the mount-time bitmap
corruption vulnerability. To keep the scope aligned, would it be
acceptable to finalize this current 2-patch series (the map access
refactoring + the Node 0 mount-time validation) in v5, and I will open
a separate thread/patchset afterward to pursue this alloc_hint and
node_count optimization?

Thanks,
Shardul

RE: [PATCH v4 2/2] hfsplus: validate b-tree node 0 bitmap at mount time

[Viacheslav Dubeyko]

On Sat, 2026-02-28 at 03:32 +0530, Shardul Bankar wrote:
> On Fri, 2026-02-27 at 20:11 +0000, Viacheslav Dubeyko wrote:
> > On Fri, 2026-02-27 at 22:34 +0530, Shardul Bankar wrote:
> > > On Thu, 2026-02-26 at 23:29 +0000, Viacheslav Dubeyko wrote:
> > > > On Thu, 2026-02-26 at 14:42 +0530, Shardul Bankar wrote:
> > > > 
> > > > 
> > > 
> > > While this byte-level interface is perfect for the mount-time
> > > validation in hfs_btree_open() where we only need to check a single
> > > bit, using it inside hfs_bmap_alloc() introduces a significant
> > > performance regression.
> > > 
> > > Because hfs_bmap_alloc() performs a linear scan to find a free
> > > node,
> > > using hfs_bmap_get_map_byte() inside the while (len) loop would
> > > force
> > > the kernel to execute kmap_local_page() and kunmap_local() for
> > > every
> > > single byte evaluated (potentially thousands of times per page).
> > > The
> > > current logic maps the page once, scans memory linearly, and only
> > > unmaps when crossing a PAGE_SIZE boundary.
> > > 
> > > To address your request for a generalized map access method without
> > > sacrificing the allocator's O(N) scanning performance, how about
> > > this
> > > for v5?
> > > 
> > >     -We introduce the hfs_bmap_get_map_byte() specifically for
> > > single-
> > > bit reads (like the mount-time check). This can internally call
> > > hfs_bmap_get_map_page() from Patch 1/2 to avoid duplicating the
> > > offset
> > > math.
> > > 
> > >     -We retain the page-level helper (hfs_bmap_get_map_page) for
> > > hfs_bmap_alloc() to preserve its fast linear scanning.
> > > 
> > > Let me know if this dual-helper approach sounds acceptable, and I
> > > will
> > > prepare v5.
> > > 
> > > 
> > 
> > I think your point makes sense. I missed this. However, we need to
> > keep the
> > methods simple and understandable. First of all, if we need to return
> > multiple
> > items from the method, then we definitely need some structure
> > declarations that
> > can be used.
> > 
> 
> Agreed. To clean up the method signature for hfs_bmap_get_map_page(), I
> will introduce a small structure (e.g., struct hfs_bmap_loc) to hold
> the off, len, and page_idx variables instead of passing multiple
> pointers.
> 
> > As far as I can see, we never had method for bit state check in the
> > b-tree map
> > before. However, we have hfs_bmap_free() method that is one bit
> > change
> > operation. So, we could have one bit check (hfs_bmap_test_bit()) and
> > one bit
> > change (hfs_bmap_set_bit()) pair of methods that could hide all of
> > these memory
> > pages operations.
> 
> This sounds like a good API improvement. I will introduce
> hfs_bmap_test_bit() for the mount-time Node 0 check in v5. It can
> internally call hfs_bmap_get_map_page() to avoid duplicating the offset
> math, while safely encapsulating the kmap_local/kunmap_local for
> single-bit reads.
> 
> > 
> > However, hfs_bmap_alloc() is slightly special one. Probably, we could
> > not make
> > significant changes in core logic of this method. However, your
> > vision of
> > auxiliary method can be useful here. Yes, we need to execute
> > kmap_local_page()
> > for the page, then do the search/allocation, and execute
> > kunmap_local(). You are
> > right here. But, for my taste, the whole logic of linear search looks
> > like not
> > very efficient. Do you see any ways of optimizations here? Could we
> > employ tree-
> > > node_count? Or, maybe, introduce some in-core variable(s) that will
> > > keep
> > knowledge about last allocation/free? And we can use this knowledge
> > to start
> > from the most beneficial region of search?
> > 
> 
> I like the idea of introducing an in-core allocation hint (a roving
> pointer) to struct hfs_btree to convert this into a next-fit allocator,
> and reusing the map-chain seek logic currently in hfs_bmap_free() to
> jump directly to the beneficial region. Bounding the inner scan loop
> with tree->node_count also seems like a good correctness optimization
> to avoid scanning padding bytes.
> 
> However, the current patch series is targeted at the mount-time bitmap
> corruption vulnerability. To keep the scope aligned, would it be
> acceptable to finalize this current 2-patch series (the map access
> refactoring + the Node 0 mount-time validation) in v5, and I will open
> a separate thread/patchset afterward to pursue this alloc_hint and
> node_count optimization?
> 

This is my point too. Let's finish this patch at first. Then, we can optimize
hfs_bmap_alloc(). Potentially, we can even consider of caching some portion of
b-tree's map for search and synchronization with map in memory pages.

Thanks,
Slava.