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

[Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>]

On Thu, 2026-03-12 at 19:45 +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.

Yeah, exactly.

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

Yes, you are right here.

> 
> 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;
> +       }

Yes, it's good improvement.

> 
> 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 don't follow how it could happen. Please, take a look here [1]:

	/* try to get the root inode */
	res = hfs_find_init(HFS_SB(sb)->cat_tree, &fd);
	if (res)
		goto bail_no_root;
	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)
			res = -EIO;
	}
	if (res)
		goto bail_hfs_find;
	res = -EINVAL;
	root_inode = hfs_iget(sb, &fd.search_key->cat, &rec);
	hfs_find_exit(&fd);
	if (!root_inode)
		goto bail_no_root;

We request to find exactly HFS_ROOT_CNID. If root folder has another CNID, then
we simply cannot find the record for root folder in Catalog File. So, we never
will call hfs_iget() if we cannot find the record.

Thanks,
Slava.

[1] https://elixir.bootlin.com/linux/v6.19/source/fs/hfs/super.c#L350