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