Merge ~mkukri/grub:mantic into ~ubuntu-core-dev/grub/+git/ubuntu:mantic

Proposed by Mate Kukri
Status: Merged
Merged at revision: 72631015e47982030c5839a2b03877d8b00f15a7
Proposed branch: ~mkukri/grub:mantic
Merge into: ~ubuntu-core-dev/grub/+git/ubuntu:mantic
Diff against target: 270 lines (+230/-1)
5 files modified
debian/build-efi-images (+0/-1)
debian/changelog (+9/-0)
debian/patches/series (+2/-0)
debian/patches/upstream/xfs-fix-directory-extend-parsing.patch (+171/-0)
debian/patches/upstream/xfs-incorrect-directory-data-boundary-check.patch (+48/-0)
Reviewer Review Type Date Requested Status
Julian Andres Klode Approve
Review via email: mp+455401@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Julian Andres Klode (juliank) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/build-efi-images b/debian/build-efi-images
2index f64a37f..d17e225 100755
3--- a/debian/build-efi-images
4+++ b/debian/build-efi-images
5@@ -194,7 +194,6 @@ GRUB_MODULES="$CD_MODULES
6 gcry_twofish
7 gcry_whirlpool
8 luks
9- luks2
10 lvm
11 mdraid09
12 mdraid1x
13diff --git a/debian/changelog b/debian/changelog
14index 7f0eb33..68e3b8c 100644
15--- a/debian/changelog
16+++ b/debian/changelog
17@@ -1,3 +1,12 @@
18+grub2 (2.12~rc1-10ubuntu4.1) mantic; urgency=medium
19+
20+ [ Julian Andres Klode ]
21+ * Cherry-pick upstream XFS directory extent parsing fixes (Closes: #1051543)
22+ (LP: #2039172)
23+ * UBUNTU: Drop luks2 (LP: #2043101)
24+
25+ -- Mate Kukri <mate.kukri@canonical.com> Thu, 09 Nov 2023 15:04:44 +0200
26+
27 grub2 (2.12~rc1-10ubuntu4) mantic; urgency=high
28
29 * SECURITY UPDATE: Crafted file system images can cause out-of-bounds write
30diff --git a/debian/patches/series b/debian/patches/series
31index a5ca765..ca68caf 100644
32--- a/debian/patches/series
33+++ b/debian/patches/series
34@@ -94,3 +94,5 @@ ntfs-cve-fixes/fs-ntfs-Fix-an-OOB-read-when-parsing-directory-entries-fr.patch
35 ntfs-cve-fixes/fs-ntfs-Fix-an-OOB-read-when-parsing-bitmaps-for-index-at.patch
36 ntfs-cve-fixes/fs-ntfs-Fix-an-OOB-read-when-parsing-a-volume-label.patch
37 ntfs-cve-fixes/fs-ntfs-Make-code-more-readable.patch
38+upstream/xfs-incorrect-directory-data-boundary-check.patch
39+upstream/xfs-fix-directory-extend-parsing.patch
40diff --git a/debian/patches/upstream/xfs-fix-directory-extend-parsing.patch b/debian/patches/upstream/xfs-fix-directory-extend-parsing.patch
41new file mode 100644
42index 0000000..8b1c124
43--- /dev/null
44+++ b/debian/patches/upstream/xfs-fix-directory-extend-parsing.patch
45@@ -0,0 +1,171 @@
46+From: Jon DeVree <nuxi@vault24.org>
47+Date: Tue, 17 Oct 2023 23:03:47 -0400
48+Subject: fs/xfs: Fix XFS directory extent parsing
49+
50+The XFS directory entry parsing code has never been completely correct
51+for extent based directories. The parser correctly handles the case
52+where the directory is contained in a single extent, but then mistakenly
53+assumes the data blocks for the multiple extent case are each identical
54+to the single extent case. The difference in the format of the data
55+blocks between the two cases is tiny enough that its gone unnoticed for
56+a very long time.
57+
58+A recent change introduced some additional bounds checking into the XFS
59+parser. Like GRUB's existing parser, it is correct for the single extent
60+case but incorrect for the multiple extent case. When parsing a directory
61+with multiple extents, this new bounds checking is sometimes (but not
62+always) tripped and triggers an "invalid XFS directory entry" error. This
63+probably would have continued to go unnoticed but the /boot/grub/<arch>
64+directory is large enough that it often has multiple extents.
65+
66+The difference between the two cases is that when there are multiple
67+extents, the data blocks do not contain a trailer nor do they contain
68+any leaf information. That information is stored in a separate set of
69+extents dedicated to just the leaf information. These extents come after
70+the directory entry extents and are not included in the inode size. So
71+the existing parser already ignores the leaf extents.
72+
73+The only reason to read the trailer/leaf information at all is so that
74+the parser can avoid misinterpreting that data as directory entries. So
75+this updates the parser as follows:
76+
77+For the single extent case the parser doesn't change much:
78+1. Read the size of the leaf information from the trailer
79+2. Set the end pointer for the parser to the start of the leaf
80+ information. (The previous bounds checking set the end pointer to the
81+ start of the trailer, so this is actually a small improvement.)
82+3. Set the entries variable to the expected number of directory entries.
83+
84+For the multiple extent case:
85+1. Set the end pointer to the end of the block.
86+2. Do not set up the entries variable. Figuring out how many entries are
87+ in each individual block is complex and does not seem worth it when
88+ it appears to be safe to just iterate over the entire block.
89+
90+The bounds check itself was also dependent upon the faulty XFS parser
91+because it accidentally used "filename + length - 1". Presumably this
92+was able to pass the fuzzer because in the old parser there was always
93+8 bytes of slack space between the tail pointer and the actual end of
94+the block. Since this is no longer the case the bounds check needs to be
95+updated to "filename + length + 1" in order to prevent a regression in
96+the handling of corrupt fliesystems.
97+
98+Notes:
99+* When there is only one extent there will only ever be one block. If
100+ more than one block is required then XFS will always switch to holding
101+ leaf information in a separate extent.
102+* B-tree based directories seems to be parsed properly by the same code
103+ that handles multiple extents. This is unlikely to ever occur within
104+ /boot though because its only used when there are an extremely large
105+ number of directory entries.
106+
107+Fixes: ef7850c75 (fs/xfs: Fix issues found while fuzzing the XFS filesystem)
108+Fixes: b2499b29c (Adds support for the XFS filesystem.)
109+Fixes: https://savannah.gnu.org/bugs/?64376
110+
111+Signed-off-by: Jon DeVree <nuxi@vault24.org>
112+Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
113+Tested-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
114+Tested-by: Marta Lewandowska <mlewando@redhat.com>
115+
116+Origin: upstream
117+Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1051543
118+Bug-Ubuntu: https://bugs.launchpad.net/bugs/2039172
119+---
120+ grub-core/fs/xfs.c | 52 ++++++++++++++++++++++++++++++++++++++--------------
121+ 1 file changed, 38 insertions(+), 14 deletions(-)
122+
123+diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
124+index ebf9627..18edfcf 100644
125+--- a/grub-core/fs/xfs.c
126++++ b/grub-core/fs/xfs.c
127+@@ -223,6 +223,12 @@ struct grub_xfs_inode
128+ /* Size of struct grub_xfs_inode v2, up to unused4 member included. */
129+ #define XFS_V2_INODE_SIZE (XFS_V3_INODE_SIZE - 76)
130+
131++struct grub_xfs_dir_leaf_entry
132++{
133++ grub_uint32_t hashval;
134++ grub_uint32_t address;
135++} GRUB_PACKED;
136++
137+ struct grub_xfs_dirblock_tail
138+ {
139+ grub_uint32_t leaf_count;
140+@@ -874,9 +880,8 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
141+ {
142+ struct grub_xfs_dir2_entry *direntry =
143+ grub_xfs_first_de(dir->data, dirblock);
144+- int entries;
145+- struct grub_xfs_dirblock_tail *tail =
146+- grub_xfs_dir_tail(dir->data, dirblock);
147++ int entries = -1;
148++ char *end = dirblock + dirblk_size;
149+
150+ numread = grub_xfs_read_file (dir, 0, 0,
151+ blk << dirblk_log2,
152+@@ -887,14 +892,27 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
153+ return 0;
154+ }
155+
156+- entries = (grub_be_to_cpu32 (tail->leaf_count)
157+- - grub_be_to_cpu32 (tail->leaf_stale));
158++ /*
159++ * Leaf and tail information are only in the data block if the number
160++ * of extents is 1.
161++ */
162++ if (dir->inode.nextents == grub_cpu_to_be32_compile_time (1))
163++ {
164++ struct grub_xfs_dirblock_tail *tail = grub_xfs_dir_tail (dir->data, dirblock);
165++
166++ end = (char *) tail;
167++
168++ /* Subtract the space used by leaf nodes. */
169++ end -= grub_be_to_cpu32 (tail->leaf_count) * sizeof (struct grub_xfs_dir_leaf_entry);
170+
171+- if (!entries)
172+- continue;
173++ entries = grub_be_to_cpu32 (tail->leaf_count) - grub_be_to_cpu32 (tail->leaf_stale);
174++
175++ if (!entries)
176++ continue;
177++ }
178+
179+ /* Iterate over all entries within this block. */
180+- while ((char *)direntry < (char *)tail)
181++ while ((char *) direntry < (char *) end)
182+ {
183+ grub_uint8_t *freetag;
184+ char *filename;
185+@@ -914,7 +932,7 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
186+ }
187+
188+ filename = (char *)(direntry + 1);
189+- if (filename + direntry->len - 1 > (char *) tail)
190++ if (filename + direntry->len + 1 > (char *) end)
191+ return grub_error (GRUB_ERR_BAD_FS, "invalid XFS directory entry");
192+
193+ /* The byte after the filename is for the filetype, padding, or
194+@@ -928,11 +946,17 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
195+ return 1;
196+ }
197+
198+- /* Check if last direntry in this block is
199+- reached. */
200+- entries--;
201+- if (!entries)
202+- break;
203++ /*
204++ * The expected number of directory entries is only tracked for the
205++ * single extent case.
206++ */
207++ if (dir->inode.nextents == grub_cpu_to_be32_compile_time (1))
208++ {
209++ /* Check if last direntry in this block is reached. */
210++ entries--;
211++ if (!entries)
212++ break;
213++ }
214+
215+ /* Select the next directory entry. */
216+ direntry = grub_xfs_next_de(dir->data, direntry);
217diff --git a/debian/patches/upstream/xfs-incorrect-directory-data-boundary-check.patch b/debian/patches/upstream/xfs-incorrect-directory-data-boundary-check.patch
218new file mode 100644
219index 0000000..ada9bc6
220--- /dev/null
221+++ b/debian/patches/upstream/xfs-incorrect-directory-data-boundary-check.patch
222@@ -0,0 +1,48 @@
223+From: Lidong Chen <lidong.chen@oracle.com>
224+Date: Thu, 28 Sep 2023 22:33:44 +0000
225+Subject: fs/xfs: Incorrect short form directory data boundary check
226+
227+After parsing of the current entry, the entry pointer is advanced
228+to the next entry at the end of the "for" loop. In case where the
229+last entry is at the end of the data boundary, the advanced entry
230+pointer can point off the data boundary. The subsequent boundary
231+check for the advanced entry pointer can cause a failure.
232+
233+The fix is to include the boundary check into the "for" loop
234+condition.
235+
236+Signed-off-by: Lidong Chen <lidong.chen@oracle.com>
237+Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
238+Tested-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
239+Tested-by: Marta Lewandowska <mlewando@redhat.com>
240+
241+Origin: upstream
242+---
243+ grub-core/fs/xfs.c | 7 ++-----
244+ 1 file changed, 2 insertions(+), 5 deletions(-)
245+
246+diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
247+index b91cd32..ebf9627 100644
248+--- a/grub-core/fs/xfs.c
249++++ b/grub-core/fs/xfs.c
250+@@ -810,7 +810,8 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
251+ if (iterate_dir_call_hook (parent, "..", &ctx))
252+ return 1;
253+
254+- for (i = 0; i < head->count; i++)
255++ for (i = 0; i < head->count &&
256++ (grub_uint8_t *) de < ((grub_uint8_t *) dir + grub_xfs_fshelp_size (dir->data)); i++)
257+ {
258+ grub_uint64_t ino;
259+ grub_uint8_t *inopos = grub_xfs_inline_de_inopos(dir->data, de);
260+@@ -845,10 +846,6 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
261+ de->name[de->len] = c;
262+
263+ de = grub_xfs_inline_next_de(dir->data, head, de);
264+-
265+- if ((grub_uint8_t *) de >= (grub_uint8_t *) dir + grub_xfs_fshelp_size (dir->data))
266+- return grub_error (GRUB_ERR_BAD_FS, "invalid XFS directory entry");
267+-
268+ }
269+ break;
270+ }

Subscribers

People subscribed via source and target branches