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