Re: [PATCH] hfsplus: Add a sanity check for catalog btree node size

[Edward Adam Davis]

On Wed, 15 Apr 2026 15:32:59 -0700, Viacheslav Dubeyko wrote:
> > Syzbot reported a uninit-value bug in [1], during the file system mounting
> > process, specifically while loading the catalog, a corrupted node_size
> > value of 1 caused the rec_off argument passed to hfs_bnode_read_u16()
> > (within hfs_bnode_find()) to be excessively large. Consequently, the
> > function failed to return a valid value to initialize the off variable,
> > triggering the bug [1].
> >
> > To prevent similar issues, a check for the catalog btree node size has
> > been added within the hfsplus_btree_open() function.
> >
> > [1]
> > BUG: KMSAN: uninit-value in hfsplus_bnode_find+0x141c/0x1600 fs/hfsplus/bnode.c:584
> >  hfsplus_bnode_find+0x141c/0x1600 fs/hfsplus/bnode.c:584
> >  hfsplus_btree_open+0x169a/0x1e40 fs/hfsplus/btree.c:382
> >  hfsplus_fill_super+0x111f/0x2770 fs/hfsplus/super.c:553
> >  get_tree_bdev_flags+0x6e6/0x920 fs/super.c:1694
> >  get_tree_bdev+0x38/0x50 fs/super.c:1717
> >  hfsplus_get_tree+0x35/0x40 fs/hfsplus/super.c:709
> >  vfs_get_tree+0xb3/0x5d0 fs/super.c:1754
> >  fc_mount fs/namespace.c:1193 [inline]
> >
> > Fixes: 8ad2c6a36ac4 ("hfsplus: validate b-tree node 0 bitmap at mount time")
> > Reported-by: syzbot+217eb327242d08197efb@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Closes: https://syzkaller.appspot.com/bug?extid=217eb327242d08197efb
> > Signed-off-by: Edward Adam Davis <eadavis@xxxxxx>
> > ---
> >  fs/hfsplus/btree.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
> > index 761c74ccd653..61050ffe425e 100644
> > --- a/fs/hfsplus/btree.c
> > +++ b/fs/hfsplus/btree.c
> > @@ -337,6 +337,11 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
> >  			pr_err("invalid catalog btree flag\n");
> >  			goto fail_page;
> >  		}
> > +		if (tree->node_size < 2) {
> 
> Every node starts from BTree node descriptor: struct hfs_bnode_desc. So, the
> size of node cannot be lesser than that. However, technical specification
> declares that: "The node size (which is expressed in bytes) must be power of
> two, from 512 through 32,768, inclusive.". So, we can add more smart check here.
Perhaps it could be adjusted as follows:
diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
index 61050ffe425e..287cef5b5c56 100644
--- a/fs/hfsplus/btree.c
+++ b/fs/hfsplus/btree.c
@@ -370,6 +370,8 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
        }
 
        size = tree->node_size;
+       if (size < sb->s_blocksize || size > HFSPLUS_NODE_MXSZ)
+               goto fail_page;
        if (!is_power_of_2(size))
                goto fail_page;
        if (!tree->node_count)

> 
> And, maybe, it makes sense to check the node size value at the places of using
> it. What do you think?
> 
> But we have this check of node_size in hfs_btree_open() [1]:
> 
> 	size = tree->node_size;
> 	if (!is_power_of_2(size))
> 		goto fail_page;
> 
> If node size is 1, for example, then this check should fail to execute the
> hfs_btree_open(). How, finally, do we have node_size == 1 during the
> hfs_bnode_find()? I don't quite follow.
You overlooked that 2 to the power of 0 is 1.

Edward
BR