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.
