Skip to content

Commit a8eed0b

Browse files
shardulsdk-mpiricdubeyko
authored andcommitted
hfsplus: refactor b-tree map page access and add node-type validation
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() and hfs_bmap_free() to duplicate boilerplate code to validate offsets, correct lengths, and map the underlying pages via kmap_local_page(). There is also currently no strict node-type validation before reading these records, leaving the allocator vulnerable if a corrupted image points a map linkage to an Index or Leaf node. Introduce a unified bit-level API to encapsulate the map record access: 1. A new `struct hfs_bmap_ctx` to cleanly pass state and safely handle page math across all architectures. 2. `hfs_bmap_get_map_page()`: Automatically validates node types (HFS_NODE_HEADER vs HFS_NODE_MAP), infers the correct record index, handles page-boundary math, and returns the unmapped `struct page *` directly to the caller to avoid asymmetric mappings. 3. `hfs_bmap_clear_bit()`: A clean wrapper that internally handles page mapping/unmapping for single-bit operations. Refactor hfs_bmap_alloc() and hfs_bmap_free() to utilize this new API. This deduplicates the allocator logic, hardens the map traversal against fuzzed images, and provides the exact abstractions needed for upcoming mount-time validation checks. Signed-off-by: Shardul Bankar <[email protected]> Reviewed-by: Viacheslav Dubeyko <[email protected]> Tested-by: Viacheslav Dubeyko <[email protected]> Signed-off-by: Viacheslav Dubeyko <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Viacheslav Dubeyko <[email protected]>
1 parent b099ed5 commit a8eed0b

2 files changed

Lines changed: 124 additions & 47 deletions

File tree

fs/hfsplus/btree.c

Lines changed: 122 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,95 @@ u32 hfsplus_calc_btree_clump_size(u32 block_size, u32 node_size,
129129
return clump_size;
130130
}
131131

