mirror of
https://src.fedoraproject.org/rpms/grub2.git
synced 2024-11-24 06:22:43 +00:00
xfs: Remove directory extent parsing patch
Some bios systems can't boot with one of the xfs upstream patches Resolves: #2254370 Signed-off-by: Nicolas Frayer <nfrayer@redhat.com>
This commit is contained in:
parent
0c1c9228d2
commit
ebd311ec52
6 changed files with 8 additions and 173 deletions
|
@ -1,168 +0,0 @@
|
||||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
||||||
From: Jon DeVree <nuxi@vault24.org>
|
|
||||||
Date: Tue, 17 Oct 2023 23:03:47 -0400
|
|
||||||
Subject: [PATCH] fs/xfs: Fix XFS directory extent parsing
|
|
||||||
|
|
||||||
The XFS directory entry parsing code has never been completely correct
|
|
||||||
for extent based directories. The parser correctly handles the case
|
|
||||||
where the directory is contained in a single extent, but then mistakenly
|
|
||||||
assumes the data blocks for the multiple extent case are each identical
|
|
||||||
to the single extent case. The difference in the format of the data
|
|
||||||
blocks between the two cases is tiny enough that its gone unnoticed for
|
|
||||||
a very long time.
|
|
||||||
|
|
||||||
A recent change introduced some additional bounds checking into the XFS
|
|
||||||
parser. Like GRUB's existing parser, it is correct for the single extent
|
|
||||||
case but incorrect for the multiple extent case. When parsing a directory
|
|
||||||
with multiple extents, this new bounds checking is sometimes (but not
|
|
||||||
always) tripped and triggers an "invalid XFS directory entry" error. This
|
|
||||||
probably would have continued to go unnoticed but the /boot/grub/<arch>
|
|
||||||
directory is large enough that it often has multiple extents.
|
|
||||||
|
|
||||||
The difference between the two cases is that when there are multiple
|
|
||||||
extents, the data blocks do not contain a trailer nor do they contain
|
|
||||||
any leaf information. That information is stored in a separate set of
|
|
||||||
extents dedicated to just the leaf information. These extents come after
|
|
||||||
the directory entry extents and are not included in the inode size. So
|
|
||||||
the existing parser already ignores the leaf extents.
|
|
||||||
|
|
||||||
The only reason to read the trailer/leaf information at all is so that
|
|
||||||
the parser can avoid misinterpreting that data as directory entries. So
|
|
||||||
this updates the parser as follows:
|
|
||||||
|
|
||||||
For the single extent case the parser doesn't change much:
|
|
||||||
1. Read the size of the leaf information from the trailer
|
|
||||||
2. Set the end pointer for the parser to the start of the leaf
|
|
||||||
information. (The previous bounds checking set the end pointer to the
|
|
||||||
start of the trailer, so this is actually a small improvement.)
|
|
||||||
3. Set the entries variable to the expected number of directory entries.
|
|
||||||
|
|
||||||
For the multiple extent case:
|
|
||||||
1. Set the end pointer to the end of the block.
|
|
||||||
2. Do not set up the entries variable. Figuring out how many entries are
|
|
||||||
in each individual block is complex and does not seem worth it when
|
|
||||||
it appears to be safe to just iterate over the entire block.
|
|
||||||
|
|
||||||
The bounds check itself was also dependent upon the faulty XFS parser
|
|
||||||
because it accidentally used "filename + length - 1". Presumably this
|
|
||||||
was able to pass the fuzzer because in the old parser there was always
|
|
||||||
8 bytes of slack space between the tail pointer and the actual end of
|
|
||||||
the block. Since this is no longer the case the bounds check needs to be
|
|
||||||
updated to "filename + length + 1" in order to prevent a regression in
|
|
||||||
the handling of corrupt fliesystems.
|
|
||||||
|
|
||||||
Notes:
|
|
||||||
* When there is only one extent there will only ever be one block. If
|
|
||||||
more than one block is required then XFS will always switch to holding
|
|
||||||
leaf information in a separate extent.
|
|
||||||
* B-tree based directories seems to be parsed properly by the same code
|
|
||||||
that handles multiple extents. This is unlikely to ever occur within
|
|
||||||
/boot though because its only used when there are an extremely large
|
|
||||||
number of directory entries.
|
|
||||||
|
|
||||||
Fixes: ef7850c75 (fs/xfs: Fix issues found while fuzzing the XFS filesystem)
|
|
||||||
Fixes: b2499b29c (Adds support for the XFS filesystem.)
|
|
||||||
Fixes: https://savannah.gnu.org/bugs/?64376
|
|
||||||
|
|
||||||
Signed-off-by: Jon DeVree <nuxi@vault24.org>
|
|
||||||
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
|
|
||||||
Tested-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
|
|
||||||
Tested-by: Marta Lewandowska <mlewando@redhat.com>
|
|
||||||
---
|
|
||||||
grub-core/fs/xfs.c | 52 ++++++++++++++++++++++++++++++++++++++--------------
|
|
||||||
1 file changed, 38 insertions(+), 14 deletions(-)
|
|
||||||
|
|
||||||
diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
|
|
||||||
index ebf962793fa7..18edfcff486c 100644
|
|
||||||
--- a/grub-core/fs/xfs.c
|
|
||||||
+++ b/grub-core/fs/xfs.c
|
|
||||||
@@ -223,6 +223,12 @@ struct grub_xfs_inode
|
|
||||||
/* Size of struct grub_xfs_inode v2, up to unused4 member included. */
|
|
||||||
#define XFS_V2_INODE_SIZE (XFS_V3_INODE_SIZE - 76)
|
|
||||||
|
|
||||||
+struct grub_xfs_dir_leaf_entry
|
|
||||||
+{
|
|
||||||
+ grub_uint32_t hashval;
|
|
||||||
+ grub_uint32_t address;
|
|
||||||
+} GRUB_PACKED;
|
|
||||||
+
|
|
||||||
struct grub_xfs_dirblock_tail
|
|
||||||
{
|
|
||||||
grub_uint32_t leaf_count;
|
|
||||||
@@ -874,9 +880,8 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
|
|
||||||
{
|
|
||||||
struct grub_xfs_dir2_entry *direntry =
|
|
||||||
grub_xfs_first_de(dir->data, dirblock);
|
|
||||||
- int entries;
|
|
||||||
- struct grub_xfs_dirblock_tail *tail =
|
|
||||||
- grub_xfs_dir_tail(dir->data, dirblock);
|
|
||||||
+ int entries = -1;
|
|
||||||
+ char *end = dirblock + dirblk_size;
|
|
||||||
|
|
||||||
numread = grub_xfs_read_file (dir, 0, 0,
|
|
||||||
blk << dirblk_log2,
|
|
||||||
@@ -887,14 +892,27 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
|
|
||||||
return 0;
|
|
||||||
}
|
|
||||||
|
|
||||||
- entries = (grub_be_to_cpu32 (tail->leaf_count)
|
|
||||||
- - grub_be_to_cpu32 (tail->leaf_stale));
|
|
||||||
+ /*
|
|
||||||
+ * Leaf and tail information are only in the data block if the number
|
|
||||||
+ * of extents is 1.
|
|
||||||
+ */
|
|
||||||
+ if (dir->inode.nextents == grub_cpu_to_be32_compile_time (1))
|
|
||||||
+ {
|
|
||||||
+ struct grub_xfs_dirblock_tail *tail = grub_xfs_dir_tail (dir->data, dirblock);
|
|
||||||
|
|
||||||
- if (!entries)
|
|
||||||
- continue;
|
|
||||||
+ end = (char *) tail;
|
|
||||||
+
|
|
||||||
+ /* Subtract the space used by leaf nodes. */
|
|
||||||
+ end -= grub_be_to_cpu32 (tail->leaf_count) * sizeof (struct grub_xfs_dir_leaf_entry);
|
|
||||||
+
|
|
||||||
+ entries = grub_be_to_cpu32 (tail->leaf_count) - grub_be_to_cpu32 (tail->leaf_stale);
|
|
||||||
+
|
|
||||||
+ if (!entries)
|
|
||||||
+ continue;
|
|
||||||
+ }
|
|
||||||
|
|
||||||
/* Iterate over all entries within this block. */
|
|
||||||
- while ((char *)direntry < (char *)tail)
|
|
||||||
+ while ((char *) direntry < (char *) end)
|
|
||||||
{
|
|
||||||
grub_uint8_t *freetag;
|
|
||||||
char *filename;
|
|
||||||
@@ -914,7 +932,7 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
|
|
||||||
}
|
|
||||||
|
|
||||||
filename = (char *)(direntry + 1);
|
|
||||||
- if (filename + direntry->len - 1 > (char *) tail)
|
|
||||||
+ if (filename + direntry->len + 1 > (char *) end)
|
|
||||||
return grub_error (GRUB_ERR_BAD_FS, "invalid XFS directory entry");
|
|
||||||
|
|
||||||
/* The byte after the filename is for the filetype, padding, or
|
|
||||||
@@ -928,11 +946,17 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
|
|
||||||
return 1;
|
|
||||||
}
|
|
||||||
|
|
||||||
- /* Check if last direntry in this block is
|
|
||||||
- reached. */
|
|
||||||
- entries--;
|
|
||||||
- if (!entries)
|
|
||||||
- break;
|
|
||||||
+ /*
|
|
||||||
+ * The expected number of directory entries is only tracked for the
|
|
||||||
+ * single extent case.
|
|
||||||
+ */
|
|
||||||
+ if (dir->inode.nextents == grub_cpu_to_be32_compile_time (1))
|
|
||||||
+ {
|
|
||||||
+ /* Check if last direntry in this block is reached. */
|
|
||||||
+ entries--;
|
|
||||||
+ if (!entries)
|
|
||||||
+ break;
|
|
||||||
+ }
|
|
||||||
|
|
||||||
/* Select the next directory entry. */
|
|
||||||
direntry = grub_xfs_next_de(dir->data, direntry);
|
|
|
@ -342,7 +342,6 @@ Patch0341: 0341-fs-Remove-trailing-whitespaces.patch
|
||||||
Patch0342: 0342-fs-xfs-Fix-memory-leaks-in-XFS-module.patch
|
Patch0342: 0342-fs-xfs-Fix-memory-leaks-in-XFS-module.patch
|
||||||
Patch0343: 0343-fs-xfs-Fix-issues-found-while-fuzzing-the-XFS-filesy.patch
|
Patch0343: 0343-fs-xfs-Fix-issues-found-while-fuzzing-the-XFS-filesy.patch
|
||||||
Patch0344: 0344-fs-xfs-Incorrect-short-form-directory-data-boundary-.patch
|
Patch0344: 0344-fs-xfs-Incorrect-short-form-directory-data-boundary-.patch
|
||||||
Patch0345: 0345-fs-xfs-Fix-XFS-directory-extent-parsing.patch
|
Patch0345: 0345-fs-xfs-Add-large-extent-counters-incompat-feature-su.patch
|
||||||
Patch0346: 0346-fs-xfs-Add-large-extent-counters-incompat-feature-su.patch
|
Patch0346: 0346-chainloader-remove-device-path-debug-message.patch
|
||||||
Patch0347: 0347-chainloader-remove-device-path-debug-message.patch
|
Patch0347: 0347-normal-Remove-grub_env_set-prefix-in-grub_try_normal.patch
|
||||||
Patch0348: 0348-normal-Remove-grub_env_set-prefix-in-grub_try_normal.patch
|
|
||||||
|
|
|
@ -17,7 +17,7 @@
|
||||||
Name: grub2
|
Name: grub2
|
||||||
Epoch: 1
|
Epoch: 1
|
||||||
Version: 2.06
|
Version: 2.06
|
||||||
Release: 114%{?dist}
|
Release: 115%{?dist}
|
||||||
Summary: Bootloader with support for Linux, Multiboot and more
|
Summary: Bootloader with support for Linux, Multiboot and more
|
||||||
License: GPL-3.0-or-later
|
License: GPL-3.0-or-later
|
||||||
URL: http://www.gnu.org/software/grub/
|
URL: http://www.gnu.org/software/grub/
|
||||||
|
@ -554,6 +554,10 @@ mv ${EFI_HOME}/grub.cfg.stb ${EFI_HOME}/grub.cfg
|
||||||
%endif
|
%endif
|
||||||
|
|
||||||
%changelog
|
%changelog
|
||||||
|
* Wed Jan 17 2024 Nicolas Frayer <nfrayer@redhat.com> - 2.06-115
|
||||||
|
- xfs: some bios systems can't boot with one of the xfs upstream patches
|
||||||
|
- Resolves: #2254370
|
||||||
|
|
||||||
* Sat Jan 13 2024 Hector Martin <marcan@fedoraproject.org> - 2.06-114
|
* Sat Jan 13 2024 Hector Martin <marcan@fedoraproject.org> - 2.06-114
|
||||||
- Switch memdisk compression to lzop
|
- Switch memdisk compression to lzop
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue