From 29406ad33366ac899ae5035cf26589d82652bc9e Mon Sep 17 00:00:00 2001 From: Leo Sandoval Date: Tue, 23 Jan 2024 11:40:22 -0600 Subject: [PATCH] xfs: include directory extent parsing patch Patch is required to boot XFS-formatted partitions created with xfsprogs 6.5.0 Resolves: #2259266 Signed-off-by: Leo Sandoval --- ...xfs-Fix-XFS-directory-extent-parsing.patch | 168 ++++++++++++++++++ grub.patches | 1 + grub2.spec | 8 +- 3 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 0350-fs-xfs-Fix-XFS-directory-extent-parsing.patch diff --git a/0350-fs-xfs-Fix-XFS-directory-extent-parsing.patch b/0350-fs-xfs-Fix-XFS-directory-extent-parsing.patch new file mode 100644 index 0000000..c005731 --- /dev/null +++ b/0350-fs-xfs-Fix-XFS-directory-extent-parsing.patch @@ -0,0 +1,168 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Jon DeVree +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/ +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 +Reviewed-by: Daniel Kiper +Tested-by: Sebastian Andrzej Siewior +Tested-by: Marta Lewandowska +--- + 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); diff --git a/grub.patches b/grub.patches index addf3e5..9df6ce7 100644 --- a/grub.patches +++ b/grub.patches @@ -347,3 +347,4 @@ Patch0346: 0346-chainloader-remove-device-path-debug-message.patch Patch0347: 0347-normal-Remove-grub_env_set-prefix-in-grub_try_normal.patch Patch0348: 0348-add-flag-to-only-search-root-dev.patch Patch0349: 0349-Ignore-warnings-for-incompatible-types.patch +Patch0350: 0350-fs-xfs-Fix-XFS-directory-extent-parsing.patch \ No newline at end of file diff --git a/grub2.spec b/grub2.spec index 34a3c8f..dcc99a4 100644 --- a/grub2.spec +++ b/grub2.spec @@ -17,7 +17,7 @@ Name: grub2 Epoch: 1 Version: 2.06 -Release: 117%{?dist} +Release: 118%{?dist} Summary: Bootloader with support for Linux, Multiboot and more License: GPL-3.0-or-later URL: http://www.gnu.org/software/grub/ @@ -554,6 +554,12 @@ mv ${EFI_HOME}/grub.cfg.stb ${EFI_HOME}/grub.cfg %endif %changelog +* Tue Jan 23 2024 Leo Sandoval - 2.06-118 +- xfs: include the 'directory extent parsing patch', otherwise +XFS-formatted partitions do not boot. This change effectively +reverts 2.06-115 +- Resolves: #2259266 + * Wed Jan 17 2024 Nicolas Frayer - 2.06-117 - Compiler flags: ignore incompatible types for now as it prevents CI builds