* [PATCH v4] hfsplus: fix uninit-value by validating catalog record size
@ 2026-02-14 0:21 Deepanshu Kartikey
2026-02-16 2:15 ` Charalampos Mitrodimas
2026-02-17 0:19 ` Viacheslav Dubeyko
0 siblings, 2 replies; 6+ messages in thread
From: Deepanshu Kartikey @ 2026-02-14 0:21 UTC (permalink / raw)
To: slava, glaubitz, frank.li
Cc: linux-fsdevel, linux-kernel, Deepanshu Kartikey,
syzbot+d80abb5b890d39261e72
Syzbot reported a KMSAN uninit-value issue in hfsplus_strcasecmp(). The
root cause is that hfs_brec_read() doesn't validate that the on-disk
record size matches the expected size for the record type being read.
When mounting a corrupted filesystem, hfs_brec_read() may read less data
than expected. For example, when reading a catalog thread record, the
debug output showed:
HFSPLUS_BREC_READ: rec_len=520, fd->entrylength=26
HFSPLUS_BREC_READ: WARNING - entrylength (26) < rec_len (520) - PARTIAL READ!
hfs_brec_read() only validates that entrylength is not greater than the
buffer size, but doesn't check if it's less than expected. It successfully
reads 26 bytes into a 520-byte structure and returns success, leaving 494
bytes uninitialized.
This uninitialized data in tmp.thread.nodeName then gets copied by
hfsplus_cat_build_key_uni() and used by hfsplus_strcasecmp(), triggering
the KMSAN warning when the uninitialized bytes are used as array indices
in case_fold().
Fix by introducing hfsplus_brec_read_cat() wrapper that:
1. Calls hfs_brec_read() to read the data
2. Validates the record size based on the type field:
- Fixed size for folder and file records
- Variable size for thread records (depends on string length)
3. Returns -EIO if size doesn't match expected
Also initialize the tmp variable in hfsplus_find_cat() as defensive
programming to ensure no uninitialized data even if validation is
bypassed.
Reported-by: syzbot+d80abb5b890d39261e72@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=d80abb5b890d39261e72
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Tested-by: syzbot+d80abb5b890d39261e72@syzkaller.appspotmail.com
Link: https://lore.kernel.org/all/20260120051114.1281285-1-kartikey406@gmail.com/ [v1]
Link: https://lore.kernel.org/all/20260121063109.1830263-1-kartikey406@gmail.com/ [v2]
Link: https://lore.kernel.org/all/20260212014233.2422046-1-kartikey406@gmail.com/ [v3]
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
Changes in v4:
- Move hfsplus_cat_thread_size() as static inline to header file as
suggested by Viacheslav Dubeyko
Changes in v3:
- Introduced hfsplus_brec_read_cat() wrapper function for catalog-specific
validation instead of modifying generic hfs_brec_read()
- Added hfsplus_cat_thread_size() helper to calculate variable-size thread
record sizes
- Use exact size match (!=) instead of minimum size check (<)
- Use sizeof(hfsplus_unichr) instead of hardcoded value 2
- Updated all catalog record read sites to use new wrapper function
- Addressed review feedback from Viacheslav Dubeyko
Changes in v2:
- Use structure initialization (= {0}) instead of memset()
- Improved commit message to clarify how uninitialized data is used
---
fs/hfsplus/bfind.c | 46 +++++++++++++++++++++++++++++++++++++++++
fs/hfsplus/catalog.c | 4 ++--
fs/hfsplus/dir.c | 2 +-
fs/hfsplus/hfsplus_fs.h | 9 ++++++++
fs/hfsplus/super.c | 2 +-
5 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/fs/hfsplus/bfind.c b/fs/hfsplus/bfind.c
index 9b89dce00ee9..4c5fd21585ef 100644
--- a/fs/hfsplus/bfind.c
+++ b/fs/hfsplus/bfind.c
@@ -297,3 +297,49 @@ int hfs_brec_goto(struct hfs_find_data *fd, int cnt)
fd->bnode = bnode;
return res;
}
+
+/**
+ * hfsplus_brec_read_cat - read and validate a catalog record
+ * @fd: find data structure
+ * @entry: pointer to catalog entry to read into
+ *
+ * Reads a catalog record and validates its size matches the expected
+ * size based on the record type.
+ *
+ * Returns 0 on success, or negative error code on failure.
+ */
+int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry)
+{
+ int res;
+ u32 expected_size;
+
+ res = hfs_brec_read(fd, entry, sizeof(hfsplus_cat_entry));
+ if (res)
+ return res;
+
+ /* Validate catalog record size based on type */
+ switch (be16_to_cpu(entry->type)) {
+ case HFSPLUS_FOLDER:
+ expected_size = sizeof(struct hfsplus_cat_folder);
+ break;
+ case HFSPLUS_FILE:
+ expected_size = sizeof(struct hfsplus_cat_file);
+ break;
+ case HFSPLUS_FOLDER_THREAD:
+ case HFSPLUS_FILE_THREAD:
+ expected_size = hfsplus_cat_thread_size(&entry->thread);
+ break;
+ default:
+ pr_err("unknown catalog record type %d\n",
+ be16_to_cpu(entry->type));
+ return -EIO;
+ }
+
+ if (fd->entrylength != expected_size) {
+ pr_err("catalog record size mismatch (type %d, got %u, expected %u)\n",
+ be16_to_cpu(entry->type), fd->entrylength, expected_size);
+ return -EIO;
+ }
+
+ return 0;
+}
diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c
index 02c1eee4a4b8..6c8380f7208d 100644
--- a/fs/hfsplus/catalog.c
+++ b/fs/hfsplus/catalog.c
@@ -194,12 +194,12 @@ static int hfsplus_fill_cat_thread(struct super_block *sb,
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));
+ err = hfsplus_brec_read_cat(fd, &tmp);
if (err)
return err;
diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index cadf0b5f9342..d86e2f7b289c 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -49,7 +49,7 @@ static struct dentry *hfsplus_lookup(struct inode *dir, struct dentry *dentry,
if (unlikely(err < 0))
goto fail;
again:
- err = hfs_brec_read(&fd, &entry, sizeof(entry));
+ err = hfsplus_brec_read_cat(&fd, &entry);
if (err) {
if (err == -ENOENT) {
hfs_find_exit(&fd);
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 45fe3a12ecba..e811d33861af 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -506,6 +506,15 @@ int hfsplus_submit_bio(struct super_block *sb, sector_t sector, void *buf,
void **data, blk_opf_t opf);
int hfsplus_read_wrapper(struct super_block *sb);
+static inline u32 hfsplus_cat_thread_size(const struct hfsplus_cat_thread *thread)
+{
+ return offsetof(struct hfsplus_cat_thread, nodeName) +
+ offsetof(struct hfsplus_unistr, unicode) +
+ be16_to_cpu(thread->nodeName.length) * sizeof(hfsplus_unichr);
+}
+
+int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry);
+
/*
* time helpers: convert between 1904-base and 1970-base timestamps
*
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index aaffa9e060a0..e59611a664ef 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -567,7 +567,7 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
err = hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, &str);
if (unlikely(err < 0))
goto out_put_root;
- if (!hfs_brec_read(&fd, &entry, sizeof(entry))) {
+ if (!hfsplus_brec_read_cat(&fd, &entry)) {
hfs_find_exit(&fd);
if (entry.type != cpu_to_be16(HFSPLUS_FOLDER)) {
err = -EIO;
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v4] hfsplus: fix uninit-value by validating catalog record size
2026-02-14 0:21 [PATCH v4] hfsplus: fix uninit-value by validating catalog record size Deepanshu Kartikey
@ 2026-02-16 2:15 ` Charalampos Mitrodimas
2026-02-17 0:13 ` Viacheslav Dubeyko
2026-02-17 0:19 ` Viacheslav Dubeyko
1 sibling, 1 reply; 6+ messages in thread
From: Charalampos Mitrodimas @ 2026-02-16 2:15 UTC (permalink / raw)
To: Deepanshu Kartikey
Cc: slava, glaubitz, frank.li, linux-fsdevel, linux-kernel,
syzbot+d80abb5b890d39261e72
Deepanshu Kartikey <kartikey406@gmail.com> writes:
> Syzbot reported a KMSAN uninit-value issue in hfsplus_strcasecmp(). The
> root cause is that hfs_brec_read() doesn't validate that the on-disk
> record size matches the expected size for the record type being read.
>
> When mounting a corrupted filesystem, hfs_brec_read() may read less data
> than expected. For example, when reading a catalog thread record, the
> debug output showed:
>
> HFSPLUS_BREC_READ: rec_len=520, fd->entrylength=26
> HFSPLUS_BREC_READ: WARNING - entrylength (26) < rec_len (520) - PARTIAL READ!
>
> hfs_brec_read() only validates that entrylength is not greater than the
> buffer size, but doesn't check if it's less than expected. It successfully
> reads 26 bytes into a 520-byte structure and returns success, leaving 494
> bytes uninitialized.
>
> This uninitialized data in tmp.thread.nodeName then gets copied by
> hfsplus_cat_build_key_uni() and used by hfsplus_strcasecmp(), triggering
> the KMSAN warning when the uninitialized bytes are used as array indices
> in case_fold().
>
> Fix by introducing hfsplus_brec_read_cat() wrapper that:
> 1. Calls hfs_brec_read() to read the data
> 2. Validates the record size based on the type field:
> - Fixed size for folder and file records
> - Variable size for thread records (depends on string length)
> 3. Returns -EIO if size doesn't match expected
>
> Also initialize the tmp variable in hfsplus_find_cat() as defensive
> programming to ensure no uninitialized data even if validation is
> bypassed.
>
> Reported-by: syzbot+d80abb5b890d39261e72@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=d80abb5b890d39261e72
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Tested-by: syzbot+d80abb5b890d39261e72@syzkaller.appspotmail.com
> Link: https://lore.kernel.org/all/20260120051114.1281285-1-kartikey406@gmail.com/ [v1]
> Link: https://lore.kernel.org/all/20260121063109.1830263-1-kartikey406@gmail.com/ [v2]
> Link: https://lore.kernel.org/all/20260212014233.2422046-1-kartikey406@gmail.com/ [v3]
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
> Changes in v4:
> - Move hfsplus_cat_thread_size() as static inline to header file as
> suggested by Viacheslav Dubeyko
>
> Changes in v3:
> - Introduced hfsplus_brec_read_cat() wrapper function for catalog-specific
> validation instead of modifying generic hfs_brec_read()
> - Added hfsplus_cat_thread_size() helper to calculate variable-size thread
> record sizes
> - Use exact size match (!=) instead of minimum size check (<)
> - Use sizeof(hfsplus_unichr) instead of hardcoded value 2
> - Updated all catalog record read sites to use new wrapper function
> - Addressed review feedback from Viacheslav Dubeyko
>
> Changes in v2:
> - Use structure initialization (= {0}) instead of memset()
> - Improved commit message to clarify how uninitialized data is used
> ---
> fs/hfsplus/bfind.c | 46 +++++++++++++++++++++++++++++++++++++++++
> fs/hfsplus/catalog.c | 4 ++--
> fs/hfsplus/dir.c | 2 +-
> fs/hfsplus/hfsplus_fs.h | 9 ++++++++
> fs/hfsplus/super.c | 2 +-
> 5 files changed, 59 insertions(+), 4 deletions(-)
>
> diff --git a/fs/hfsplus/bfind.c b/fs/hfsplus/bfind.c
> index 9b89dce00ee9..4c5fd21585ef 100644
> --- a/fs/hfsplus/bfind.c
> +++ b/fs/hfsplus/bfind.c
> @@ -297,3 +297,49 @@ int hfs_brec_goto(struct hfs_find_data *fd, int cnt)
> fd->bnode = bnode;
> return res;
> }
> +
> +/**
> + * hfsplus_brec_read_cat - read and validate a catalog record
> + * @fd: find data structure
> + * @entry: pointer to catalog entry to read into
> + *
> + * Reads a catalog record and validates its size matches the expected
> + * size based on the record type.
> + *
> + * Returns 0 on success, or negative error code on failure.
> + */
> +int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry)
> +{
> + int res;
> + u32 expected_size;
> +
> + res = hfs_brec_read(fd, entry, sizeof(hfsplus_cat_entry));
> + if (res)
> + return res;
> +
> + /* Validate catalog record size based on type */
> + switch (be16_to_cpu(entry->type)) {
> + case HFSPLUS_FOLDER:
> + expected_size = sizeof(struct hfsplus_cat_folder);
> + break;
> + case HFSPLUS_FILE:
> + expected_size = sizeof(struct hfsplus_cat_file);
> + break;
> + case HFSPLUS_FOLDER_THREAD:
> + case HFSPLUS_FILE_THREAD:
> + expected_size = hfsplus_cat_thread_size(&entry->thread);
Should we check fd->entrylength < HFSPLUS_MIN_THREAD_SZ here before
calling hfsplus_cat_thread_size(), so we don't read uninitialized
nodeName.length at call sites that don't zero-initialize entry?
hfsplus_readdir() already does this check for thread records.
Cheers,
C. Mitrodimas
> + break;
> + default:
> + pr_err("unknown catalog record type %d\n",
> + be16_to_cpu(entry->type));
> + return -EIO;
> + }
> +
> + if (fd->entrylength != expected_size) {
> + pr_err("catalog record size mismatch (type %d, got %u, expected %u)\n",
> + be16_to_cpu(entry->type), fd->entrylength, expected_size);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c
> index 02c1eee4a4b8..6c8380f7208d 100644
> --- a/fs/hfsplus/catalog.c
> +++ b/fs/hfsplus/catalog.c
> @@ -194,12 +194,12 @@ static int hfsplus_fill_cat_thread(struct super_block *sb,
> 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));
> + err = hfsplus_brec_read_cat(fd, &tmp);
> if (err)
> return err;
>
> diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
> index cadf0b5f9342..d86e2f7b289c 100644
> --- a/fs/hfsplus/dir.c
> +++ b/fs/hfsplus/dir.c
> @@ -49,7 +49,7 @@ static struct dentry *hfsplus_lookup(struct inode *dir, struct dentry *dentry,
> if (unlikely(err < 0))
> goto fail;
> again:
> - err = hfs_brec_read(&fd, &entry, sizeof(entry));
> + err = hfsplus_brec_read_cat(&fd, &entry);
> if (err) {
> if (err == -ENOENT) {
> hfs_find_exit(&fd);
> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> index 45fe3a12ecba..e811d33861af 100644
> --- a/fs/hfsplus/hfsplus_fs.h
> +++ b/fs/hfsplus/hfsplus_fs.h
> @@ -506,6 +506,15 @@ int hfsplus_submit_bio(struct super_block *sb, sector_t sector, void *buf,
> void **data, blk_opf_t opf);
> int hfsplus_read_wrapper(struct super_block *sb);
>
> +static inline u32 hfsplus_cat_thread_size(const struct hfsplus_cat_thread *thread)
> +{
> + return offsetof(struct hfsplus_cat_thread, nodeName) +
> + offsetof(struct hfsplus_unistr, unicode) +
> + be16_to_cpu(thread->nodeName.length) * sizeof(hfsplus_unichr);
> +}
> +
> +int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry);
> +
> /*
> * time helpers: convert between 1904-base and 1970-base timestamps
> *
> diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
> index aaffa9e060a0..e59611a664ef 100644
> --- a/fs/hfsplus/super.c
> +++ b/fs/hfsplus/super.c
> @@ -567,7 +567,7 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
> err = hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, &str);
> if (unlikely(err < 0))
> goto out_put_root;
> - if (!hfs_brec_read(&fd, &entry, sizeof(entry))) {
> + if (!hfsplus_brec_read_cat(&fd, &entry)) {
> hfs_find_exit(&fd);
> if (entry.type != cpu_to_be16(HFSPLUS_FOLDER)) {
> err = -EIO;
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH v4] hfsplus: fix uninit-value by validating catalog record size
2026-02-16 2:15 ` Charalampos Mitrodimas
@ 2026-02-17 0:13 ` Viacheslav Dubeyko
0 siblings, 0 replies; 6+ messages in thread
From: Viacheslav Dubeyko @ 2026-02-17 0:13 UTC (permalink / raw)
To: charmitro@posteo.net, kartikey406@gmail.com
Cc: glaubitz@physik.fu-berlin.de, frank.li@vivo.com,
slava@dubeyko.com, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org,
syzbot+d80abb5b890d39261e72@syzkaller.appspotmail.com
On Mon, 2026-02-16 at 02:15 +0000, Charalampos Mitrodimas wrote:
> Deepanshu Kartikey <kartikey406@gmail.com> writes:
>
> > Syzbot reported a KMSAN uninit-value issue in hfsplus_strcasecmp(). The
> > root cause is that hfs_brec_read() doesn't validate that the on-disk
> > record size matches the expected size for the record type being read.
> >
> > When mounting a corrupted filesystem, hfs_brec_read() may read less data
> > than expected. For example, when reading a catalog thread record, the
> > debug output showed:
> >
> > HFSPLUS_BREC_READ: rec_len=520, fd->entrylength=26
> > HFSPLUS_BREC_READ: WARNING - entrylength (26) < rec_len (520) - PARTIAL READ!
> >
> > hfs_brec_read() only validates that entrylength is not greater than the
> > buffer size, but doesn't check if it's less than expected. It successfully
> > reads 26 bytes into a 520-byte structure and returns success, leaving 494
> > bytes uninitialized.
> >
> > This uninitialized data in tmp.thread.nodeName then gets copied by
> > hfsplus_cat_build_key_uni() and used by hfsplus_strcasecmp(), triggering
> > the KMSAN warning when the uninitialized bytes are used as array indices
> > in case_fold().
> >
> > Fix by introducing hfsplus_brec_read_cat() wrapper that:
> > 1. Calls hfs_brec_read() to read the data
> > 2. Validates the record size based on the type field:
> > - Fixed size for folder and file records
> > - Variable size for thread records (depends on string length)
> > 3. Returns -EIO if size doesn't match expected
> >
> > Also initialize the tmp variable in hfsplus_find_cat() as defensive
> > programming to ensure no uninitialized data even if validation is
> > bypassed.
> >
> > Reported-by: syzbot+d80abb5b890d39261e72@syzkaller.appspotmail.com
> > Closes: https://urldefense.proofpoint.com/v2/url?u=https-3A__syzkaller.appspot.com_bug-3Fextid-3Dd80abb5b890d39261e72&d=DwIBAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=nC0HfdhtxlARLS3_CZ1hmzBHc6jFNK-5cUGbrUjuDbLPDcgRvEFW2wgjZMITubfk&s=LJy4ssHVkAhJ1O6iwvIHnY4XWlRC-EiUdTCwCCS18j0&e=
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Tested-by: syzbot+d80abb5b890d39261e72@syzkaller.appspotmail.com
> > Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_20260120051114.1281285-2D1-2Dkartikey406-40gmail.com_&d=DwIBAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=nC0HfdhtxlARLS3_CZ1hmzBHc6jFNK-5cUGbrUjuDbLPDcgRvEFW2wgjZMITubfk&s=VYf_6vN6_Imd-hafYJbBj62WwnlPNdacx73WDLussWo&e= [v1]
> > Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_20260121063109.1830263-2D1-2Dkartikey406-40gmail.com_&d=DwIBAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=nC0HfdhtxlARLS3_CZ1hmzBHc6jFNK-5cUGbrUjuDbLPDcgRvEFW2wgjZMITubfk&s=qjAt8GxYxwdQndNEWiiBy2NRhLCvDcaXdvAFv7yxAj8&e= [v2]
> > Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_20260212014233.2422046-2D1-2Dkartikey406-40gmail.com_&d=DwIBAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=nC0HfdhtxlARLS3_CZ1hmzBHc6jFNK-5cUGbrUjuDbLPDcgRvEFW2wgjZMITubfk&s=zdyPgw-L5MS2vYOhrVGxHfOfAtjJkjM7nzGw6JJC1GQ&e= [v3]
> > Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> > ---
> > Changes in v4:
> > - Move hfsplus_cat_thread_size() as static inline to header file as
> > suggested by Viacheslav Dubeyko
> >
> > Changes in v3:
> > - Introduced hfsplus_brec_read_cat() wrapper function for catalog-specific
> > validation instead of modifying generic hfs_brec_read()
> > - Added hfsplus_cat_thread_size() helper to calculate variable-size thread
> > record sizes
> > - Use exact size match (!=) instead of minimum size check (<)
> > - Use sizeof(hfsplus_unichr) instead of hardcoded value 2
> > - Updated all catalog record read sites to use new wrapper function
> > - Addressed review feedback from Viacheslav Dubeyko
> >
> > Changes in v2:
> > - Use structure initialization (= {0}) instead of memset()
> > - Improved commit message to clarify how uninitialized data is used
> > ---
> > fs/hfsplus/bfind.c | 46 +++++++++++++++++++++++++++++++++++++++++
> > fs/hfsplus/catalog.c | 4 ++--
> > fs/hfsplus/dir.c | 2 +-
> > fs/hfsplus/hfsplus_fs.h | 9 ++++++++
> > fs/hfsplus/super.c | 2 +-
> > 5 files changed, 59 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/hfsplus/bfind.c b/fs/hfsplus/bfind.c
> > index 9b89dce00ee9..4c5fd21585ef 100644
> > --- a/fs/hfsplus/bfind.c
> > +++ b/fs/hfsplus/bfind.c
> > @@ -297,3 +297,49 @@ int hfs_brec_goto(struct hfs_find_data *fd, int cnt)
> > fd->bnode = bnode;
> > return res;
> > }
> > +
> > +/**
> > + * hfsplus_brec_read_cat - read and validate a catalog record
> > + * @fd: find data structure
> > + * @entry: pointer to catalog entry to read into
> > + *
> > + * Reads a catalog record and validates its size matches the expected
> > + * size based on the record type.
> > + *
> > + * Returns 0 on success, or negative error code on failure.
> > + */
> > +int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry)
> > +{
> > + int res;
> > + u32 expected_size;
> > +
> > + res = hfs_brec_read(fd, entry, sizeof(hfsplus_cat_entry));
> > + if (res)
> > + return res;
> > +
> > + /* Validate catalog record size based on type */
> > + switch (be16_to_cpu(entry->type)) {
> > + case HFSPLUS_FOLDER:
> > + expected_size = sizeof(struct hfsplus_cat_folder);
> > + break;
> > + case HFSPLUS_FILE:
> > + expected_size = sizeof(struct hfsplus_cat_file);
> > + break;
> > + case HFSPLUS_FOLDER_THREAD:
> > + case HFSPLUS_FILE_THREAD:
> > + expected_size = hfsplus_cat_thread_size(&entry->thread);
>
> Should we check fd->entrylength < HFSPLUS_MIN_THREAD_SZ here before
> calling hfsplus_cat_thread_size(), so we don't read uninitialized
> nodeName.length at call sites that don't zero-initialize entry?
> hfsplus_readdir() already does this check for thread records.
>
I think that it makes sense. It sounds like a good suggestion.
Thanks,
Slava.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] hfsplus: fix uninit-value by validating catalog record size
2026-02-14 0:21 [PATCH v4] hfsplus: fix uninit-value by validating catalog record size Deepanshu Kartikey
2026-02-16 2:15 ` Charalampos Mitrodimas
@ 2026-02-17 0:19 ` Viacheslav Dubeyko
2026-02-17 23:16 ` Viacheslav Dubeyko
1 sibling, 1 reply; 6+ messages in thread
From: Viacheslav Dubeyko @ 2026-02-17 0:19 UTC (permalink / raw)
To: glaubitz@physik.fu-berlin.de, frank.li@vivo.com,
slava@dubeyko.com, kartikey406@gmail.com
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
syzbot+d80abb5b890d39261e72@syzkaller.appspotmail.com
On Sat, 2026-02-14 at 05:51 +0530, Deepanshu Kartikey wrote:
> Syzbot reported a KMSAN uninit-value issue in hfsplus_strcasecmp(). The
> root cause is that hfs_brec_read() doesn't validate that the on-disk
> record size matches the expected size for the record type being read.
>
> When mounting a corrupted filesystem, hfs_brec_read() may read less data
> than expected. For example, when reading a catalog thread record, the
> debug output showed:
>
> HFSPLUS_BREC_READ: rec_len=520, fd->entrylength=26
> HFSPLUS_BREC_READ: WARNING - entrylength (26) < rec_len (520) - PARTIAL READ!
>
> hfs_brec_read() only validates that entrylength is not greater than the
> buffer size, but doesn't check if it's less than expected. It successfully
> reads 26 bytes into a 520-byte structure and returns success, leaving 494
> bytes uninitialized.
>
> This uninitialized data in tmp.thread.nodeName then gets copied by
> hfsplus_cat_build_key_uni() and used by hfsplus_strcasecmp(), triggering
> the KMSAN warning when the uninitialized bytes are used as array indices
> in case_fold().
>
> Fix by introducing hfsplus_brec_read_cat() wrapper that:
> 1. Calls hfs_brec_read() to read the data
> 2. Validates the record size based on the type field:
> - Fixed size for folder and file records
> - Variable size for thread records (depends on string length)
> 3. Returns -EIO if size doesn't match expected
>
> Also initialize the tmp variable in hfsplus_find_cat() as defensive
> programming to ensure no uninitialized data even if validation is
> bypassed.
>
> Reported-by: syzbot+d80abb5b890d39261e72@syzkaller.appspotmail.com
> Closes: https://urldefense.proofpoint.com/v2/url?u=https-3A__syzkaller.appspot.com_bug-3Fextid-3Dd80abb5b890d39261e72&d=DwIDAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=HsA3_4uwJKmSS8-Kh_KSSNZDdIJJ37fQg1Qkn8HNVtlvOWTRUotWdX-YU-6BG5Ek&s=ahSiNp09Xtk6fZ4J1HKNVIKGYMyAAB6pB0zSMMLuiDY&e=
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Tested-by: syzbot+d80abb5b890d39261e72@syzkaller.appspotmail.com
> Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_20260120051114.1281285-2D1-2Dkartikey406-40gmail.com_&d=DwIDAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=HsA3_4uwJKmSS8-Kh_KSSNZDdIJJ37fQg1Qkn8HNVtlvOWTRUotWdX-YU-6BG5Ek&s=m5KATSecj_N59jRyckqWwTDHLWsAk-SA4McFiUTpyHg&e= [v1]
> Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_20260121063109.1830263-2D1-2Dkartikey406-40gmail.com_&d=DwIDAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=HsA3_4uwJKmSS8-Kh_KSSNZDdIJJ37fQg1Qkn8HNVtlvOWTRUotWdX-YU-6BG5Ek&s=Ktxd-8E848Ko3SKJ-hZ2_Voa5CBICSaYmGY1i9FnL6M&e= [v2]
> Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_20260212014233.2422046-2D1-2Dkartikey406-40gmail.com_&d=DwIDAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=HsA3_4uwJKmSS8-Kh_KSSNZDdIJJ37fQg1Qkn8HNVtlvOWTRUotWdX-YU-6BG5Ek&s=KcMP_3KY8SuKRFhhCrjaKfUJ9Ce4nbh_WuuYl-4dyvU&e= [v3]
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
> Changes in v4:
> - Move hfsplus_cat_thread_size() as static inline to header file as
> suggested by Viacheslav Dubeyko
>
> Changes in v3:
> - Introduced hfsplus_brec_read_cat() wrapper function for catalog-specific
> validation instead of modifying generic hfs_brec_read()
> - Added hfsplus_cat_thread_size() helper to calculate variable-size thread
> record sizes
> - Use exact size match (!=) instead of minimum size check (<)
> - Use sizeof(hfsplus_unichr) instead of hardcoded value 2
> - Updated all catalog record read sites to use new wrapper function
> - Addressed review feedback from Viacheslav Dubeyko
>
> Changes in v2:
> - Use structure initialization (= {0}) instead of memset()
> - Improved commit message to clarify how uninitialized data is used
> ---
> fs/hfsplus/bfind.c | 46 +++++++++++++++++++++++++++++++++++++++++
> fs/hfsplus/catalog.c | 4 ++--
> fs/hfsplus/dir.c | 2 +-
> fs/hfsplus/hfsplus_fs.h | 9 ++++++++
> fs/hfsplus/super.c | 2 +-
> 5 files changed, 59 insertions(+), 4 deletions(-)
>
> diff --git a/fs/hfsplus/bfind.c b/fs/hfsplus/bfind.c
> index 9b89dce00ee9..4c5fd21585ef 100644
> --- a/fs/hfsplus/bfind.c
> +++ b/fs/hfsplus/bfind.c
> @@ -297,3 +297,49 @@ int hfs_brec_goto(struct hfs_find_data *fd, int cnt)
> fd->bnode = bnode;
> return res;
> }
> +
> +/**
> + * hfsplus_brec_read_cat - read and validate a catalog record
> + * @fd: find data structure
> + * @entry: pointer to catalog entry to read into
> + *
> + * Reads a catalog record and validates its size matches the expected
> + * size based on the record type.
> + *
> + * Returns 0 on success, or negative error code on failure.
> + */
> +int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry)
> +{
> + int res;
> + u32 expected_size;
> +
> + res = hfs_brec_read(fd, entry, sizeof(hfsplus_cat_entry));
> + if (res)
> + return res;
> +
> + /* Validate catalog record size based on type */
> + switch (be16_to_cpu(entry->type)) {
> + case HFSPLUS_FOLDER:
> + expected_size = sizeof(struct hfsplus_cat_folder);
> + break;
> + case HFSPLUS_FILE:
> + expected_size = sizeof(struct hfsplus_cat_file);
> + break;
> + case HFSPLUS_FOLDER_THREAD:
> + case HFSPLUS_FILE_THREAD:
> + expected_size = hfsplus_cat_thread_size(&entry->thread);
> + break;
> + default:
> + pr_err("unknown catalog record type %d\n",
> + be16_to_cpu(entry->type));
> + return -EIO;
> + }
> +
> + if (fd->entrylength != expected_size) {
> + pr_err("catalog record size mismatch (type %d, got %u, expected %u)\n",
> + be16_to_cpu(entry->type), fd->entrylength, expected_size);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c
> index 02c1eee4a4b8..6c8380f7208d 100644
> --- a/fs/hfsplus/catalog.c
> +++ b/fs/hfsplus/catalog.c
> @@ -194,12 +194,12 @@ static int hfsplus_fill_cat_thread(struct super_block *sb,
> 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));
> + err = hfsplus_brec_read_cat(fd, &tmp);
> if (err)
> return err;
>
> diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
> index cadf0b5f9342..d86e2f7b289c 100644
> --- a/fs/hfsplus/dir.c
> +++ b/fs/hfsplus/dir.c
> @@ -49,7 +49,7 @@ static struct dentry *hfsplus_lookup(struct inode *dir, struct dentry *dentry,
> if (unlikely(err < 0))
> goto fail;
> again:
> - err = hfs_brec_read(&fd, &entry, sizeof(entry));
> + err = hfsplus_brec_read_cat(&fd, &entry);
> if (err) {
> if (err == -ENOENT) {
> hfs_find_exit(&fd);
> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> index 45fe3a12ecba..e811d33861af 100644
> --- a/fs/hfsplus/hfsplus_fs.h
> +++ b/fs/hfsplus/hfsplus_fs.h
> @@ -506,6 +506,15 @@ int hfsplus_submit_bio(struct super_block *sb, sector_t sector, void *buf,
> void **data, blk_opf_t opf);
> int hfsplus_read_wrapper(struct super_block *sb);
>
> +static inline u32 hfsplus_cat_thread_size(const struct hfsplus_cat_thread *thread)
> +{
> + return offsetof(struct hfsplus_cat_thread, nodeName) +
> + offsetof(struct hfsplus_unistr, unicode) +
> + be16_to_cpu(thread->nodeName.length) * sizeof(hfsplus_unichr);
> +}
> +
> +int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry);
> +
> /*
> * time helpers: convert between 1904-base and 1970-base timestamps
> *
> diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
> index aaffa9e060a0..e59611a664ef 100644
> --- a/fs/hfsplus/super.c
> +++ b/fs/hfsplus/super.c
> @@ -567,7 +567,7 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
> err = hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, &str);
> if (unlikely(err < 0))
> goto out_put_root;
> - if (!hfs_brec_read(&fd, &entry, sizeof(entry))) {
> + if (!hfsplus_brec_read_cat(&fd, &entry)) {
> hfs_find_exit(&fd);
> if (entry.type != cpu_to_be16(HFSPLUS_FOLDER)) {
> err = -EIO;
The patch looks good. I don't have any remarks. I think that we've received
pretty good suggestion how the patch could be improved. Could you improve the
patch? :)
I assume that we have the same issue for HFS case. Frankly speaking, I am
considering to make the b-tree functionality generic one for HFS/HFS+ because
there are multiple similarities in this logic. Would you like to try to
generalize this b-tree functionality in the form of libhfs or something like
this?
Thanks,
Slava.
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH v4] hfsplus: fix uninit-value by validating catalog record size
2026-02-17 0:19 ` Viacheslav Dubeyko
@ 2026-02-17 23:16 ` Viacheslav Dubeyko
2026-02-21 4:44 ` Deepanshu Kartikey
0 siblings, 1 reply; 6+ messages in thread
From: Viacheslav Dubeyko @ 2026-02-17 23:16 UTC (permalink / raw)
To: glaubitz@physik.fu-berlin.de, frank.li@vivo.com,
slava@dubeyko.com, kartikey406@gmail.com
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
syzbot+d80abb5b890d39261e72@syzkaller.appspotmail.com
On Tue, 2026-02-17 at 00:19 +0000, Viacheslav Dubeyko wrote:
> On Sat, 2026-02-14 at 05:51 +0530, Deepanshu Kartikey wrote:
> > Syzbot reported a KMSAN uninit-value issue in hfsplus_strcasecmp(). The
> > root cause is that hfs_brec_read() doesn't validate that the on-disk
> > record size matches the expected size for the record type being read.
> >
> > When mounting a corrupted filesystem, hfs_brec_read() may read less data
> > than expected. For example, when reading a catalog thread record, the
> > debug output showed:
> >
> > HFSPLUS_BREC_READ: rec_len=520, fd->entrylength=26
> > HFSPLUS_BREC_READ: WARNING - entrylength (26) < rec_len (520) - PARTIAL READ!
> >
> > hfs_brec_read() only validates that entrylength is not greater than the
> > buffer size, but doesn't check if it's less than expected. It successfully
> > reads 26 bytes into a 520-byte structure and returns success, leaving 494
> > bytes uninitialized.
> >
> > This uninitialized data in tmp.thread.nodeName then gets copied by
> > hfsplus_cat_build_key_uni() and used by hfsplus_strcasecmp(), triggering
> > the KMSAN warning when the uninitialized bytes are used as array indices
> > in case_fold().
> >
> > Fix by introducing hfsplus_brec_read_cat() wrapper that:
> > 1. Calls hfs_brec_read() to read the data
> > 2. Validates the record size based on the type field:
> > - Fixed size for folder and file records
> > - Variable size for thread records (depends on string length)
> > 3. Returns -EIO if size doesn't match expected
> >
> > Also initialize the tmp variable in hfsplus_find_cat() as defensive
> > programming to ensure no uninitialized data even if validation is
> > bypassed.
> >
> > Reported-by: syzbot+d80abb5b890d39261e72@syzkaller.appspotmail.com
> > Closes: https://urldefense.proofpoint.com/v2/url?u=https-3A__syzkaller.appspot.com_bug-3Fextid-3Dd80abb5b890d39261e72&d=DwIDAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=HsA3_4uwJKmSS8-Kh_KSSNZDdIJJ37fQg1Qkn8HNVtlvOWTRUotWdX-YU-6BG5Ek&s=ahSiNp09Xtk6fZ4J1HKNVIKGYMyAAB6pB0zSMMLuiDY&e=
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Tested-by: syzbot+d80abb5b890d39261e72@syzkaller.appspotmail.com
> > Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_20260120051114.1281285-2D1-2Dkartikey406-40gmail.com_&d=DwIDAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=HsA3_4uwJKmSS8-Kh_KSSNZDdIJJ37fQg1Qkn8HNVtlvOWTRUotWdX-YU-6BG5Ek&s=m5KATSecj_N59jRyckqWwTDHLWsAk-SA4McFiUTpyHg&e= [v1]
> > Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_20260121063109.1830263-2D1-2Dkartikey406-40gmail.com_&d=DwIDAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=HsA3_4uwJKmSS8-Kh_KSSNZDdIJJ37fQg1Qkn8HNVtlvOWTRUotWdX-YU-6BG5Ek&s=Ktxd-8E848Ko3SKJ-hZ2_Voa5CBICSaYmGY1i9FnL6M&e= [v2]
> > Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_20260212014233.2422046-2D1-2Dkartikey406-40gmail.com_&d=DwIDAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=HsA3_4uwJKmSS8-Kh_KSSNZDdIJJ37fQg1Qkn8HNVtlvOWTRUotWdX-YU-6BG5Ek&s=KcMP_3KY8SuKRFhhCrjaKfUJ9Ce4nbh_WuuYl-4dyvU&e= [v3]
> > Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> > ---
> > Changes in v4:
> > - Move hfsplus_cat_thread_size() as static inline to header file as
> > suggested by Viacheslav Dubeyko
> >
> > Changes in v3:
> > - Introduced hfsplus_brec_read_cat() wrapper function for catalog-specific
> > validation instead of modifying generic hfs_brec_read()
> > - Added hfsplus_cat_thread_size() helper to calculate variable-size thread
> > record sizes
> > - Use exact size match (!=) instead of minimum size check (<)
> > - Use sizeof(hfsplus_unichr) instead of hardcoded value 2
> > - Updated all catalog record read sites to use new wrapper function
> > - Addressed review feedback from Viacheslav Dubeyko
> >
> > Changes in v2:
> > - Use structure initialization (= {0}) instead of memset()
> > - Improved commit message to clarify how uninitialized data is used
> > ---
> > fs/hfsplus/bfind.c | 46 +++++++++++++++++++++++++++++++++++++++++
> > fs/hfsplus/catalog.c | 4 ++--
> > fs/hfsplus/dir.c | 2 +-
> > fs/hfsplus/hfsplus_fs.h | 9 ++++++++
> > fs/hfsplus/super.c | 2 +-
> > 5 files changed, 59 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/hfsplus/bfind.c b/fs/hfsplus/bfind.c
> > index 9b89dce00ee9..4c5fd21585ef 100644
> > --- a/fs/hfsplus/bfind.c
> > +++ b/fs/hfsplus/bfind.c
> > @@ -297,3 +297,49 @@ int hfs_brec_goto(struct hfs_find_data *fd, int cnt)
> > fd->bnode = bnode;
> > return res;
> > }
> > +
> > +/**
> > + * hfsplus_brec_read_cat - read and validate a catalog record
> > + * @fd: find data structure
> > + * @entry: pointer to catalog entry to read into
> > + *
> > + * Reads a catalog record and validates its size matches the expected
> > + * size based on the record type.
> > + *
> > + * Returns 0 on success, or negative error code on failure.
> > + */
> > +int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry)
> > +{
> > + int res;
> > + u32 expected_size;
> > +
> > + res = hfs_brec_read(fd, entry, sizeof(hfsplus_cat_entry));
> > + if (res)
> > + return res;
> > +
> > + /* Validate catalog record size based on type */
> > + switch (be16_to_cpu(entry->type)) {
> > + case HFSPLUS_FOLDER:
> > + expected_size = sizeof(struct hfsplus_cat_folder);
> > + break;
> > + case HFSPLUS_FILE:
> > + expected_size = sizeof(struct hfsplus_cat_file);
> > + break;
> > + case HFSPLUS_FOLDER_THREAD:
> > + case HFSPLUS_FILE_THREAD:
> > + expected_size = hfsplus_cat_thread_size(&entry->thread);
> > + break;
> > + default:
> > + pr_err("unknown catalog record type %d\n",
> > + be16_to_cpu(entry->type));
> > + return -EIO;
> > + }
> > +
> > + if (fd->entrylength != expected_size) {
> > + pr_err("catalog record size mismatch (type %d, got %u, expected %u)\n",
> > + be16_to_cpu(entry->type), fd->entrylength, expected_size);
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
> > diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c
> > index 02c1eee4a4b8..6c8380f7208d 100644
> > --- a/fs/hfsplus/catalog.c
> > +++ b/fs/hfsplus/catalog.c
> > @@ -194,12 +194,12 @@ static int hfsplus_fill_cat_thread(struct super_block *sb,
> > 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));
> > + err = hfsplus_brec_read_cat(fd, &tmp);
> > if (err)
> > return err;
> >
> > diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
> > index cadf0b5f9342..d86e2f7b289c 100644
> > --- a/fs/hfsplus/dir.c
> > +++ b/fs/hfsplus/dir.c
> > @@ -49,7 +49,7 @@ static struct dentry *hfsplus_lookup(struct inode *dir, struct dentry *dentry,
> > if (unlikely(err < 0))
> > goto fail;
> > again:
> > - err = hfs_brec_read(&fd, &entry, sizeof(entry));
> > + err = hfsplus_brec_read_cat(&fd, &entry);
> > if (err) {
> > if (err == -ENOENT) {
> > hfs_find_exit(&fd);
> > diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> > index 45fe3a12ecba..e811d33861af 100644
> > --- a/fs/hfsplus/hfsplus_fs.h
> > +++ b/fs/hfsplus/hfsplus_fs.h
> > @@ -506,6 +506,15 @@ int hfsplus_submit_bio(struct super_block *sb, sector_t sector, void *buf,
> > void **data, blk_opf_t opf);
> > int hfsplus_read_wrapper(struct super_block *sb);
> >
> > +static inline u32 hfsplus_cat_thread_size(const struct hfsplus_cat_thread *thread)
> > +{
> > + return offsetof(struct hfsplus_cat_thread, nodeName) +
> > + offsetof(struct hfsplus_unistr, unicode) +
> > + be16_to_cpu(thread->nodeName.length) * sizeof(hfsplus_unichr);
> > +}
> > +
> > +int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry);
> > +
> > /*
> > * time helpers: convert between 1904-base and 1970-base timestamps
> > *
> > diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
> > index aaffa9e060a0..e59611a664ef 100644
> > --- a/fs/hfsplus/super.c
> > +++ b/fs/hfsplus/super.c
> > @@ -567,7 +567,7 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
> > err = hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, &str);
> > if (unlikely(err < 0))
> > goto out_put_root;
> > - if (!hfs_brec_read(&fd, &entry, sizeof(entry))) {
> > + if (!hfsplus_brec_read_cat(&fd, &entry)) {
> > hfs_find_exit(&fd);
> > if (entry.type != cpu_to_be16(HFSPLUS_FOLDER)) {
> > err = -EIO;
>
> The patch looks good. I don't have any remarks. I think that we've received
> pretty good suggestion how the patch could be improved. Could you improve the
> patch? :)
>
> I assume that we have the same issue for HFS case. Frankly speaking, I am
> considering to make the b-tree functionality generic one for HFS/HFS+ because
> there are multiple similarities in this logic. Would you like to try to
> generalize this b-tree functionality in the form of libhfs or something like
> this?
>
I did run xfstests for the patch. The patch hasn't introduced any new issues.
Everything looks good.
Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>
Tested-by: Viacheslav Dubeyko <slava@dubeyko.com>
I can take the patch as it is. But I would like to see the suggested small
improvement.
Thanks,
Slava.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v4] hfsplus: fix uninit-value by validating catalog record size
2026-02-17 23:16 ` Viacheslav Dubeyko
@ 2026-02-21 4:44 ` Deepanshu Kartikey
0 siblings, 0 replies; 6+ messages in thread
From: Deepanshu Kartikey @ 2026-02-21 4:44 UTC (permalink / raw)
To: Viacheslav Dubeyko
Cc: glaubitz@physik.fu-berlin.de, frank.li@vivo.com,
slava@dubeyko.com, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org,
syzbot+d80abb5b890d39261e72@syzkaller.appspotmail.com
On Wed, Feb 18, 2026 at 4:46 AM Viacheslav Dubeyko
<Slava.Dubeyko@ibm.com> wrote:
>
>
> I did run xfstests for the patch. The patch hasn't introduced any new issues.
> Everything looks good.
>
Hi Slava,
Thank you for the review and testing! I'm glad the patch passes xfstests.
I'll send v5 with the minimum size check as suggested by Charalampos.
Regarding your suggestions for future work:
1. **HFS fix**: I'd be happy to investigate and fix the same issue in fs/hfs/.
I'll check if the bug exists there and send a patch.
2. **Generalizing b-tree functionality**: This sounds like an
interesting project!
Let me study the fs/hfs and fs/hfsplus b-tree code to understand the
similarities and come back with some thoughts on how to approach this.
Thanks,
Deepanshu
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-02-21 4:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-14 0:21 [PATCH v4] hfsplus: fix uninit-value by validating catalog record size Deepanshu Kartikey
2026-02-16 2:15 ` Charalampos Mitrodimas
2026-02-17 0:13 ` Viacheslav Dubeyko
2026-02-17 0:19 ` Viacheslav Dubeyko
2026-02-17 23:16 ` Viacheslav Dubeyko
2026-02-21 4:44 ` Deepanshu Kartikey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox