[PATCH 1/3] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length

[PATCH 1/3] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

check_and_correct_requested_length() compares (off + len) against
node_size using u32 arithmetic.  When the caller passes a large len
value (e.g. from an underflowed subtraction in hfs_brec_remove()),
off + len can wrap past 2^32 and produce a small result, causing the
bounds check to pass when it should fail.

For example, with off=14 and len=0xFFFFFFF2 (underflowed from
data_off - keyoffset - size in hfs_brec_remove), off + len wraps to 6,
which is less than a typical node_size of 512, so the check passes and
the subsequent memmove reads ~4GB past the node buffer.

Fix this by comparing len against (node_size - off) instead.  Since
is_bnode_offset_valid() already guarantees off < node_size before this
point, the subtraction cannot underflow.

Reported-by: syzbot+6df204b70bf3261691c5@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=6df204b70bf3261691c5
Tested-by: syzbot+6df204b70bf3261691c5@syzkaller.appspotmail.com
Reported-by: syzbot+e76bf3d19b85350571ac@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e76bf3d19b85350571ac
Tested-by: syzbot+e76bf3d19b85350571ac@syzkaller.appspotmail.com
Fixes: a431930c9bac ("hfs: fix slab-out-of-bounds in hfs_bnode_read()")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 fs/hfs/bnode.c          | 2 +-
 fs/hfsplus/hfsplus_fs.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
index 13d58c51fc46b..c00645a4a5733 100644
--- a/fs/hfs/bnode.c
+++ b/fs/hfs/bnode.c
@@ -41,7 +41,7 @@ u32 check_and_correct_requested_length(struct hfs_bnode *node, u32 off, u32 len)
 
 	node_size = node->tree->node_size;
 