132+
/* Context for iterating b-tree map pages
133+
* @page_idx: The index of the page within the b-node's page array
134+
* @off: The byte offset within the mapped page
135+
* @len: The remaining length of the map record
136+
*/
137+
struct hfs_bmap_ctx {
138+
unsigned int page_idx;
139+
unsigned int off;
140+
u16 len;
141+
};
142+
143+
/*
144+
* Finds the specific page containing the requested byte offset within the map
145+
* record. Automatically handles the difference between header and map nodes.
146+
* Returns the struct page pointer, or an ERR_PTR on failure.
147+
* Note: The caller is responsible for mapping/unmapping the returned page.
148+
*/
149+
static struct page *hfs_bmap_get_map_page(struct hfs_bnode *node, struct hfs_bmap_ctx *ctx,
150+
u32 byte_offset)
151+
{
152+
u16 rec_idx, off16;
153+
unsigned int page_off;
154+
155+
if (node->this == HFSPLUS_TREE_HEAD) {
156+
if (node->type != HFS_NODE_HEADER) {
157+
pr_err("hfsplus: invalid btree header node\n");
158+
return ERR_PTR(-EIO);
159+
}
160+
rec_idx = HFSPLUS_BTREE_HDR_MAP_REC_INDEX;
161+
} else {
162+
if (node->type != HFS_NODE_MAP) {
163+
pr_err("hfsplus: invalid btree map node\n");
164+
return ERR_PTR(-EIO);
165+
}
166+
rec_idx = HFSPLUS_BTREE_MAP_NODE_REC_INDEX;
167+
}
168+
169+
ctx->len = hfs_brec_lenoff(node, rec_idx, &off16);
170+
if (!ctx->len)
171+
return ERR_PTR(-ENOENT);
172+
173+
if (!is_bnode_offset_valid(node, off16))
174+
return ERR_PTR(-EIO);
175+
176+
ctx->len = check_and_correct_requested_length(node, off16, ctx->len);
177+
178+
if (byte_offset >= ctx->len)
179+
return ERR_PTR(-EINVAL);
180+
181+
page_off = (u32)off16 + node->page_offset + byte_offset;
182+
ctx->page_idx = page_off >> PAGE_SHIFT;
183+
ctx->off = page_off & ~PAGE_MASK;
184+
185+
return node->page[ctx->page_idx];
186+
}
187+
188+
/**
189+
* hfs_bmap_clear_bit - clear a bit in the b-tree map
190+
* @node: the b-tree node containing the map record
191+
* @node_bit_idx: the relative bit index within the node's map record
192+
*
193+
* Returns 0 on success, -EINVAL if already clear, or negative error code.
194+
*/
195+
static int hfs_bmap_clear_bit(struct hfs_bnode *node, u32 node_bit_idx)
196+
{
197+
struct hfs_bmap_ctx ctx;
198+
struct page *page;
199+
u8 *bmap, mask;
200+
201+
page = hfs_bmap_get_map_page(node, &ctx, node_bit_idx / BITS_PER_BYTE);
202+
if (IS_ERR(page))
203+
return PTR_ERR(page);
204+
205+
bmap = kmap_local_page(page);
206+
207+
mask = 1 << (7 - (node_bit_idx % BITS_PER_BYTE));
208+
209+
if (!(bmap[ctx.off] & mask)) {
210+
kunmap_local(bmap);
211+
return -EINVAL;
212+
}
213+
214+
bmap[ctx.off] &= ~mask;
215+
set_page_dirty(page);
216+
kunmap_local(bmap);
217+
218+
return 0;
219+
}
220+
132221
/* Get a reference to a B*Tree and do some initial checks */
133222
struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
134223
{
@@ -374,11 +463,9 @@ int hfs_bmap_reserve(struct hfs_btree *tree, u32 rsvd_nodes)
374463
struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
375464
{
376465
struct hfs_bnode *node, *next_node;
377-
struct page **pagep;
466+
struct hfs_bmap_ctx ctx;
467+
struct page *page;
378468
u32 nidx, idx;
379-
unsigned off;
380-
u16 off16;
381-
u16 len;
382469
u8 *data, byte, m;
383470
int i, res;
384471

@@ -390,30 +477,26 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
390477
node = hfs_bnode_find(tree, nidx);
391478
if (IS_ERR(node))
392479
return node;
393-
len = hfs_brec_lenoff(node, 2, &off16);
394-
off = off16;
395480

396-
if (!is_bnode_offset_valid(node, off)) {
481+
page = hfs_bmap_get_map_page(node, &ctx, 0);
482+
if (IS_ERR(page)) {
483+
res = PTR_ERR(page);
397484
hfs_bnode_put(node);
398-
return ERR_PTR(-EIO);
485+
return ERR_PTR(res);
399486
}
400-
len = check_and_correct_requested_length(node, off, len);
401487

402-
off += node->page_offset;
403-
pagep = node->page + (off >> PAGE_SHIFT);
404-
data = kmap_local_page(*pagep);
405-
off &= ~PAGE_MASK;
488+
data = kmap_local_page(page);
406489
idx = 0;
407490

408491
for (;;) {
409-
while (len) {
410-
byte = data[off];
492+
while (ctx.len) {
493+
byte = data[ctx.off];
411494
if (byte != 0xff) {
412495
for (m = 0x80, i = 0; i < 8; m >>= 1, i++) {
413496
if (!(byte & m)) {
414497
idx += i;
415-
data[off] |= m;
416-
set_page_dirty(*pagep);
498+
data[ctx.off] |= m;
499+
set_page_dirty(page);
417500
kunmap_local(data);
418501
tree->free_nodes--;
419502
mark_inode_dirty(tree->inode);
@@ -423,13 +506,14 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
423506
}
424507
}
425508
}
426-
if (++off >= PAGE_SIZE) {
509+
if (++ctx.off >= PAGE_SIZE) {
427510
kunmap_local(data);
428-
data = kmap_local_page(*++pagep);
429-
off = 0;
511+
page = node->page[++ctx.page_idx];
512+
data = kmap_local_page(page);
513+
ctx.off = 0;
430514
}
431515
idx += 8;
432-
len--;
516+
ctx.len--;
433517
}
434518
kunmap_local(data);
435519
nidx = node->next;
@@ -443,22 +527,22 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
443527
return next_node;
444528
node = next_node;
445529

446-
len = hfs_brec_lenoff(node, 0, &off16);
447-
off = off16;
448-
off += node->page_offset;
449-
pagep = node->page + (off >> PAGE_SHIFT);
450-
data = kmap_local_page(*pagep);
451-
off &= ~PAGE_MASK;
530+
page = hfs_bmap_get_map_page(node, &ctx, 0);
531+
if (IS_ERR(page)) {
532+
res = PTR_ERR(page);
533+
hfs_bnode_put(node);
534+
return ERR_PTR(res);
535+
}
536+
data = kmap_local_page(page);
452537
}
453538
}
454539

455540
void hfs_bmap_free(struct hfs_bnode *node)
456541
{
457542
struct hfs_btree *tree;
458-
struct page *page;
459543
u16 off, len;
460544
u32 nidx;
461-
u8 *data, byte, m;
545+
int res;
462546

463547
hfs_dbg("node %u\n", node->this);
464548
BUG_ON(!node->this);
@@ -495,24 +579,15 @@ void hfs_bmap_free(struct hfs_bnode *node)
495579
}
496580
len = hfs_brec_lenoff(node, 0, &off);
497581
}
498-
off += node->page_offset + nidx / 8;
499-
page = node->page[off >> PAGE_SHIFT];
500-
data = kmap_local_page(page);
501-
off &= ~PAGE_MASK;
502-
m = 1 << (~nidx & 7);
503-
byte = data[off];
504-
if (!(byte & m)) {
505-
pr_crit("trying to free free bnode "
506-
"%u(%d)\n",
507-
node->this, node->type);
508-
kunmap_local(data);
509-
hfs_bnode_put(node);
510-
return;
582+
583+
res = hfs_bmap_clear_bit(node, nidx);
584+
if (res == -EINVAL) {
585+
pr_crit("trying to free free bnode %u(%d)\n",
586+
node->this, node->type);
587+
} else if (!res) {
588+
tree->free_nodes++;
589+
mark_inode_dirty(tree->inode);
511590
}
512-
data[off] = byte & ~m;
513-
set_page_dirty(page);
514-
kunmap_local(data);
591+
515592
hfs_bnode_put(node);
516-
tree->free_nodes++;
517-
mark_inode_dirty(tree->inode);
518593
}

include/linux/hfs_common.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,8 @@ struct hfs_btree_header_rec {
510510
#define HFSPLUS_NODE_MXSZ 32768
511511
#define HFSPLUS_ATTR_TREE_NODE_SIZE 8192
512512
#define HFSPLUS_BTREE_HDR_NODE_RECS_COUNT 3
513+
#define HFSPLUS_BTREE_HDR_MAP_REC_INDEX 2 /* Map (bitmap) record in Header node */
514+
#define HFSPLUS_BTREE_MAP_NODE_REC_INDEX 0 /* Map record in Map Node */
513515
#define HFSPLUS_BTREE_HDR_USER_BYTES 128
514516

515517
/* btree key type */

0 commit comments

Comments
 (0)