1. Patchew
  2. linux
  3. View series

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

Edward Adam Davis posted 1 patch 2 months ago
There is a newer version of this series
fs/hfsplus/btree.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH v2] hfsplus: Add a sanity check for btree node size
Posted by Edward Adam Davis 2 months ago 
Syzbot reported an uninit-value bug in [1] with a corrupted HFS+ image,
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].

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." Add a check
for btree node size base on technical specification.

[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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=217eb327242d08197efb
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
v1 -> v2: change check base on technical specification

 fs/hfsplus/btree.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
index 761c74ccd653..857705c3fe0d 100644
--- a/fs/hfsplus/btree.c
+++ b/fs/hfsplus/btree.c
@@ -365,6 +365,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)
-- 
2.43.0
Re: [PATCH v2] hfsplus: Add a sanity check for btree node size
Posted by Viacheslav Dubeyko 2 months ago 
On Thu, 2026-04-16 at 17:53 +0800, Edward Adam Davis wrote:
> Syzbot reported an uninit-value bug in [1] with a corrupted HFS+ image,
> 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].
> 
> 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." Add a check
> for btree node size base on technical specification.
> 
> [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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=217eb327242d08197efb
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> v1 -> v2: change check base on technical specification
> 
>  fs/hfsplus/btree.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
> index 761c74ccd653..857705c3fe0d 100644
> --- a/fs/hfsplus/btree.c
> +++ b/fs/hfsplus/btree.c
> @@ -365,6 +365,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)

Technically speaking, you are right that b-tree node size should be aligned on
logical block size. However, I am not sure that mkfs.hfsplus restricts the
creation of volume with b-tree's node size smaller than logical block size but
still in the required range of sizes.

Maybe, we need to declare the constant of HFSPLUS_NODE_MINSZ (512) and to check
this constant instead of logical block size. What do you think?

Thanks,
Slava. 

> +		goto fail_page;
>  	if (!is_power_of_2(size))
>  		goto fail_page;
>  	if (!tree->node_count)
Re: [PATCH v2] hfsplus: Add a sanity check for btree node size
Posted by Edward Adam Davis 2 months ago 
On Thu, 16 Apr 2026 15:16:15 -0700, Viacheslav Dubeyko wrote:
> > Syzbot reported an uninit-value bug in [1] with a corrupted HFS+ image,
> > 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].
> > 
> > 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." Add a check
> > for btree node size base on technical specification.
> > 
> > [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@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=217eb327242d08197efb
> > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > ---
> > v1 -> v2: change check base on technical specification
> > 
> >  fs/hfsplus/btree.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
> > index 761c74ccd653..857705c3fe0d 100644
> > --- a/fs/hfsplus/btree.c
> > +++ b/fs/hfsplus/btree.c
> > @@ -365,6 +365,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)
> 
> Technically speaking, you are right that b-tree node size should be aligned on
> logical block size. However, I am not sure that mkfs.hfsplus restricts the
> creation of volume with b-tree's node size smaller than logical block size but
> still in the required range of sizes.
> 
> Maybe, we need to declare the constant of HFSPLUS_NODE_MINSZ (512) and to check
> this constant instead of logical block size. What do you think?
Hmm, that's much safer.

Edward
BR
[PATCH v3] hfsplus: Add a sanity check for btree node size
Posted by Edward Adam Davis 2 months ago 
Syzbot reported an uninit-value bug in [1] with a corrupted HFS+ image,
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].

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." Add a check
for btree node size base on technical specification.

[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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=217eb327242d08197efb
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
v1 -> v2: change check base on technical specification
v2 -> v3: using const min size

 fs/hfsplus/btree.c         | 2 ++
 include/linux/hfs_common.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
index 761c74ccd653..394542a47e60 100644
--- a/fs/hfsplus/btree.c
+++ b/fs/hfsplus/btree.c
@@ -365,6 +365,8 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
 	}
 
 	size = tree->node_size;
+	if (size < HFSPLUS_NODE_MINSZ || size > HFSPLUS_NODE_MXSZ)
+		goto fail_page;
 	if (!is_power_of_2(size))
 		goto fail_page;
 	if (!tree->node_count)
diff --git a/include/linux/hfs_common.h b/include/linux/hfs_common.h
index 07dfc39630ab..45fb4c9ff9f5 100644
--- a/include/linux/hfs_common.h
+++ b/include/linux/hfs_common.h
@@ -513,6 +513,7 @@ struct hfs_btree_header_rec {
 /* HFS+ BTree misc info */
 #define HFSPLUS_TREE_HEAD			0
 #define HFSPLUS_NODE_MXSZ			32768
+#define HFSPLUS_NODE_MINSZ			512
 #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 */
-- 
2.43.0
Re: [PATCH v3] hfsplus: Add a sanity check for btree node size
Posted by Viacheslav Dubeyko 2 months ago 
On Fri, 2026-04-17 at 07:44 +0800, Edward Adam Davis wrote:
> Syzbot reported an uninit-value bug in [1] with a corrupted HFS+ image,
> 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].
> 
> 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." Add a check
> for btree node size base on technical specification.
> 
> [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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=217eb327242d08197efb
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> v1 -> v2: change check base on technical specification
> v2 -> v3: using const min size
> 
>  fs/hfsplus/btree.c         | 2 ++
>  include/linux/hfs_common.h | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
> index 761c74ccd653..394542a47e60 100644
> --- a/fs/hfsplus/btree.c
> +++ b/fs/hfsplus/btree.c
> @@ -365,6 +365,8 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
>  	}
>  
>  	size = tree->node_size;
> +	if (size < HFSPLUS_NODE_MINSZ || size > HFSPLUS_NODE_MXSZ)
> +		goto fail_page;
>  	if (!is_power_of_2(size))
>  		goto fail_page;
>  	if (!tree->node_count)
> diff --git a/include/linux/hfs_common.h b/include/linux/hfs_common.h
> index 07dfc39630ab..45fb4c9ff9f5 100644
> --- a/include/linux/hfs_common.h
> +++ b/include/linux/hfs_common.h
> @@ -513,6 +513,7 @@ struct hfs_btree_header_rec {
>  /* HFS+ BTree misc info */
>  #define HFSPLUS_TREE_HEAD			0
>  #define HFSPLUS_NODE_MXSZ			32768
> +#define HFSPLUS_NODE_MINSZ			512
>  #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 */

Looks good. Thanks a lot for the fix.

Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>

Thanks,
Slava.

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.