123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108 |
- From 2ca0e5dbcdcb6fc93ccae39a0f39d0dba4a7ff20 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>
- Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.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 1c7791b..361e5be 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 117740a..e14dd31 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;
-
- --
- 2.14.2
|