RE: [PATCH] hfsplus: fix uninit-value in hfsplus_strcasecmp

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

On Sat, 2026-01-24 at 08:16 +0530, Deepanshu Kartikey wrote:
> On Sat, Jan 24, 2026 at 4:51 AM Viacheslav Dubeyko
> <Slava.Dubeyko@ibm.com> wrote:
> > 
> > 
> > If the whole fix will be small, then one patch is better to have. Otherwise,
> > patchset could be more better for the review.
> > 
> 
> Hi Slava,
> 
> Thank you for the guidance! Based on your feedback and the debug output showing
> the partial read (26 bytes read, 520 expected), here's my proposed fix.
> 
> I'd appreciate your review before I send the formal patch.
> 
> ---
> 
> The fix includes two changes:
> 
> 1. Add validation in hfs_brec_read() to reject partial reads:
> 
> int hfs_brec_read(struct hfs_find_data *fd, void *rec, u32 rec_len)
> {
> int res;
> 
> res = hfs_brec_find(fd, hfs_find_rec_by_key);
> if (res)
> return res;
> if (fd->entrylength > rec_len)
> return -EINVAL;
> ++ if (fd->entrylength < rec_len) {

It looks like we can simply combined to check into one:

if (fd->entrylength != rec_len)

However, I am not completely sure that it's completely correct fix. Because, for
example, hfs_cat_find_brec() tries to read hfs_cat_rec union:

	hfs_cat_build_key(sb, fd->search_key, cnid, NULL);
	res = hfs_brec_read(fd, &rec, sizeof(rec));
	if (res)
		return res;

It means that we provide the bigger length that it is required for struct
hfs_cat_file or struct hfs_cat_dir. It sounds to me that the reading of these
records will be rejected. Am I wrong here? 

> ++ pr_err("hfsplus: incomplete catalog record (got %u, expected %u)\n",
> ++        fd->entrylength, rec_len);

It doesn't need to mention "hfsplus:" in error message.

> ++ return -EINVAL;
> ++ }
> hfs_bnode_read(fd->bnode, rec, fd->entryoffset, fd->entrylength);
> return 0;
> }
> 
> 2. Initialize tmp in hfsplus_find_cat() as defensive programming:
> 
> int hfsplus_find_cat(struct super_block *sb, u32 cnid,
>      struct hfs_find_data *fd)
> {
> -- hfsplus_cat_entry tmp;
> ++ hfsplus_cat_entry tmp = {0};
> int err;
> u16 type;
> ...
> }
> 
> ---
> 
> Does this look correct to you? Should I proceed with this approach, or would
> you suggest any modifications?
> 
> 

Please, see my comments above.

Thanks,
Slava.

[1] https://elixir.bootlin.com/linux/v6.19-rc5/source/fs/hfs/catalog.c#L194