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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADhLXY54yiFoqGghDQ9=p7PQXSo7caJ17pBrGS3Ck3uuRDOB5A@mail.gmail.com>
Date: Tue, 27 Jan 2026 13:50:50 +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 Tue, Jan 27, 2026 at 2:07 AM Viacheslav Dubeyko
<Slava.Dubeyko@....com> wrote:
>


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

Hi Slava,

Thank you for the feedback! You're absolutely right - using != would break
callers that read unions with different-sized members.

Instead of validating in hfs_brec_read() (which is generic), I should validate
specifically in hfsplus_find_cat() where we know we're reading a thread record.

Here's the corrected approach:

---

int hfsplus_find_cat(struct super_block *sb, u32 cnid,
                     struct hfs_find_data *fd)
{
        hfsplus_cat_entry tmp = {0};
        int err;
        u16 type;
        u32 min_size;

        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;

        type = be16_to_cpu(tmp.type);
        if (type != HFSPLUS_FOLDER_THREAD && type != HFSPLUS_FILE_THREAD) {
                pr_err("found bad thread record in catalog\n");
                return -EIO;
        }

++      /* Validate we read a complete thread record */
++      min_size = offsetof(hfsplus_cat_entry, thread.nodeName) +
++                 offsetof(struct hfsplus_unistr, unicode) +
++                 be16_to_cpu(tmp.thread.nodeName.length) * 2;
++      if (fd->entrylength < min_size) {
++              pr_err("incomplete thread record read (got %u, need %u)\n",
++                     fd->entrylength, min_size);
++              return -EIO;
++      }

        if (be16_to_cpu(tmp.thread.nodeName.length) > 255) {
                pr_err("catalog name length corrupted\n");
                return -EIO;
        }

        hfsplus_cat_build_key_uni(fd->search_key,
                be32_to_cpu(tmp.thread.parentID),
                &tmp.thread.nodeName);
        return hfs_brec_find(fd, hfs_find_rec_by_key);
}

---

This way:
1. hfs_brec_read() remains generic (doesn't break other callers)
2. We validate specifically for thread records where we know the
expected structure
3. We calculate minimum required size based on the string length the
record claims
4. We initialize tmp = {0} as defensive programming

Does this look correct?

Thanks,
Deepanshu