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