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 --- 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 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 --- 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