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: <12de16685af71b513f8027a8bfd14bc0322eb043.camel@ibm.com>
Date: Mon, 28 Jul 2025 19:37:13 +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 Sat, 2025-07-26 at 06:52 +0900, Tetsuo Handa wrote:
> On 2025/07/26 2:47, Viacheslav Dubeyko wrote:
> > > 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?
> 
> Please explicitly answer this question.
> 
> Is this validation correct that rec.dir.DirID has to be HFS_ROOT_CNID ?
> 
>  	res = hfs_cat_find_brec(sb, HFS_ROOT_CNID, &fd);
>  	if (!res) {
>  		if (fd.entrylength != sizeof(rec.dir)) {
>  			res =  -EIO;
>  			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;
>  	}
> 
> I hope that this validation is correct because the "rec" which hfs_bnode_read()
> reads is controlled by the result of hfs_cat_find_brec(HFS_ROOT_CNID).
> 

I don't see the point of making modifications here. The modification of
hfs_read_inode() should be enough.

Thanks,
Slava.