linux-kernel - Re: [PATCH] hfsplus: fix uninit-value in hfsplus

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CADhLXY6wFsspQMe0C4BNRsmKn2LaPaBFfOh1T+OBibuZVSo70g@mail.gmail.com>
Date: Fri, 23 Jan 2026 07:18:37 +0530
From: Deepanshu Kartikey <kartikey406@...il.com>
To: Viacheslav Dubeyko <Slava.Dubeyko@....com>
Cc: "glaubitz@...sik.fu-berlin.de" <glaubitz@...sik.fu-berlin.de>, "frank.li@...o.com" <frank.li@...o.com>, 
	"slava@...eyko.com" <slava@...eyko.com>, 
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"syzbot+d80abb5b890d39261e72@...kaller.appspotmail.com" <syzbot+d80abb5b890d39261e72@...kaller.appspotmail.com>
Subject: Re: [PATCH] hfsplus: fix uninit-value in hfsplus_strcasecmp

On Thu, Jan 22, 2026 at 1:41 AM Viacheslav Dubeyko
<Slava.Dubeyko@....com> wrote:
>
> 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.
>

Hi Slava,

Thank you for pushing me to investigate deeper. I added debug printk to
hfs_brec_read() and tested with syzbot. Here's the concrete evidence:

HFSPLUS_BREC_READ: rec_len=520, fd->entrylength=26
HFSPLUS_BREC_READ: WARNING - entrylength (26) < rec_len (520) - PARTIAL READ!
HFSPLUS_BREC_READ: Successfully read 26 bytes (expected 520)

So the exact scenario is:
1. hfsplus_find_cat() calls hfs_brec_read(&tmp, 520)
2. The corrupted b-tree node has fd->entrylength = 26 bytes
3. hfs_brec_read() checks: if (26 > 520) - FALSE, continues
4. Reads only 26 bytes into tmp
5. Returns 0 (success!)
6. Bytes 0-25 are filled, bytes 26-519 remain uninitialized
7. tmp.thread.nodeName contains uninitialized data
8. KMSAN detects this when hfsplus_strcasecmp() uses it

You were absolutely right to question this. The real issues are:
a) hfs_brec_read() doesn't validate minimum expected size
b) hfsplus_find_cat() doesn't validate the read was complete

However, initializing tmp is still necessary as defensive programming - even
with better validation, we shouldn't have uninitialized kernel stack data in
filesystem structures.

Would you prefer:
1. Current patch (initialize tmp) + separate patch to add validation?
2. Combined patch with both initialization and validation?

I can prepare whichever you think is better.

Thanks for your guidance in understanding this properly!

Deepanshu