[PATCH v2] hfs: return -EFSCORRUPTED instead of BUG() in hfs_write_inode()

[PATCH v2] hfs: return -EFSCORRUPTED instead of BUG() in hfs_write_inode()

[David Maximiliano Hermitte]

hfs: return -EFSCORRUPTED instead of BUG() in hfs_write_inode()

A corrupted or otherwise malformed HFS filesystem image can reach the
default case in hfs_write_inode(). The current code calls BUG() there,
which turns an on-disk filesystem condition into a kernel crash.

Return -EFSCORRUPTED instead. This reports filesystem metadata
corruption consistently with other major filesystems while avoiding a
reachable kernel BUG.

The issue is reproducible with the public syzbot C reproducer linked
below. Before this change, the reproducer triggers a kernel BUG at
fs/hfs/inode.c with RIP in hfs_write_inode(). After this change, the
same reproducer no longer triggers kernel BUG, hfs_write_inode, KASAN,
Oops, Call Trace, or RIP evidence in the validation window.

Reported-by: syzbot+97e301b4b82ae803d21b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=97e301b4b82ae803d21b
Tested-by: David Maximiliano Hermitte <davemadmaxxx@gmail.com>
Signed-off-by: David Maximiliano Hermitte <davemadmaxxx@gmail.com>
---
Changes in v2:
- Return -EFSCORRUPTED for metadata corruption, as suggested by Darrick.

 fs/hfs/inode.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index 89b33a9d46d5..4192f660b64f 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -471,7 +471,6 @@ int hfs_write_inode(struct inode *inode, struct writeback_control *wbc)
 			hfs_btree_write(HFS_SB(inode->i_sb)->cat_tree);
 			return 0;
 		default:
-			BUG();
-			return -EIO;
+			return -EFSCORRUPTED;
 		}
 	}

Re: [PATCH v2] hfs: return -EFSCORRUPTED instead of BUG() in hfs_write_inode()

[Jori Koolstra]

On Sun, Jun 07, 2026 at 03:53:39AM +0000, David Maximiliano Hermitte wrote:
> hfs: return -EFSCORRUPTED instead of BUG() in hfs_write_inode()
> 
> A corrupted or otherwise malformed HFS filesystem image can reach the
> default case in hfs_write_inode(). The current code calls BUG() there,
> which turns an on-disk filesystem condition into a kernel crash.
> 
> Return -EFSCORRUPTED instead. This reports filesystem metadata
> corruption consistently with other major filesystems while avoiding a
> reachable kernel BUG.

How is -EFSCORRUPTED handled now? The BUG() marks a situation that is
pretty bad, so maybe you don't want to continue to do everything as
usual (although a BUG() is very harsh). At the very least, there should
be a warning.

> 
> The issue is reproducible with the public syzbot C reproducer linked
> below. Before this change, the reproducer triggers a kernel BUG at
> fs/hfs/inode.c with RIP in hfs_write_inode(). After this change, the
> same reproducer no longer triggers kernel BUG, hfs_write_inode, KASAN,
> Oops, Call Trace, or RIP evidence in the validation window.
> 
> Reported-by: syzbot+97e301b4b82ae803d21b@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=97e301b4b82ae803d21b
> Tested-by: David Maximiliano Hermitte <davemadmaxxx@gmail.com>
> Signed-off-by: David Maximiliano Hermitte <davemadmaxxx@gmail.com>
> ---
> Changes in v2:
> - Return -EFSCORRUPTED for metadata corruption, as suggested by Darrick.
> 
>  fs/hfs/inode.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
> index 89b33a9d46d5..4192f660b64f 100644
> --- a/fs/hfs/inode.c
> +++ b/fs/hfs/inode.c
> @@ -471,7 +471,6 @@ int hfs_write_inode(struct inode *inode, struct writeback_control *wbc)
>  			hfs_btree_write(HFS_SB(inode->i_sb)->cat_tree);
>  			return 0;
>  		default:
> -			BUG();
> -			return -EIO;
> +			return -EFSCORRUPTED;
>  		}
>  	}

Re: [PATCH v2] hfs: return -EFSCORRUPTED instead of BUG() in hfs_write_inode()

[David Maximiliano Hermitte]

Thank you for the review.

That makes sense. I agree that replacing the BUG() with a silent return is probably not enough if this state indicates serious filesystem corruption.

