1From 2ca0e5dbcdcb6fc93ccae39a0f39d0dba4a7ff20 Mon Sep 17 00:00:00 2001 2From: Daniel Axtens <dja@axtens.net> 3Date: Tue, 2 Feb 2021 16:59:35 +1100 4Subject: [PATCH] fs/hfsplus: Don't use uninitialized data on corrupt 5 filesystems 6 7Valgrind identified the following use of uninitialized data: 8 9 ==2782220== Conditional jump or move depends on uninitialised value(s) 10 ==2782220== at 0x42B364: grub_hfsplus_btree_search (hfsplus.c:566) 11 ==2782220== by 0x42B21D: grub_hfsplus_read_block (hfsplus.c:185) 12 ==2782220== by 0x42A693: grub_fshelp_read_file (fshelp.c:386) 13 ==2782220== by 0x42C598: grub_hfsplus_read_file (hfsplus.c:219) 14 ==2782220== by 0x42C598: grub_hfsplus_mount (hfsplus.c:330) 15 ==2782220== by 0x42B8C5: grub_hfsplus_dir (hfsplus.c:958) 16 ==2782220== by 0x4C1AE6: grub_fs_probe (fs.c:73) 17 ==2782220== by 0x407C94: grub_ls_list_files (ls.c:186) 18 ==2782220== by 0x407C94: grub_cmd_ls (ls.c:284) 19 ==2782220== by 0x4D7130: grub_extcmd_dispatcher (extcmd.c:55) 20 ==2782220== by 0x4045A6: execute_command (grub-fstest.c:59) 21 ==2782220== by 0x4045A6: fstest (grub-fstest.c:433) 22 ==2782220== by 0x4045A6: main (grub-fstest.c:772) 23 ==2782220== Uninitialised value was created by a heap allocation 24 ==2782220== at 0x483C7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) 25 ==2782220== by 0x4C0305: grub_malloc (mm.c:42) 26 ==2782220== by 0x42C21D: grub_hfsplus_mount (hfsplus.c:239) 27 ==2782220== by 0x42B8C5: grub_hfsplus_dir (hfsplus.c:958) 28 ==2782220== by 0x4C1AE6: grub_fs_probe (fs.c:73) 29 ==2782220== by 0x407C94: grub_ls_list_files (ls.c:186) 30 ==2782220== by 0x407C94: grub_cmd_ls (ls.c:284) 31 ==2782220== by 0x4D7130: grub_extcmd_dispatcher (extcmd.c:55) 32 ==2782220== by 0x4045A6: execute_command (grub-fstest.c:59) 33 ==2782220== by 0x4045A6: fstest (grub-fstest.c:433) 34 ==2782220== by 0x4045A6: main (grub-fstest.c:772) 35 36This happens when the process of reading the catalog file goes sufficiently 37wrong that there's an attempt to read the extent overflow file, which has 38not yet been loaded. Keep track of when the extent overflow file is 39fully loaded and refuse to use it before then. 40 41The load valgrind doesn't like is btree->nodesize, and that's then used 42to allocate a data structure. It looks like there are subsequently a lot 43of reads based on that pointer so OOB reads are likely, and indeed crashes 44(albeit difficult-to-replicate ones) have been observed in fuzzing. 45 46Signed-off-by: Daniel Axtens <dja@axtens.net> 47Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> 48Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com> 49--- 50 grub-core/fs/hfsplus.c | 14 ++++++++++++++ 51 include/grub/hfsplus.h | 2 ++ 52 2 files changed, 16 insertions(+) 53 54diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c 55index 1c7791b..361e5be 100644 56--- a/grub-core/fs/hfsplus.c 57+++ b/grub-core/fs/hfsplus.c 58@@ -177,6 +177,17 @@ grub_hfsplus_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock) 59 break; 60 } 61 62+ /* 63+ * If the extent overflow tree isn't ready yet, we can't look 64+ * in it. This can happen where the catalog file is corrupted. 65+ */ 66+ if (!node->data->extoverflow_tree_ready) 67+ { 68+ grub_error (GRUB_ERR_BAD_FS, 69+ "attempted to read extent overflow tree before loading"); 70+ break; 71+ } 72+ 73 /* Set up the key to look for in the extent overflow file. */ 74 extoverflow.extkey.fileid = node->fileid; 75 extoverflow.extkey.type = 0; 76@@ -241,6 +252,7 @@ grub_hfsplus_mount (grub_disk_t disk) 77 return 0; 78 79 data->disk = disk; 80+ data->extoverflow_tree_ready = 0; 81 82 /* Read the bootblock. */ 83 grub_disk_read (disk, GRUB_HFSPLUS_SBLOCK, 0, sizeof (volheader), 84@@ -357,6 +369,8 @@ grub_hfsplus_mount (grub_disk_t disk) 85 if (data->extoverflow_tree.nodesize < 2) 86 goto fail; 87 88+ data->extoverflow_tree_ready = 1; 89+ 90 if (grub_hfsplus_read_file (&data->attr_tree.file, 0, 0, 91 sizeof (struct grub_hfsplus_btnode), 92 sizeof (header), (char *) &header) <= 0) 93diff --git a/include/grub/hfsplus.h b/include/grub/hfsplus.h 94index 117740a..e14dd31 100644 95--- a/include/grub/hfsplus.h 96+++ b/include/grub/hfsplus.h 97@@ -113,6 +113,8 @@ struct grub_hfsplus_data 98 struct grub_hfsplus_btree extoverflow_tree; 99 struct grub_hfsplus_btree attr_tree; 100 101+ int extoverflow_tree_ready; 102+ 103 struct grub_hfsplus_file dirroot; 104 struct grub_hfsplus_file opened_file; 105 106-- 1072.14.2 108 109