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

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

On Wed, 2026-01-21 at 11:56 +0530, Deepanshu Kartikey wrote:
> On Wed, Jan 21, 2026 at 3:59 AM Viacheslav Dubeyko
> <Slava.Dubeyko@ibm.com> wrote:
> 
> > Frankly speaking, I don't quite follow what is wrong with current logic from
> > your explanation. Only, struct hfsplus_cat_thread contains nodeName string. And
> > hfsplus_strcasecmp() can try to compare some strings only for Catalog thread
> > type. But hfs_brec_read() should read namely only this type of Catalog File
> > record. So, I cannot imagine how likewise issue could happen. Could you explain
> > the issue more clearly? How uninitiliazed nodeName strings can be used for
> > comparison? How does it happened? Because, struct hfsplus_cat_thread is the
> > biggest item in hfsplus_cat_entry union. The struct hfsplus_cat_file and struct
> > hfsplus_cat_folder don't contain any strings and strings cannot be used for
> > comparison in these structures case. But struct hfsplus_cat_thread should be
> > read completely with the string.
> 
> Hi Slava,
> 
> Thank you for the review.
> 
> Regarding your question about how uninitialized data is used: You're correct
> that in normal operation, hfs_brec_read() should fully read the catalog
> thread record. However, the KMSAN report from syzbot shows that uninitialized
> data is being accessed.
> 
> Looking at the code flow and the KMSAN stack trace:
> 1. tmp is declared on the stack without initialization in hfsplus_find_cat()
> 2. hfs_brec_read() is called to fill it from disk
> 3. With a corrupted/malformed filesystem image (which syzbot fuzzes),
>    hfs_brec_read() may read partial or invalid data
> 4. The tmp.thread.nodeName.unicode array may not be fully populated
> 5. hfsplus_cat_build_key_uni() copies from this array based on nodeName.length
> 6. hfsplus_strcasecmp() then reads these bytes and passes them to case_fold()
> 7. case_fold() uses the value as an array index: hfsplus_case_fold_table[c >> 8]
> 8. KMSAN detects this uninitialized value being used as an array index
> 

I would like to see this explanation for concrete particular example. We have
this as thread record in Catalog tree [1,2]:

struct hfsplus_unistr {
	__be16 length;
	hfsplus_unichr unicode[HFSPLUS_MAX_STRLEN];
} __packed;

struct hfsplus_cat_thread {
	__be16 type;
	s16 reserved;
	hfsplus_cnid parentID;
	struct hfsplus_unistr nodeName;
} __packed;

So, If hfs_brec_read() reads the hfsplus_cat_thread, the it reads the whole
hfsplus_unistr object that contains as string as length. Even if filesystem
image is corrupted, then, anyway, we have some hfsplus_unistr blob in the b-tree
node. If you talk about "hfs_brec_read() may read partial or invalid data", then
what do you mean here? Do you mean that length is incorrect or string contains
"garbage". My misunderstanding here if hfs_brec_read() reads the
hfsplus_cat_thread from the node, then it reads the whole hfsplus_unistr blob.
Then, how can we "read partial or invalid data"? I don't quite follow what is
wrong here.

My worry is that by this initialization we can hide but not fix the real issue.
So, I would like to see the complete picture here.

Thanks,
Slava.

> The KMSAN report explicitly shows the uninit-value originates from the
> uninitialized tmp variable and propagates through the code path. The
> initialization ensures that even with corrupted filesystem images, we use
> zeros instead of random stack data.
> 
> Syzbot has confirmed that the fix resolves the issue:
> https://syzkaller.appspot.com/bug?extid=d80abb5b890d39261e72  
> 
> I will update the patch to use = {0} initialization as you suggested.
> 
> Thanks,
> Deepanshu

[1]
https://elixir.bootlin.com/linux/v6.19-rc5/source/include/linux/hfs_common.h#L571
[2]
https://elixir.bootlin.com/linux/v6.19-rc5/source/include/linux/hfs_common.h#L155