105 lines
4.3 KiB
Diff
105 lines
4.3 KiB
Diff
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
From: Daniel Axtens <dja@axtens.net>
|
|
Date: Tue, 2 Feb 2021 16:59:35 +1100
|
|
Subject: [PATCH] fs/hfsplus: Don't use uninitialized data on corrupt
|
|
filesystems
|
|
|
|
Valgrind identified the following use of uninitialized data:
|
|
|
|
==2782220== Conditional jump or move depends on uninitialised value(s)
|
|
==2782220== at 0x42B364: grub_hfsplus_btree_search (hfsplus.c:566)
|
|
==2782220== by 0x42B21D: grub_hfsplus_read_block (hfsplus.c:185)
|
|
==2782220== by 0x42A693: grub_fshelp_read_file (fshelp.c:386)
|
|
==2782220== by 0x42C598: grub_hfsplus_read_file (hfsplus.c:219)
|
|
==2782220== by 0x42C598: grub_hfsplus_mount (hfsplus.c:330)
|
|
==2782220== by 0x42B8C5: grub_hfsplus_dir (hfsplus.c:958)
|
|
==2782220== by 0x4C1AE6: grub_fs_probe (fs.c:73)
|
|
==2782220== by 0x407C94: grub_ls_list_files (ls.c:186)
|
|
==2782220== by 0x407C94: grub_cmd_ls (ls.c:284)
|
|
==2782220== by 0x4D7130: grub_extcmd_dispatcher (extcmd.c:55)
|
|
==2782220== by 0x4045A6: execute_command (grub-fstest.c:59)
|
|
==2782220== by 0x4045A6: fstest (grub-fstest.c:433)
|
|
==2782220== by 0x4045A6: main (grub-fstest.c:772)
|
|
==2782220== Uninitialised value was created by a heap allocation
|
|
==2782220== at 0x483C7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
|
|
==2782220== by 0x4C0305: grub_malloc (mm.c:42)
|
|
==2782220== by 0x42C21D: grub_hfsplus_mount (hfsplus.c:239)
|
|
==2782220== by 0x42B8C5: grub_hfsplus_dir (hfsplus.c:958)
|
|
==2782220== by 0x4C1AE6: grub_fs_probe (fs.c:73)
|
|
==2782220== by 0x407C94: grub_ls_list_files (ls.c:186)
|
|
==2782220== by 0x407C94: grub_cmd_ls (ls.c:284)
|
|
==2782220== by 0x4D7130: grub_extcmd_dispatcher (extcmd.c:55)
|
|
==2782220== by 0x4045A6: execute_command (grub-fstest.c:59)
|
|
==2782220== by 0x4045A6: fstest (grub-fstest.c:433)
|
|
==2782220== by 0x4045A6: main (grub-fstest.c:772)
|
|
|
|
This happens when the process of reading the catalog file goes sufficiently
|
|
wrong that there's an attempt to read the extent overflow file, which has
|
|
not yet been loaded. Keep track of when the extent overflow file is
|
|
fully loaded and refuse to use it before then.
|
|
|
|
The load valgrind doesn't like is btree->nodesize, and that's then used
|
|
to allocate a data structure. It looks like there are subsequently a lot
|
|
of reads based on that pointer so OOB reads are likely, and indeed crashes
|
|
(albeit difficult-to-replicate ones) have been observed in fuzzing.
|
|
|
|
Signed-off-by: Daniel Axtens <dja@axtens.net>
|
|
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
|
|
---
|
|
grub-core/fs/hfsplus.c | 14 ++++++++++++++
|
|
include/grub/hfsplus.h | 2 ++
|
|
2 files changed, 16 insertions(+)
|
|
|
|
diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c
|
|
index 423f4b956..8c0c80473 100644
|
|
--- a/grub-core/fs/hfsplus.c
|
|
+++ b/grub-core/fs/hfsplus.c
|
|
@@ -177,6 +177,17 @@ grub_hfsplus_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
|
|
break;
|
|
}
|
|
|
|
+ /*
|
|
+ * If the extent overflow tree isn't ready yet, we can't look
|
|
+ * in it. This can happen where the catalog file is corrupted.
|
|
+ */
|
|
+ if (!node->data->extoverflow_tree_ready)
|
|
+ {
|
|
+ grub_error (GRUB_ERR_BAD_FS,
|
|
+ "attempted to read extent overflow tree before loading");
|
|
+ break;
|
|
+ }
|
|
+
|
|
/* Set up the key to look for in the extent overflow file. */
|
|
extoverflow.extkey.fileid = node->fileid;
|
|
extoverflow.extkey.type = 0;
|
|
@@ -241,6 +252,7 @@ grub_hfsplus_mount (grub_disk_t disk)
|
|
return 0;
|
|
|
|
data->disk = disk;
|
|
+ data->extoverflow_tree_ready = 0;
|
|
|
|
/* Read the bootblock. */
|
|
grub_disk_read (disk, GRUB_HFSPLUS_SBLOCK, 0, sizeof (volheader),
|
|
@@ -357,6 +369,8 @@ grub_hfsplus_mount (grub_disk_t disk)
|
|
if (data->extoverflow_tree.nodesize < 2)
|
|
goto fail;
|
|
|
|
+ data->extoverflow_tree_ready = 1;
|
|
+
|
|
if (grub_hfsplus_read_file (&data->attr_tree.file, 0, 0,
|
|
sizeof (struct grub_hfsplus_btnode),
|
|
sizeof (header), (char *) &header) <= 0)
|
|
diff --git a/include/grub/hfsplus.h b/include/grub/hfsplus.h
|
|
index 117740ae2..e14dd31ff 100644
|
|
--- a/include/grub/hfsplus.h
|
|
+++ b/include/grub/hfsplus.h
|
|
@@ -113,6 +113,8 @@ struct grub_hfsplus_data
|
|
struct grub_hfsplus_btree extoverflow_tree;
|
|
struct grub_hfsplus_btree attr_tree;
|
|
|
|
+ int extoverflow_tree_ready;
|
|
+
|
|
struct grub_hfsplus_file dirroot;
|
|
struct grub_hfsplus_file opened_file;
|
|
|