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: <1779f2ad-77da-40e3-9ee0-ef6c4cd468fa@I-love.SAKURA.ne.jp>
Date: Sat, 26 Jul 2025 06:52:21 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: Viacheslav Dubeyko <Slava.Dubeyko@....com>,
        "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 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).

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

Checking which particular inode IDs hfs_read_inode() should accept is fine.
But such check cannot fix the bug reported at
https://syzkaller.appspot.com/bug?extid=97e301b4b82ae803d21b .

Checking the inode ID retrieved by hfs_cat_find_brec(HFS_ROOT_CNID) is indeed
HFS_ROOT_CNID can fix the bug reported at
https://syzkaller.appspot.com/bug?extid=97e301b4b82ae803d21b .