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

[George Anthony Vernon <contact@gvernon.com>]

On Thu, Mar 12, 2026 at 07:45:17PM +0900, Tetsuo Handa wrote:
> Since is_valid_catalog_record() is called before inode->i_ino is assigned,
> 
> +               pr_warn("Invalid inode with cnid %lu\n", inode->i_ino);
> 
> always prints 0.
> 
Thank you, I will include a fix in the next patch version.
> kernel test robot <lkp@intel.com> reported that this patch needs below change.
> 
> -               if (!is_valid_catalog_record(rec->file.FlNum, rec->type))
> +               if (!is_valid_catalog_record(be32_to_cpu(rec->file.FlNum), rec->type))
> 
> -               if (!is_valid_catalog_record(rec->dir.DirID, rec->type))
> +               if (!is_valid_catalog_record(be32_to_cpu(rec->dir.DirID), rec->type))
> 
> Because of this endian bug, syzbot did not test is_valid_catalog_record() == false case.
> 
Sorry I don't follow this. How can you tell syzbot did not test the case?
> This patch also needs below change.
> 
> -       if (!root_inode || is_bad_inode(root_inode))
> +       if (!root_inode)
>                 goto bail_no_root;
> +       if (is_bad_inode(root_inode)) {
> +               iput(root_inode);
> +               goto bail_no_root;
> +       }
> 
> Since this bug is reported when "rmmod hfs" is done, syzbot would not be
> able to find this bug.
> 
> And even after both changes are applied, my patch still makes sense
> because mount() operation still succeeds for cnid >= 16. :-)
I agree your patch fixes a real bug. I think Slava is suggesting a fix
to hfs_cat_find_brec() so that it validates the catalog records it
returns. Let's continue this discussion in reply to his email if that's
okay.

Thanks,

George