linux-kernel - RE: [PATCH v3] hfs: remove BUG() from hfs_release_folio()/hfs_test_inode()/hfs_write

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: <6db77f5cb0a35de69a5b6b26719e4ffb3fdac8c5.camel@ibm.com>
Date: Fri, 25 Jul 2025 17:47:57 +0000
From: Viacheslav Dubeyko <Slava.Dubeyko@....com>
To: "penguin-kernel@...ove.SAKURA.ne.jp" <penguin-kernel@...ove.SAKURA.ne.jp>,
        "willy@...radead.org" <willy@...radead.org>
CC: "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>,
        "akpm@...ux-foundation.org"
	<akpm@...ux-foundation.org>
Subject: RE: [PATCH v3] hfs: remove BUG() from
 hfs_release_folio()/hfs_test_inode()/hfs_write_inode()

On Fri, 2025-07-25 at 13:16 +0900, Tetsuo Handa wrote:
> On 2025/07/25 8:20, Tetsuo Handa wrote:
> > On 2025/07/25 7:05, Tetsuo Handa wrote:
> > > > > But I can't be convinced that above change is sufficient, for if I do
> > > > > 
> > > > > +	            static u8 serial;
> > > > > +               if (inode->i_ino < HFS_FIRSTUSER_CNID && ((1U << inode->i_ino) & bad_cnid_list))
> > > > > +                       inode->i_ino = (serial++) % 16;
> > > > 
> > > > I don't see the point in flags introduction. It makes logic very complicated.
> > > 
> > > The point of this change is to excecise inode->i_ino for all values between 0 and 15.
> > > Some of values between 0 and 15 must be valid as inode->i_ino , doesn't these? Then,
> > 
> > Background: I assume that the value of rec->dir.DirID comes from the hfs filesystem image in the
> > reproducer (i.e. memfd file associated with /dev/loop0 ). But since I don't know the offset to modify
> > the value if I want the reproducer to pass rec->dir.DirID == 1...15 instead of rec->dir.DirID == 0,
> > I am modifying inode->i_ino here when rec->dir.DirID == 0.
> > 
> 
> I managed to find the offset of rec->dir.DirID in the filesystem image used by
> the reproducer, and confirmed that any 0...15 values except 2..4 shall hit BUG()
> in hfs_write_inode().
> 
> Also, a legitimate filesystem image seems to have rec->dir.DirID == 2.
> 
> That is, the only approach that can avoid hitting BUG() without removing BUG()
> would be to verify that rec.type is HFS_CDR_DIR and rec.dir.DirID is HFS_ROOT_CNID.
> 
> --- a/fs/hfs/super.c
> +++ b/fs/hfs/super.c
> @@ -354,7 +354,7 @@ static int hfs_fill_super(struct super_block *sb, struct fs_context *fc)
>                         goto bail_hfs_find;
>                 }
>                 hfs_bnode_read(fd.bnode, &rec, fd.entryoffset, fd.entrylength);
> -               if (rec.type != HFS_CDR_DIR)
> +               if (rec.type != HFS_CDR_DIR || rec.dir.DirID != cpu_to_be32(HFS_ROOT_CNID))
>                         res = -EIO;
>         }
>         if (res)
> 
> Is this condition correct?
> 
> Discussion on what values should be filtered by hfs_read_inode() is
> out of scope for this syzbot report.

I already shared in previous emails which particular inode IDs are valid or not
for [0-16] group of values in the environment of hfs_read_inode().

Thanks,
Slava.