linux-kernel - Re: [PATCH v2] hfsplus: validate btree bitmap during mount and handle corruption gracefully

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2283988f9edee66e648e257c303d5e4d77925402.camel@mpiricsoftware.com>
Date: Fri, 30 Jan 2026 15:13:04 +0530
From: Shardul Bankar <shardul.b@...ricsoftware.com>
To: Viacheslav Dubeyko <Slava.Dubeyko@....com>, 
	"glaubitz@...sik.fu-berlin.de"
	 <glaubitz@...sik.fu-berlin.de>, "frank.li@...o.com" <frank.li@...o.com>, 
	"slava@...eyko.com"
	 <slava@...eyko.com>, "linux-fsdevel@...r.kernel.org"
	 <linux-fsdevel@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	 <linux-kernel@...r.kernel.org>
Cc: "syzbot+1c8ff72d0cd8a50dfeaa@...kaller.appspotmail.com"
	 <syzbot+1c8ff72d0cd8a50dfeaa@...kaller.appspotmail.com>, 
	"janak@...ricsoftware.com"
	 <janak@...ricsoftware.com>, shardulsb08@...il.com
Subject: Re:  [PATCH v2] hfsplus: validate btree bitmap during mount and
 handle corruption gracefully

On Mon, 2026-01-26 at 22:42 +0000, Viacheslav Dubeyko wrote:
> On Sun, 2026-01-25 at 08:37 +0530, Shardul Bankar wrote:
> > 
> > @@ -176,6 +238,13 @@ struct hfs_btree *hfs_btree_open(struct
> > super_block *sb, u32 id)
> >         tree->max_key_len = be16_to_cpu(head->max_key_len);
> >         tree->depth = be16_to_cpu(head->depth);
> >  
> > +       /* Validate bitmap: node 0 must be marked allocated */
> > +       if (hfsplus_validate_btree_bitmap(tree, head)) {
> > +               struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
> > +
> > +               sbi->btree_bitmap_corrupted = true;
> 
> Please, see my comment about this field.
> 
> > +       }
> > +
> >         /* Verify the tree and set the correct compare function */
> >         switch (id) {
> >         case HFSPLUS_EXT_CNID:
> > diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> > index 45fe3a12ecba..b925878333d4 100644
> > --- a/fs/hfsplus/hfsplus_fs.h
> > +++ b/fs/hfsplus/hfsplus_fs.h
> > @@ -154,6 +154,7 @@ struct hfsplus_sb_info {
> >  
> >         int part, session;
> >         unsigned long flags;
> > +       bool btree_bitmap_corrupted;    /* Bitmap corruption
> > detected during btree open */
> 
> This field is completely unnecessary. The hfs_btree_open() can return
> -EROFS
> error code and hfsplus_fill_super() can process it.
>   
Hi Slava,

Thanks for the review.

Regarding the suggestion to convert hfs_btree_open() to return
ERR_PTR(-EROFS):

I reviewed this, but I cannot use ERR_PTR for the corruption case
because it would defeat a purpose of the patch (data recovery).

If hfs_btree_open() returns -EROFS, the caller hfsplus_fill_super()
would receive the error code but would have no tree object to work
with. Without the B-tree structure, we cannot mount the filesystem-even
read-only-making data recovery impossible.

To support recovery, hfs_btree_open() must return a valid tree pointer
even when corruption is detected.

Therefore, for v3, I plan to:

    -Keep the return type as-is to avoid scope creep and ensure easy
backporting.

    -Use sb->s_flags |= SB_RDONLY inside hfs_btree_open() to flag the
safety issue.

    -Drop the bool flag I added in v2 (as you requested) and simply
check sb_rdonly(sb) in fill_super to print the warning.

I will, of course, address your other comments regarding named
constants and pointer arithmetic in v3.

Does this sound acceptable?

Thanks,
Shardul