mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] hfs, hfsplus: remove WARN_ON when new bnode already hashed
@ 2026-06-22 17:17 Aditya Srivastava
  2026-06-23  6:03 ` Viacheslav Dubeyko
  0 siblings, 1 reply; 2+ messages in thread
From: Aditya Srivastava @ 2026-06-22 17:17 UTC (permalink / raw)
  To: slava, glaubitz, frank.li
  Cc: linux-fsdevel, linux-kernel, Aditya Prakash Srivastava,
	syzbot+2bf21610eea63cb2ce93

From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>

Syzbot reported a warning in hfsplus_bnode_create() when mounting and
traversing a fuzzed/corrupted disk image.

The warning is triggered because the node index is found to be already
active and hashed in the btree hash table, which indicates on-disk
metadata corruption.

Filesystem corruption should be handled gracefully by returning an error
and logging a message, rather than triggering kernel-level warnings
(which can panic the system under panic_on_warn=1 configurations).
The code already prints a critical message and returns -EEXIST (or the
node in hfs), so the WARN_ON() is completely redundant and dangerous.

Remove the WARN_ON(1) from both hfs_bnode_create() and
hfsplus_bnode_create().

Reported-by: syzbot+2bf21610eea63cb2ce93@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=2bf21610eea63cb2ce93
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
---
 fs/hfs/bnode.c     | 1 -
 fs/hfsplus/bnode.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
index 13d58c51fc46..335834961741 100644
--- a/fs/hfs/bnode.c
+++ b/fs/hfs/bnode.c
@@ -517,7 +517,6 @@ struct hfs_bnode *hfs_bnode_create(struct hfs_btree *tree, u32 num)
 	spin_unlock(&tree->hash_lock);
 	if (node) {
 		pr_crit("new node %u already hashed?\n", num);
-		WARN_ON(1);
 		return node;
 	}
 	node = __hfs_bnode_create(tree, num);
diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
index f8b5a8ae58ff..bfc2a7f53edd 100644
--- a/fs/hfsplus/bnode.c
+++ b/fs/hfsplus/bnode.c
@@ -631,7 +631,6 @@ struct hfs_bnode *hfs_bnode_create(struct hfs_btree *tree, u32 num)
 	spin_unlock(&tree->hash_lock);
 	if (node) {
 		pr_crit("new node %u already hashed?\n", num);
-		WARN_ON(1);
 		return ERR_PTR(-EEXIST);
 	}
 	node = __hfs_bnode_create(tree, num);
-- 
2.47.3


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

* Re: [PATCH] hfs, hfsplus: remove WARN_ON when new bnode already hashed
  2026-06-22 17:17 [PATCH] hfs, hfsplus: remove WARN_ON when new bnode already hashed Aditya Srivastava
@ 2026-06-23  6:03 ` Viacheslav Dubeyko
  0 siblings, 0 replies; 2+ messages in thread
From: Viacheslav Dubeyko @ 2026-06-23  6:03 UTC (permalink / raw)
  To: Aditya Srivastava, glaubitz, frank.li
  Cc: linux-fsdevel, linux-kernel, syzbot+2bf21610eea63cb2ce93

On Mon, 2026-06-22 at 17:17 +0000, Aditya Srivastava wrote:
> From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
> 
> Syzbot reported a warning in hfsplus_bnode_create() when mounting and
> traversing a fuzzed/corrupted disk image.
> 
> The warning is triggered because the node index is found to be
> already
> active and hashed in the btree hash table, which indicates on-disk
> metadata corruption.
> 
> Filesystem corruption should be handled gracefully by returning an
> error
> and logging a message, rather than triggering kernel-level warnings
> (which can panic the system under panic_on_warn=1 configurations).
> The code already prints a critical message and returns -EEXIST (or
> the
> node in hfs), so the WARN_ON() is completely redundant and dangerous.
> 
> Remove the WARN_ON(1) from both hfs_bnode_create() and
> hfsplus_bnode_create().
> 
> Reported-by: syzbot+2bf21610eea63cb2ce93@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=2bf21610eea63cb2ce93
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
> ---
>  fs/hfs/bnode.c     | 1 -
>  fs/hfsplus/bnode.c | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
> index 13d58c51fc46..335834961741 100644
> --- a/fs/hfs/bnode.c
> +++ b/fs/hfs/bnode.c
> @@ -517,7 +517,6 @@ struct hfs_bnode *hfs_bnode_create(struct
> hfs_btree *tree, u32 num)
>  	spin_unlock(&tree->hash_lock);
>  	if (node) {
>  		pr_crit("new node %u already hashed?\n", num);
> -		WARN_ON(1);
>  		return node;
>  	}
>  	node = __hfs_bnode_create(tree, num);
> diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
> index f8b5a8ae58ff..bfc2a7f53edd 100644
> --- a/fs/hfsplus/bnode.c
> +++ b/fs/hfsplus/bnode.c
> @@ -631,7 +631,6 @@ struct hfs_bnode *hfs_bnode_create(struct
> hfs_btree *tree, u32 num)
>  	spin_unlock(&tree->hash_lock);
>  	if (node) {
>  		pr_crit("new node %u already hashed?\n", num);
> -		WARN_ON(1);
>  		return ERR_PTR(-EEXIST);
>  	}
>  	node = __hfs_bnode_create(tree, num);

If we have corrupted HFS/HFS+ volume and we can detect it during the
mount operation, then we should not mount the corrupted volume or mount
in READ-ONLY mode. As I remember, we already have HFS+ logic that
detects b-tree's map corruption for the case of node 0. We haven't this
logic for HFS file system logic yet, if I remember correctly. So, you
are welcomed to port this logic for HFS case. Removing WARN_ON() simply
hides the potential issues. So, I don't think that we need to remove
it.

Thanks,
Slava.

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

end of thread, other threads:[~2026-06-23  6:03 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-22 17:17 [PATCH] hfs, hfsplus: remove WARN_ON when new bnode already hashed Aditya Srivastava
2026-06-23  6:03 ` Viacheslav Dubeyko

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