Re: [PATCH v4] hfs: Validate CNIDs in hfs_read_inode

[George Vernon]

March 19, 2026 at 9:57 AM, "Tetsuo Handa" <penguin-kernel@xxxxxxxxxxxxxxxxxxx mailto:penguin-kernel@xxxxxxxxxxxxxxxxxxx?to=%22Tetsuo%20Handa%22%20%3Cpenguin-kernel%40i-love.sakura.ne.jp%3E > wrote:


> 
> On 2026/03/19 7:49, George Anthony Vernon wrote:
> 
> > 
> > Tetsuo, what do you think about doing your check inside
> >  hfs_cat_find_brec? Something a bit like this:
> > 
> Currently hfs_cat_find_brec() is called by only hfs_fill_super()
> with cnid == HFS_ROOT_CNID. Are you planning to add more callers?
> 
> If no, my patch
> 
>  - if (rec.type != HFS_CDR_DIR)
>  + if (rec.type != HFS_CDR_DIR || rec.dir.DirID != cpu_to_be32(HFS_ROOT_CNID))
> 
> is sufficient.

I'm trying to write a patch Slava will accept, maybe I didn't fully understand what he wanted but I think he wanted to fix hfs_cat_find_brec() so it should not return an incorrect cat record.

> 
> If yes, your
> 
> > 
> > + if (fd->entrylength > sizeof(rec)) {
> >  + pr_err("Bad catalog entry size\n");
> >  + return -EIO;
> >  + }
> > 
> part is not compatible with https://elixir.bootlin.com/linux/v7.0-rc4/source/fs/hfs/super.c#L359
> which requires that fd->entrylength is exactly 70 bytes in order to make sure that rec.dir is
> fully initialized.
> 
>  sizeof(struct hfs_cat_file) = 102
>  sizeof(struct hfs_cat_dir) = 70
>  sizeof(struct hfs_cat_thread) = 46
> 
This can be changed.
> Even if you are planning to add more callers, my patch will be easier to apply to stable.
>
Yes your patch will be easier to backport, but then again HFS has had minimal churn in recent years, so I think this version will still be simple enough, only maybe the interface for debug prints has changed that I can think of.

Thanks,

George