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.
