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] [day] [month] [year] [list]

Message-ID: <b78c1e380a17186b73bc8641b139eca56a8de964.camel@ibm.com>
Date: Fri, 30 Jan 2026 18:10:09 +0000
From: Viacheslav Dubeyko <Slava.Dubeyko@....com>
To: "shardul.b@...ricsoftware.com" <shardul.b@...ricsoftware.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" <shardulsb08@...il.com>
Subject: RE:  [PATCH v2] hfsplus: validate btree bitmap during mount and
 handle corruption gracefully

On Fri, 2026-01-30 at 15:13 +0530, Shardul Bankar wrote:
> 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.

Yeah, I missed this point. You are correct here.

> 
> 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.

Sounds like a better option.

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

Sounds good.

> 
> I will, of course, address your other comments regarding named
> constants and pointer arithmetic in v3.
> 
> Does this sound acceptable?

Yes, it sounds reasonably well. I like your suggestions.

Thanks,
Slava.