-	if ((off + len) > node_size) {
+	if (len > node_size - off) {
 		u32 new_len = node_size - off;
 
 		pr_err("requested length has been corrected: "
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 3545b8dbf11c5..10b2dda3f8044 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -600,7 +600,7 @@ u32 check_and_correct_requested_length(struct hfs_bnode *node, u32 off, u32 len)
 
 	node_size = node->tree->node_size;
 
-	if ((off + len) > node_size) {
+	if (len > node_size - off) {
 		u32 new_len = node_size - off;
 
 		pr_err("requested length has been corrected: "
-- 
2.47.3

[PATCH 2/3] hfs/hfsplus: initialize data buffer in hfs_bnode_read_u16 and hfs_bnode_read_u8

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

hfs_bnode_read_u16() and hfs_bnode_read_u8() declare local data buffers
without initialization, then pass them to hfs_bnode_read().  If
is_bnode_offset_valid() fails inside hfs_bnode_read(), the function
returns early without writing to the buffer, leaving it uninitialized.
The caller then returns the garbage value to its caller.

This triggers KMSAN uninit-value reports when a corrupted HFS+ image
has a node_size of 1, causing rec_off to underflow in hfs_bnode_find()
and the subsequent hfs_bnode_read_u16() to operate on an invalid offset.

Zero-initialize both buffers so that callers get a deterministic zero
value when the underlying read fails.

Reported-by: syzbot+217eb327242d08197efb@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=217eb327242d08197efb
Tested-by: syzbot+217eb327242d08197efb@syzkaller.appspotmail.com
Fixes: a431930c9bac ("hfs: fix slab-out-of-bounds in hfs_bnode_read()")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 fs/hfs/bnode.c     | 4 ++--
 fs/hfsplus/bnode.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
index c00645a4a5733..08307faea7a68 100644
--- a/fs/hfs/bnode.c
+++ b/fs/hfs/bnode.c
@@ -97,7 +97,7 @@ void hfs_bnode_read(struct hfs_bnode *node, void *buf, u32 off, u32 len)
 
 u16 hfs_bnode_read_u16(struct hfs_bnode *node, u32 off)
 {
-	__be16 data;
+	__be16 data = 0;
 	// optimize later...
 	hfs_bnode_read(node, &data, off, 2);
 	return be16_to_cpu(data);
@@ -105,7 +105,7 @@ u16 hfs_bnode_read_u16(struct hfs_bnode *node, u32 off)
 
 u8 hfs_bnode_read_u8(struct hfs_bnode *node, u32 off)
 {
-	u8 data;
+	u8 data = 0;
 	// optimize later...
 	hfs_bnode_read(node, &data, off, 1);
 	return data;
diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
index f8b5a8ae58ff5..35790085b5b2e 100644
--- a/fs/hfsplus/bnode.c
+++ b/fs/hfsplus/bnode.c
@@ -55,7 +55,7 @@ void hfs_bnode_read(struct hfs_bnode *node, void *buf, u32 off, u32 len)
 
 u16 hfs_bnode_read_u16(struct hfs_bnode *node, u32 off)
 {
-	__be16 data;
+	__be16 data = 0;
 	/* TODO: optimize later... */
 	hfs_bnode_read(node, &data, off, 2);
 	return be16_to_cpu(data);
@@ -63,7 +63,7 @@ u16 hfs_bnode_read_u16(struct hfs_bnode *node, u32 off)
 
 u8 hfs_bnode_read_u8(struct hfs_bnode *node, u32 off)
 {
-	u8 data;
+	u8 data = 0;
 	/* TODO: optimize later... */
 	hfs_bnode_read(node, &data, off, 1);
 	return data;
-- 
2.47.3

Re: [PATCH 2/3] hfs/hfsplus: initialize data buffer in hfs_bnode_read_u16 and hfs_bnode_read_u8

[Viacheslav Dubeyko]

On Fri, 2026-05-01 at 11:02 +0000, Tristan Madani wrote:
> From: Tristan Madani <tristan@talencesecurity.com>
> 
> hfs_bnode_read_u16() and hfs_bnode_read_u8() declare local data buffers
> without initialization, then pass them to hfs_bnode_read().  If
> is_bnode_offset_valid() fails inside hfs_bnode_read(), the function
> returns early without writing to the buffer, leaving it uninitialized.
> The caller then returns the garbage value to its caller.
> 
> This triggers KMSAN uninit-value reports when a corrupted HFS+ image
> has a node_size of 1, causing rec_off to underflow in hfs_bnode_find()
> and the subsequent hfs_bnode_read_u16() to operate on an invalid offset.
> 
> Zero-initialize both buffers so that callers get a deterministic zero
> value when the underlying read fails.
> 
> Reported-by: syzbot+217eb327242d08197efb@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=217eb327242d08197efb
> Tested-by: syzbot+217eb327242d08197efb@syzkaller.appspotmail.com
> Fixes: a431930c9bac ("hfs: fix slab-out-of-bounds in hfs_bnode_read()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
>  fs/hfs/bnode.c     | 4 ++--
>  fs/hfsplus/bnode.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
> index c00645a4a5733..08307faea7a68 100644
> --- a/fs/hfs/bnode.c
> +++ b/fs/hfs/bnode.c
> @@ -97,7 +97,7 @@ void hfs_bnode_read(struct hfs_bnode *node, void *buf, u32 off, u32 len)
>  
>  u16 hfs_bnode_read_u16(struct hfs_bnode *node, u32 off)
>  {
> -	__be16 data;
> +	__be16 data = 0;
>  	// optimize later...
>  	hfs_bnode_read(node, &data, off, 2);
>  	return be16_to_cpu(data);
> @@ -105,7 +105,7 @@ u16 hfs_bnode_read_u16(struct hfs_bnode *node, u32 off)
>  
>  u8 hfs_bnode_read_u8(struct hfs_bnode *node, u32 off)
>  {
> -	u8 data;
> +	u8 data = 0;
>  	// optimize later...
>  	hfs_bnode_read(node, &data, off, 1);
>  	return data;
> diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
> index f8b5a8ae58ff5..35790085b5b2e 100644
> --- a/fs/hfsplus/bnode.c
> +++ b/fs/hfsplus/bnode.c
> @@ -55,7 +55,7 @@ void hfs_bnode_read(struct hfs_bnode *node, void *buf, u32 off, u32 len)
>  
>  u16 hfs_bnode_read_u16(struct hfs_bnode *node, u32 off)
>  {
> -	__be16 data;
> +	__be16 data = 0;
>  	/* TODO: optimize later... */
>  	hfs_bnode_read(node, &data, off, 2);
>  	return be16_to_cpu(data);
> @@ -63,7 +63,7 @@ u16 hfs_bnode_read_u16(struct hfs_bnode *node, u32 off)
>  
>  u8 hfs_bnode_read_u8(struct hfs_bnode *node, u32 off)
>  {
> -	u8 data;
> +	u8 data = 0;
>  	/* TODO: optimize later... */
>  	hfs_bnode_read(node, &data, off, 1);
>  	return data;

If I remember correctly, I already shared that hfs_bnode_read() is called in
multiple places. So, this patch is not enough for complete fix of the issue.

Thanks,
Slava.

[PATCH 3/3] hfsplus: fix null pointer dereference in hfsplus_create_attributes_file

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

hfsplus_create_attributes_file() calls hfsplus_mark_inode_dirty() with
HFSPLUS_ATTR_TREE_I(sb) before sbi->attr_tree has been set by
hfs_btree_open().  HFSPLUS_ATTR_TREE_I dereferences sbi->attr_tree to
reach ->inode, causing a null pointer dereference when attr_tree is
still NULL.

Move the mark_dirty call to after hfs_btree_open() and guard it with a
NULL check on sbi->attr_tree.

Reported-by: syzbot+bc70a12e438dadba4fb4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=bc70a12e438dadba4fb4
Tested-by: syzbot+bc70a12e438dadba4fb4@syzkaller.appspotmail.com
Fixes: ee8422d00b7c ("hfsplus: fix potential Allocation File corruption after fsync")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 fs/hfsplus/xattr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
index 452a1f9becb2d..1ea9f313368c5 100644
--- a/fs/hfsplus/xattr.c
+++ b/fs/hfsplus/xattr.c
@@ -317,12 +317,13 @@ static int hfsplus_create_attributes_file(struct super_block *sb)
 		next_node++;
 	}
 
-	hfsplus_mark_inode_dirty(HFSPLUS_ATTR_TREE_I(sb), HFSPLUS_I_ATTR_DIRTY);
 	hfsplus_mark_inode_dirty(attr_file, HFSPLUS_I_ATTR_DIRTY);
 
 	sbi->attr_tree = hfs_btree_open(sb, HFSPLUS_ATTR_CNID);
 	if (!sbi->attr_tree)
 		pr_err("failed to load attributes file\n");
+	else
+		hfsplus_mark_inode_dirty(HFSPLUS_ATTR_TREE_I(sb), HFSPLUS_I_ATTR_DIRTY);
 
 failed_header_node_init:
 	kfree(buf);
-- 
2.47.3

Re: [PATCH 3/3] hfsplus: fix null pointer dereference in hfsplus_create_attributes_file

[Viacheslav Dubeyko]

On Fri, 2026-05-01 at 11:02 +0000, Tristan Madani wrote:
> From: Tristan Madani <tristan@talencesecurity.com>
> 
> hfsplus_create_attributes_file() calls hfsplus_mark_inode_dirty() with
> HFSPLUS_ATTR_TREE_I(sb) before sbi->attr_tree has been set by
> hfs_btree_open().  HFSPLUS_ATTR_TREE_I dereferences sbi->attr_tree to
> reach ->inode, causing a null pointer dereference when attr_tree is
> still NULL.
> 
> Move the mark_dirty call to after hfs_btree_open() and guard it with a
> NULL check on sbi->attr_tree.
> 
> Reported-by: syzbot+bc70a12e438dadba4fb4@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=bc70a12e438dadba4fb4
> Tested-by: syzbot+bc70a12e438dadba4fb4@syzkaller.appspotmail.com
> Fixes: ee8422d00b7c ("hfsplus: fix potential Allocation File corruption after fsync")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
>  fs/hfsplus/xattr.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
> index 452a1f9becb2d..1ea9f313368c5 100644
> --- a/fs/hfsplus/xattr.c
> +++ b/fs/hfsplus/xattr.c
> @@ -317,12 +317,13 @@ static int hfsplus_create_attributes_file(struct super_block *sb)
>  		next_node++;
>  	}
>  
> -	hfsplus_mark_inode_dirty(HFSPLUS_ATTR_TREE_I(sb), HFSPLUS_I_ATTR_DIRTY);
>  	hfsplus_mark_inode_dirty(attr_file, HFSPLUS_I_ATTR_DIRTY);
>  
>  	sbi->attr_tree = hfs_btree_open(sb, HFSPLUS_ATTR_CNID);
>  	if (!sbi->attr_tree)
>  		pr_err("failed to load attributes file\n");
> +	else
> +		hfsplus_mark_inode_dirty(HFSPLUS_ATTR_TREE_I(sb), HFSPLUS_I_ATTR_DIRTY);
>  
>  failed_header_node_init:
>  	kfree(buf);


This patch already fixes the issue:

https://lore.kernel.org/linux-fsdevel/6601b6ec0de087674f60566db950449c4e821bfc.camel@redhat.com/

Thanks,
Slava.

Re: [PATCH 1/3] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length

[Viacheslav Dubeyko]

On Fri, 2026-05-01 at 11:02 +0000, Tristan Madani wrote:
> From: Tristan Madani <tristan@talencesecurity.com>
> 
> check_and_correct_requested_length() compares (off + len) against
> node_size using u32 arithmetic.  When the caller passes a large len
> value (e.g. from an underflowed subtraction in hfs_brec_remove()),
> off + len can wrap past 2^32 and produce a small result, causing the
> bounds check to pass when it should fail.
> 
> For example, with off=14 and len=0xFFFFFFF2 (underflowed from
> data_off - keyoffset - size in hfs_brec_remove), off + len wraps to 6,
> which is less than a typical node_size of 512, so the check passes and
> the subsequent memmove reads ~4GB past the node buffer.
> 
> Fix this by comparing len against (node_size - off) instead.  Since
> is_bnode_offset_valid() already guarantees off < node_size before this
> point, the subtraction cannot underflow.
> 
> Reported-by: syzbot+6df204b70bf3261691c5@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=6df204b70bf3261691c5
> Tested-by: syzbot+6df204b70bf3261691c5@syzkaller.appspotmail.com
> Reported-by: syzbot+e76bf3d19b85350571ac@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=e76bf3d19b85350571ac
> Tested-by: syzbot+e76bf3d19b85350571ac@syzkaller.appspotmail.com
> Fixes: a431930c9bac ("hfs: fix slab-out-of-bounds in hfs_bnode_read()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
>  fs/hfs/bnode.c          | 2 +-
>  fs/hfsplus/hfsplus_fs.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
> index 13d58c51fc46b..c00645a4a5733 100644
> --- a/fs/hfs/bnode.c
> +++ b/fs/hfs/bnode.c
> @@ -41,7 +41,7 @@ u32 check_and_correct_requested_length(struct hfs_bnode *node, u32 off, u32 len)
>  
>  	node_size = node->tree->node_size;
>  
> -	if ((off + len) > node_size) {
> +	if (len > node_size - off) {

I don't agree with likewise change. Probably, we need to have:

(u64)off + len

Thanks,
Slava.

>  		u32 new_len = node_size - off;
>  
>  		pr_err("requested length has been corrected: "
> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> index 3545b8dbf11c5..10b2dda3f8044 100644
> --- a/fs/hfsplus/hfsplus_fs.h
> +++ b/fs/hfsplus/hfsplus_fs.h
> @@ -600,7 +600,7 @@ u32 check_and_correct_requested_length(struct hfs_bnode *node, u32 off, u32 len)
>  
>  	node_size = node->tree->node_size;
>  
> -	if ((off + len) > node_size) {
> +	if (len > node_size - off) {
>  		u32 new_len = node_size - off;
>  
>  		pr_err("requested length has been corrected: "