public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length
@ 2026-05-01 11:02 Tristan Madani
  2026-05-01 11:02 ` [PATCH 2/3] hfs/hfsplus: initialize data buffer in hfs_bnode_read_u16 and hfs_bnode_read_u8 Tristan Madani
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Tristan Madani @ 2026-05-01 11:02 UTC (permalink / raw)
  To: Viacheslav Dubeyko, John Paul Adrian Glaubitz, Yangtao Li
  Cc: linux-fsdevel, linux-kernel, stable, Tristan Madani,
	syzbot+6df204b70bf3261691c5, syzbot+e76bf3d19b85350571ac

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] hfs/hfsplus: initialize data buffer in hfs_bnode_read_u16 and hfs_bnode_read_u8
  2026-05-01 11:02 [PATCH 1/3] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length Tristan Madani
@ 2026-05-01 11:02 ` Tristan Madani
  2026-05-04 23:29   ` Viacheslav Dubeyko
  2026-05-01 11:02 ` [PATCH 3/3] hfsplus: fix null pointer dereference in hfsplus_create_attributes_file Tristan Madani
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Tristan Madani @ 2026-05-01 11:02 UTC (permalink / raw)
  To: Viacheslav Dubeyko, John Paul Adrian Glaubitz, Yangtao Li
  Cc: linux-fsdevel, linux-kernel, stable, Tristan Madani,
	syzbot+217eb327242d08197efb

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] hfsplus: fix null pointer dereference in hfsplus_create_attributes_file
  2026-05-01 11:02 [PATCH 1/3] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length Tristan Madani
  2026-05-01 11:02 ` [PATCH 2/3] hfs/hfsplus: initialize data buffer in hfs_bnode_read_u16 and hfs_bnode_read_u8 Tristan Madani
@ 2026-05-01 11:02 ` Tristan Madani
  2026-05-04 23:31   ` Viacheslav Dubeyko
  2026-05-04 23:27 ` [PATCH 1/3] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length Viacheslav Dubeyko
  2026-05-05 11:12 ` [PATCH v2 0/2] hfs/hfsplus: fix OOB access and uninit-value in bnode operations Tristan Madani
  3 siblings, 1 reply; 9+ messages in thread
From: Tristan Madani @ 2026-05-01 11:02 UTC (permalink / raw)
  To: Viacheslav Dubeyko, John Paul Adrian Glaubitz, Yangtao Li
  Cc: linux-fsdevel, linux-kernel, stable, Tristan Madani,
	syzbot+bc70a12e438dadba4fb4

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length
  2026-05-01 11:02 [PATCH 1/3] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length Tristan Madani
  2026-05-01 11:02 ` [PATCH 2/3] hfs/hfsplus: initialize data buffer in hfs_bnode_read_u16 and hfs_bnode_read_u8 Tristan Madani
  2026-05-01 11:02 ` [PATCH 3/3] hfsplus: fix null pointer dereference in hfsplus_create_attributes_file Tristan Madani
@ 2026-05-04 23:27 ` Viacheslav Dubeyko
  2026-05-05 11:12 ` [PATCH v2 0/2] hfs/hfsplus: fix OOB access and uninit-value in bnode operations Tristan Madani
  3 siblings, 0 replies; 9+ messages in thread
From: Viacheslav Dubeyko @ 2026-05-04 23:27 UTC (permalink / raw)
  To: Tristan Madani, Viacheslav Dubeyko, John Paul Adrian Glaubitz,
	Yangtao Li
  Cc: linux-fsdevel, linux-kernel, stable, Tristan Madani,
	syzbot+6df204b70bf3261691c5, syzbot+e76bf3d19b85350571ac

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] hfs/hfsplus: initialize data buffer in hfs_bnode_read_u16 and hfs_bnode_read_u8
  2026-05-01 11:02 ` [PATCH 2/3] hfs/hfsplus: initialize data buffer in hfs_bnode_read_u16 and hfs_bnode_read_u8 Tristan Madani
@ 2026-05-04 23:29   ` Viacheslav Dubeyko
  0 siblings, 0 replies; 9+ messages in thread
From: Viacheslav Dubeyko @ 2026-05-04 23:29 UTC (permalink / raw)
  To: Tristan Madani, Viacheslav Dubeyko, John Paul Adrian Glaubitz,
	Yangtao Li
  Cc: linux-fsdevel, linux-kernel, stable, Tristan Madani,
	syzbot+217eb327242d08197efb

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.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] hfsplus: fix null pointer dereference in hfsplus_create_attributes_file
  2026-05-01 11:02 ` [PATCH 3/3] hfsplus: fix null pointer dereference in hfsplus_create_attributes_file Tristan Madani
@ 2026-05-04 23:31   ` Viacheslav Dubeyko
  0 siblings, 0 replies; 9+ messages in thread
From: Viacheslav Dubeyko @ 2026-05-04 23:31 UTC (permalink / raw)
  To: Tristan Madani, Viacheslav Dubeyko, John Paul Adrian Glaubitz,
	Yangtao Li
  Cc: linux-fsdevel, linux-kernel, stable, Tristan Madani,
	syzbot+bc70a12e438dadba4fb4

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.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 0/2] hfs/hfsplus: fix OOB access and uninit-value in bnode operations
  2026-05-01 11:02 [PATCH 1/3] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length Tristan Madani
                   ` (2 preceding siblings ...)
  2026-05-04 23:27 ` [PATCH 1/3] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length Viacheslav Dubeyko
@ 2026-05-05 11:12 ` Tristan Madani
  2026-05-05 11:12   ` [PATCH v2 1/2] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length Tristan Madani
  2026-05-05 11:12   ` [PATCH v2 2/2] hfs/hfsplus: zero-initialize buffer in hfs_bnode_read Tristan Madani
  3 siblings, 2 replies; 9+ messages in thread