At this point I would prefer not to slow down the fix by guessing the best HFS/VFS error-handling policy. My main goal was to report and narrow down the reachable BUG() site and provide a minimal tested patch showing that the crash can be avoided.

If someone with more HFS/VFS experience wants to take over the final form of the fix, I would be happy with that. I can also help retest a proposed v3 against the public syzbot reproducer.

Thanks again for taking a look.

Re: [PATCH v2] hfs: return -EFSCORRUPTED instead of BUG() in hfs_write_inode()

[Viacheslav Dubeyko]

On Mon, 2026-06-08 at 04:24 +0000, David Maximiliano Hermitte wrote:
> Thank you for the review.
> 
> That makes sense. I agree that replacing the BUG() with a silent
> return is probably not enough if this state indicates serious
> filesystem corruption.
> 
> At this point I would prefer not to slow down the fix by guessing the
> best HFS/VFS error-handling policy. My main goal was to report and
> narrow down the reachable BUG() site and provide a minimal tested
> patch showing that the crash can be avoided.
> 
> If someone with more HFS/VFS experience wants to take over the final
> form of the fix, I would be happy with that. I can also help retest a
> proposed v3 against the public syzbot reproducer.
> 
> Thanks again for taking a look.

If you have corrupted HFS volume, then the issue should be resolved in
mount logic. I suspect that we discussed this issue multiple times. But
I still don't see that anybody can finish this work.

Thanks,
Slava.

Re: [PATCH v2] hfs: return -EFSCORRUPTED instead of BUG() in hfs_write_inode()

[David Maximiliano Hermitte]

Hello Slava, Jori, all,

I wanted to share one additional diagnostic result from our side.

We are not HFS maintainers, and I do not want to present this as a final patch proposal. We used a small change only as a route-checking experiment, to verify whether the crash ending in hfs_write_inode() / BUG() can be prevented by rejecting corrupt metadata earlier.

The experimental change was in fs/hfs/btree.c, after reading the HFS B-tree header:

	if (tree->root >= tree->node_count)
		goto fail_folio;
	if (tree->leaf_head >= tree->node_count)
		goto fail_folio;
	if (tree->leaf_tail >= tree->node_count)
		goto fail_folio;
	if (tree->leaf_count && (!tree->leaf_head || !tree->leaf_tail))
		goto fail_folio;
	if (tree->depth > tree->node_count)
		goto fail_folio;

We specifically did not modify:

	fs/hfs/inode.c
	hfs_write_inode()
	BUG()

With the same syzbot-style reproducer and the same QEMU setup, we observed:

BEFORE / unpatched:
	boot_linux_seen=true
	root_mounted_seen=true
	repro_started=true
	VQBIT_HFS_INODE_BUG_SITE_3 line=474 hit
	kernel_bug_seen=true
	verdict=BEFORE_BUG_REPRODUCED

AFTER / diagnostic btree validation:
	boot_linux_seen=true
	root_mounted_seen=true
	repro_started=true
	kernel_bug_seen=false
	hfs_write_inode_seen=false
	kasan_seen=false
	oops_seen=false
	verdict=REPRO_RAN_NO_KERNEL_BUG_SEEN

So our current interpretation is:

	The BUG in hfs_write_inode() appears to be the crash sink.
	The corrupt state seems to be accepted earlier.
	Rejecting inconsistent HFS B-tree header fields before traversal prevents the reproducer from reaching hfs_write_inode::BUG().

Again, this is not intended as a polished HFS fix. It is only evidence from a diagnostic route check. I would appreciate your opinion on whether this direction makes sense, and whether the proper fix should live around HFS B-tree / catalog metadata validation rather than changing the final BUG() in hfs_write_inode().

Best regards,
David

Re: [PATCH v2] hfs: return -EFSCORRUPTED instead of BUG() in hfs_write_inode()

[Viacheslav Dubeyko]

On Thu, 2026-06-11 at 17:57 +0000, David Maximiliano Hermitte wrote:
> Hello Slava, Jori, all,
> 
> I wanted to share one additional diagnostic result from our side.
> 

We have long multi-months discussion of this issue. Please, double
check the email list. From my point of view, the issue needs to be
fixed in hfs_read_inode() + hfs_brec_read().

Thanks,
Slava.