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: <CADhLXY6WTN1gTYZ72_GvMyS2RJArX=6-h5-NmpwBGRU_m5FjQA@mail.gmail.com>
Date: Wed, 28 Jan 2026 09:59:07 +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 Wed, Jan 28, 2026 at 4:51 AM Viacheslav Dubeyko
<Slava.Dubeyko@....com> wrote:
> As far as I can see, hfs_brec_read() mostly used for operations with Catalog
> File. And we always read hfsplus_cat_entry union. And you can see that it starts
> from type field.
>
> typedef union {
>         __be16 type;
>         struct hfsplus_cat_folder folder;
>         struct hfsplus_cat_file file;
>         struct hfsplus_cat_thread thread;
> } __packed hfsplus_cat_entry;
>
> So, you can use this field to make a decision which type of record is under
> check. So, I think we need to implement generic logic, anyway.
>
> Thanks,
> Slava.
>
> [1] https://elixir.bootlin.com/linux/v6.19-rc5/source/fs/hfsplus/super.c#L570
> [2] https://elixir.bootlin.com/linux/v6.19-rc5/source/fs/hfsplus/dir.c#L52
> [3] https://elixir.bootlin.com/linux/v6.19-rc5/source/fs/hfsplus/catalog.c#L202


Hi Slava,

Thank you for the guidance! You're right - we need a generic solution in
hfs_brec_read() that validates based on the actual record type read.

Here's my understanding of the approach:

---

int hfs_brec_read(struct hfs_find_data *fd, void *rec, u32 rec_len)
{
int res;
u16 type;
u32 min_size;
hfsplus_cat_entry *entry = rec;

res = hfs_brec_find(fd, hfs_find_rec_by_key);
if (res)
return res;
if (fd->entrylength > rec_len)
return -EINVAL;
hfs_bnode_read(fd->bnode, rec, fd->entryoffset, fd->entrylength);
++ /* Validate based on record type */
++ type = be16_to_cpu(entry->type);
++
++ switch (type) {
++ case HFSPLUS_FOLDER:
++ min_size = sizeof(struct hfsplus_cat_folder);
++ break;
++ case HFSPLUS_FILE:
++ min_size = sizeof(struct hfsplus_cat_file);
++ break;
++ case HFSPLUS_FOLDER_THREAD:
++ case HFSPLUS_FILE_THREAD:
++ /* For threads, size depends on string length */
++ min_size = offsetof(hfsplus_cat_entry, thread.nodeName) +
++    offsetof(struct hfsplus_unistr, unicode) +
++    be16_to_cpu(entry->thread.nodeName.length) * 2;
++ break;
++ default:
++ pr_err("unknown catalog record type %d\n", type);
++ return -EIO;
++ }
++
++ if (fd->entrylength < min_size) {
++ pr_err("incomplete record read (type %d, got %u, need %u)\n",
++        type, fd->entrylength, min_size);
++ return -EIO;
++ }
return 0;
}

And in hfsplus_find_cat():

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;
hfsplus_cat_build_key_with_cnid(sb, fd->search_key, cnid);
err = hfs_brec_read(fd, &tmp, sizeof(hfsplus_cat_entry));
if (err)
return err;
/* hfs_brec_read() already validated the record */
...
}

---

This way:
1. Generic validation in hfs_brec_read() using the type field
2. Works for all callers (folder, file, thread records)
3. Variable-size validation for thread records based on string length
4. Fixed-size validation for folder and file records
5. Initialize tmp = {0} as defensive programming

Does this approach look correct? Should I handle any other record types in
the switch statement?

Thanks,
Deepanshu