From: Tristan Madani @ 2026-05-05 11:12 UTC (permalink / raw)
  To: Viacheslav Dubeyko; +Cc: linux-fsdevel, linux-kernel

From: Tristan Madani <tristan@talencesecurity.com>

Two fixes for long-standing syzbot reports in the HFS/HFS+ B-tree node
handling code.

Changes since v1 (20260501110218.29906-{1..3}-tristmd@gmail.com):
 - Patch 1: use (u64) widening cast per Dubeyko's feedback
 - Patch 2: memset inside hfs_bnode_read() covering all callers in both
   fs/hfs/ and fs/hfsplus/, per Dubeyko's feedback
 - Dropped patch 3 (NULL deref in hfsplus_create_attributes_file):
   already fixed by Dubeyko's patch [1]

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

Tristan Madani (2):
  hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length
  hfs/hfsplus: zero-initialize buffer in hfs_bnode_read

 fs/hfs/bnode.c          | 4 +++-
 fs/hfsplus/bnode.c      | 2 ++
 fs/hfsplus/hfsplus_fs.h | 2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

-- 
2.47.3

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/2] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length
  2026-05-05 11:12 ` [PATCH v2 0/2] hfs/hfsplus: fix OOB access and uninit-value in bnode operations Tristan Madani
@ 2026-05-05 11:12   ` Tristan Madani
  2026-05-05 11:12   ` [PATCH v2 2/2] hfs/hfsplus: zero-initialize buffer in hfs_bnode_read Tristan Madani
  1 sibling, 0 replies; 9+ messages in thread
From: Tristan Madani @ 2026-05-05 11:12 UTC (permalink / raw)
  To: Viacheslav Dubeyko; +Cc: linux-fsdevel, linux-kernel

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 widening the addition to u64 before comparing against
node_size.  This prevents the u32 wrap while keeping the logic
straightforward.

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..9571f33b91085 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 ((u64)off + len > node_size) {
 		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..0e4268de9e60e 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 ((u64)off + len > node_size) {
 		u32 new_len = node_size - off;
 
 		pr_err("requested length has been corrected: "
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/2] hfs/hfsplus: zero-initialize buffer in hfs_bnode_read
  2026-05-05 11:12 ` [PATCH v2 0/2] hfs/hfsplus: fix OOB access and uninit-value in bnode operations Tristan Madani
  2026-05-05 11:12   ` [PATCH v2 1/2] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length Tristan Madani
@ 2026-05-05 11:12   ` Tristan Madani
  1 sibling, 0 replies; 9+ messages in thread
From: Tristan Madani @ 2026-05-05 11:12 UTC (permalink / raw)
  To: Viacheslav Dubeyko; +Cc: linux-fsdevel, linux-kernel

From: Tristan Madani <tristan@talencesecurity.com>

hfs_bnode_read() can return early without writing to the output buffer
when is_bnode_offset_valid() fails or when check_and_correct_requested_
length() corrects the length to zero.  Callers such as hfs_bnode_read_
u16() and hfs_bnode_read_u8() pass stack-allocated buffers and use the
result unconditionally, leading to KMSAN uninit-value reports.

Rather than initializing at each individual call site, zero the buffer
at the start of hfs_bnode_read() before any validation checks.  This
ensures all callers in both hfs and hfsplus get a deterministic zero
value regardless of which early-return path is taken.

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     | 2 ++
 fs/hfsplus/bnode.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
index 9571f33b91085..25cef62fbba6d 100644
--- a/fs/hfs/bnode.c
+++ b/fs/hfs/bnode.c
@@ -64,6 +64,8 @@ void hfs_bnode_read(struct hfs_bnode *node, void *buf, u32 off, u32 len)
 	u32 bytes_read;
 	u32 bytes_to_read;
 
+	memset(buf, 0, len);
+
 	if (!is_bnode_offset_valid(node, off))
 		return;
 
diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
index f8b5a8ae58ff5..14d1af2c7ba93 100644
--- a/fs/hfsplus/bnode.c
+++ b/fs/hfsplus/bnode.c
@@ -25,6 +25,8 @@ void hfs_bnode_read(struct hfs_bnode *node, void *buf, u32 off, u32 len)
 	struct page **pagep;
 	u32 l;
 
+	memset(buf, 0, len);
+
 	if (!is_bnode_offset_valid(node, off))
 		return;
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-05-05 11:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-01 11:02 [PATCH 1/3] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length Tristan Madani
2026-05-01 11:02 ` [PATCH 2/3] hfs/hfsplus: initialize data buffer in hfs_bnode_read_u16 and hfs_bnode_read_u8 Tristan Madani
2026-05-04 23:29   ` Viacheslav Dubeyko
2026-05-01 11:02 ` [PATCH 3/3] hfsplus: fix null pointer dereference in hfsplus_create_attributes_file Tristan Madani
2026-05-04 23:31   ` Viacheslav Dubeyko
2026-05-04 23:27 ` [PATCH 1/3] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length Viacheslav Dubeyko
2026-05-05 11:12 ` [PATCH v2 0/2] hfs/hfsplus: fix OOB access and uninit-value in bnode operations Tristan Madani
2026-05-05 11:12   ` [PATCH v2 1/2] hfs/hfsplus: fix u32 overflow in check_and_correct_requested_length Tristan Madani
2026-05-05 11:12   ` [PATCH v2 2/2] hfs/hfsplus: zero-initialize buffer in hfs_bnode_read Tristan Madani

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox