Skip to content

Commit b6b5922

Browse files
deepanshu406dubeyko
authored andcommitted
hfsplus: fix uninit-value by validating catalog record size
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 For thread records, check against HFSPLUS_MIN_THREAD_SZ before reading nodeName.length to avoid reading uninitialized data at call sites that don't zero-initialize the entry structure. Also initialize the tmp variable in hfsplus_find_cat() as defensive programming to ensure no uninitialized data even if validation is bypassed. Reported-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=d80abb5b890d39261e72 Fixes: 1da177e ("Linux-2.6.12-rc2") Tested-by: [email protected] Reviewed-by: Viacheslav Dubeyko <[email protected]> Tested-by: Viacheslav Dubeyko <[email protected]> Suggested-by: Charalampos Mitrodimas <[email protected]> Link: https://lore.kernel.org/all/[email protected]/ [v1] Link: https://lore.kernel.org/all/[email protected]/ [v2] Link: https://lore.kernel.org/all/[email protected]/ [v3] Link: https://lore.kernel.org/all/[email protected]/T/ [v4] Link: https://lore.kernel.org/all/[email protected]/T/ [v5] Signed-off-by: Deepanshu Kartikey <[email protected]> Signed-off-by: Viacheslav Dubeyko <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Viacheslav Dubeyko <[email protected]>
1 parent ee8422d commit b6b5922

5 files changed

Lines changed: 64 additions & 4 deletions

File tree

fs/hfsplus/bfind.c

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,3 +287,54 @@ int hfs_brec_goto(struct hfs_find_data *fd, int cnt)
287287
fd->bnode = bnode;
288288
return res;
289289
}
290+
291+
/**
292+
* hfsplus_brec_read_cat - read and validate a catalog record
293+
* @fd: find data structure
294+
* @entry: pointer to catalog entry to read into
295+
*
296+
* Reads a catalog record and validates its size matches the expected
297+
* size based on the record type.
298+
*
299+
* Returns 0 on success, or negative error code on failure.
300+
*/
301+
int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry)
302+
{
303+
int res;
304+
u32 expected_size;
305+
306+
res = hfs_brec_read(fd, entry, sizeof(hfsplus_cat_entry));
307+
if (res)
308+
return res;
309+
310+
/* Validate catalog record size based on type */
311+
switch (be16_to_cpu(entry->type)) {
312+
case HFSPLUS_FOLDER:
313+
expected_size = sizeof(struct hfsplus_cat_folder);
314+
break;
315+
case HFSPLUS_FILE:
316+
expected_size = sizeof(struct hfsplus_cat_file);
317+
break;
318+
case HFSPLUS_FOLDER_THREAD:
319+
case HFSPLUS_FILE_THREAD:
320+
/* Ensure we have at least the fixed fields before reading nodeName.length */
321+
if (fd->entrylength < HFSPLUS_MIN_THREAD_SZ) {
322+
pr_err("thread record too short (got %u)\n", fd->entrylength);
323+
return -EIO;
324+
}
325+
expected_size = hfsplus_cat_thread_size(&entry->thread);
326+
break;
327+
default:
328+
pr_err("unknown catalog record type %d\n",
329+
be16_to_cpu(entry->type));
330+
return -EIO;
331+
}
332+
333+
if (fd->entrylength != expected_size) {
334+
pr_err("catalog record size mismatch (type %d, got %u, expected %u)\n",
335+
be16_to_cpu(entry->type), fd->entrylength, expected_size);
336+
return -EIO;
337+
}
338+
339+
return 0;
340+
}

fs/hfsplus/catalog.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,12 +194,12 @@ static int hfsplus_fill_cat_thread(struct super_block *sb,
194194
int hfsplus_find_cat(struct super_block *sb, u32 cnid,
195195
struct hfs_find_data *fd)
196196
{
197-
hfsplus_cat_entry tmp;
197+
hfsplus_cat_entry tmp = {0};
198198
int err;
199199
u16 type;
200200

201201
hfsplus_cat_build_key_with_cnid(sb, fd->search_key, cnid);
202-
err = hfs_brec_read(fd, &tmp, sizeof(hfsplus_cat_entry));
202+
err = hfsplus_brec_read_cat(fd, &tmp);
203203
if (err)
204204
return err;
205205

fs/hfsplus/dir.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ static struct dentry *hfsplus_lookup(struct inode *dir, struct dentry *dentry,
4949
if (unlikely(err < 0))
5050
goto fail;
5151
again:
52-
err = hfs_brec_read(&fd, &entry, sizeof(entry));
52+
err = hfsplus_brec_read_cat(&fd, &entry);
5353
if (err) {
5454
if (err == -ENOENT) {
5555
hfs_find_exit(&fd);

fs/hfsplus/hfsplus_fs.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,15 @@ int hfsplus_submit_bio(struct super_block *sb, sector_t sector, void *buf,
516516
void **data, blk_opf_t opf);
517517
int hfsplus_read_wrapper(struct super_block *sb);
518518

519+
static inline u32 hfsplus_cat_thread_size(const struct hfsplus_cat_thread *thread)
520+
{
521+
return offsetof(struct hfsplus_cat_thread, nodeName) +
522+
offsetof(struct hfsplus_unistr, unicode) +
523+
be16_to_cpu(thread->nodeName.length) * sizeof(hfsplus_unichr);
524+
}
525+
526+
int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry);
527+
519528
/*
520529
* time helpers: convert between 1904-base and 1970-base timestamps
521530
*

fs/hfsplus/super.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
571571
err = hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, &str);
572572
if (unlikely(err < 0))
573573
goto out_put_root;
574-
if (!hfs_brec_read(&fd, &entry, sizeof(entry))) {
574+
if (!hfsplus_brec_read_cat(&fd, &entry)) {
575575
hfs_find_exit(&fd);
576576
if (entry.type != cpu_to_be16(HFSPLUS_FOLDER)) {
577577
err = -EIO;

0 commit comments

Comments
 (0)