From d2d2c88ea6b9183fc7763750a07a3ee0d9fba5d14b5c7c7f9b1667ab9477f66e Mon Sep 17 00:00:00 2001 From: Michael Chang Date: Fri, 17 Nov 2023 06:37:28 +0000 Subject: [PATCH] Accepting request 1126507 from home:michael-chang:branches:Base:System - Fix XFS regression in 2.12~rc1 and support large extent counters * 0001-fs-xfs-Incorrect-short-form-directory-data-boundary-.patch * 0002-fs-xfs-Fix-XFS-directory-extent-parsing.patch * 0003-fs-xfs-add-large-extent-counters-incompat-feature-su.patch OBS-URL: https://build.opensuse.org/request/show/1126507 OBS-URL: https://build.opensuse.org/package/show/Base:System/grub2?expand=0&rev=474 --- ...-short-form-directory-data-boundary-.patch | 51 ++++++ ...xfs-Fix-XFS-directory-extent-parsing.patch | 171 ++++++++++++++++++ ...-extent-counters-incompat-feature-su.patch | 115 ++++++++++++ grub2.changes | 8 + grub2.spec | 3 + 5 files changed, 348 insertions(+) create mode 100644 0001-fs-xfs-Incorrect-short-form-directory-data-boundary-.patch create mode 100644 0002-fs-xfs-Fix-XFS-directory-extent-parsing.patch create mode 100644 0003-fs-xfs-add-large-extent-counters-incompat-feature-su.patch diff --git a/0001-fs-xfs-Incorrect-short-form-directory-data-boundary-.patch b/0001-fs-xfs-Incorrect-short-form-directory-data-boundary-.patch new file mode 100644 index 0000000..7fc6d53 --- /dev/null +++ b/0001-fs-xfs-Incorrect-short-form-directory-data-boundary-.patch @@ -0,0 +1,51 @@ +From b541e93b4dab6f652941d086af4fe2da676d0ee3 Mon Sep 17 00:00:00 2001 +From: Lidong Chen +Date: Thu, 28 Sep 2023 22:33:44 +0000 +Subject: [PATCH 1/3] fs/xfs: Incorrect short form directory data boundary + check + +After parsing of the current entry, the entry pointer is advanced +to the next entry at the end of the "for" loop. In case where the +last entry is at the end of the data boundary, the advanced entry +pointer can point off the data boundary. The subsequent boundary +check for the advanced entry pointer can cause a failure. + +The fix is to include the boundary check into the "for" loop +condition. + +Signed-off-by: Lidong Chen +Reviewed-by: Daniel Kiper +Tested-by: Sebastian Andrzej Siewior +Tested-by: Marta Lewandowska +--- + grub-core/fs/xfs.c | 7 ++----- + 1 file changed, 2 insertions(+), 5 deletions(-) + +diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c +index b91cd32b4..ebf962793 100644 +--- a/grub-core/fs/xfs.c ++++ b/grub-core/fs/xfs.c +@@ -810,7 +810,8 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir, + if (iterate_dir_call_hook (parent, "..", &ctx)) + return 1; + +- for (i = 0; i < head->count; i++) ++ for (i = 0; i < head->count && ++ (grub_uint8_t *) de < ((grub_uint8_t *) dir + grub_xfs_fshelp_size (dir->data)); i++) + { + grub_uint64_t ino; + grub_uint8_t *inopos = grub_xfs_inline_de_inopos(dir->data, de); +@@ -845,10 +846,6 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir, + de->name[de->len] = c; + + de = grub_xfs_inline_next_de(dir->data, head, de); +- +- if ((grub_uint8_t *) de >= (grub_uint8_t *) dir + grub_xfs_fshelp_size (dir->data)) +- return grub_error (GRUB_ERR_BAD_FS, "invalid XFS directory entry"); +- + } + break; + } +-- +2.42.1 + diff --git a/0002-fs-xfs-Fix-XFS-directory-extent-parsing.patch b/0002-fs-xfs-Fix-XFS-directory-extent-parsing.patch new file mode 100644 index 0000000..02fca17 --- /dev/null +++ b/0002-fs-xfs-Fix-XFS-directory-extent-parsing.patch @@ -0,0 +1,171 @@ +From 4a6a5c4a6bb2426235364be9f3698763ddcf4775 Mon Sep 17 00:00:00 2001 +From: Jon DeVree +Date: Tue, 17 Oct 2023 23:03:47 -0400 +Subject: [PATCH 2/3] 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 ebf962793..18edfcff4 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); ++ ++ 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); + +- if (!entries) +- continue; ++ 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); +-- +2.42.1 + diff --git a/0003-fs-xfs-add-large-extent-counters-incompat-feature-su.patch b/0003-fs-xfs-add-large-extent-counters-incompat-feature-su.patch new file mode 100644 index 0000000..2edee66 --- /dev/null +++ b/0003-fs-xfs-add-large-extent-counters-incompat-feature-su.patch @@ -0,0 +1,115 @@ +From e7b1a524d5f86dcfddfbb069577e3b148dbb19cd Mon Sep 17 00:00:00 2001 +From: Anthony Iliopoulos via Grub-devel +Date: Thu, 26 Oct 2023 11:53:39 +0200 +Subject: [PATCH 3/3] fs/xfs: add large extent counters incompat feature + support + +XFS introduced 64-bit extent counters for inodes via a series of +upstream commits, and the feature was marked as stable in v6.5 via +commit 61d7e8274cd8 ("xfs: drop EXPERIMENTAL tag for large extent +counts"). + +Further, xfsprogs release v6.5.0 switched this feature on by default in +mkfs.xfs via commit e5b18d7d1d96 ("mkfs: enable large extent counts by +default"). + +Filesystems formatted with large extent count support (nrext64=1) are +thus currently not recognizable by grub, since this is an incompat +feature. Add the required support so that those filesystems and inodes +with large extent counters can be read by grub. + +Signed-off-by: Anthony Iliopoulos +--- + grub-core/fs/xfs.c | 30 +++++++++++++++++++++++++----- + 1 file changed, 25 insertions(+), 5 deletions(-) + +diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c +index 18edfcff4..bc2224dbb 100644 +--- a/grub-core/fs/xfs.c ++++ b/grub-core/fs/xfs.c +@@ -79,6 +79,8 @@ GRUB_MOD_LICENSE ("GPLv3+"); + /* Inode flags2 flags */ + #define XFS_DIFLAG2_BIGTIME_BIT 3 + #define XFS_DIFLAG2_BIGTIME (1 << XFS_DIFLAG2_BIGTIME_BIT) ++#define XFS_DIFLAG2_NREXT64_BIT 4 ++#define XFS_DIFLAG2_NREXT64 (1 << XFS_DIFLAG2_NREXT64_BIT) + + /* incompat feature flags */ + #define XFS_SB_FEAT_INCOMPAT_FTYPE (1 << 0) /* filetype in dirent */ +@@ -86,6 +88,7 @@ GRUB_MOD_LICENSE ("GPLv3+"); + #define XFS_SB_FEAT_INCOMPAT_META_UUID (1 << 2) /* metadata UUID */ + #define XFS_SB_FEAT_INCOMPAT_BIGTIME (1 << 3) /* large timestamps */ + #define XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR (1 << 4) /* needs xfs_repair */ ++#define XFS_SB_FEAT_INCOMPAT_NREXT64 (1 << 5) /* large extent counters */ + + /* + * Directory entries with ftype are explicitly handled by GRUB code. +@@ -101,7 +104,8 @@ GRUB_MOD_LICENSE ("GPLv3+"); + XFS_SB_FEAT_INCOMPAT_SPINODES | \ + XFS_SB_FEAT_INCOMPAT_META_UUID | \ + XFS_SB_FEAT_INCOMPAT_BIGTIME | \ +- XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR) ++ XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR | \ ++ XFS_SB_FEAT_INCOMPAT_NREXT64) + + struct grub_xfs_sblock + { +@@ -203,7 +207,8 @@ struct grub_xfs_inode + grub_uint16_t mode; + grub_uint8_t version; + grub_uint8_t format; +- grub_uint8_t unused2[26]; ++ grub_uint8_t unused2[18]; ++ grub_uint64_t nextents_big; + grub_uint64_t atime; + grub_uint64_t mtime; + grub_uint64_t ctime; +@@ -545,11 +550,26 @@ get_fsb (const void *keys, int idx) + return grub_be_to_cpu64 (grub_get_unaligned64 (p)); + } + ++static int ++grub_xfs_inode_has_large_extent_counts (const struct grub_xfs_inode *inode) ++{ ++ return inode->version >= 3 && ++ (inode->flags2 & grub_cpu_to_be64_compile_time (XFS_DIFLAG2_NREXT64)); ++} ++ ++static grub_uint64_t ++grub_xfs_get_inode_nextents (struct grub_xfs_inode *inode) ++{ ++ return (grub_xfs_inode_has_large_extent_counts (inode)) ? ++ grub_be_to_cpu64 (inode->nextents_big) : ++ grub_be_to_cpu32 (inode->nextents); ++} ++ + static grub_disk_addr_t + grub_xfs_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock) + { + struct grub_xfs_btree_node *leaf = 0; +- int ex, nrec; ++ grub_uint64_t ex, nrec; + struct grub_xfs_extent *exts; + grub_uint64_t ret = 0; + +@@ -574,7 +594,7 @@ grub_xfs_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock) + / (2 * sizeof (grub_uint64_t)); + do + { +- int i; ++ grub_uint64_t i; + + for (i = 0; i < nrec; i++) + { +@@ -621,7 +641,7 @@ grub_xfs_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock) + grub_addr_t exts_end = 0; + grub_addr_t data_end = 0; + +- nrec = grub_be_to_cpu32 (node->inode.nextents); ++ nrec = grub_xfs_get_inode_nextents (&node->inode); + exts = (struct grub_xfs_extent *) grub_xfs_inode_data(&node->inode); + + if (grub_mul (sizeof (struct grub_xfs_extent), nrec, &exts_end) || +-- +2.42.1 + diff --git a/grub2.changes b/grub2.changes index 53e4849..287314c 100644 --- a/grub2.changes +++ b/grub2.changes @@ -1,3 +1,11 @@ +------------------------------------------------------------------- +Tue Nov 14 07:52:41 UTC 2023 - Michael Chang + +- Fix XFS regression in 2.12~rc1 and support large extent counters + * 0001-fs-xfs-Incorrect-short-form-directory-data-boundary-.patch + * 0002-fs-xfs-Fix-XFS-directory-extent-parsing.patch + * 0003-fs-xfs-add-large-extent-counters-incompat-feature-su.patch + ------------------------------------------------------------------- Mon Oct 30 07:15:17 UTC 2023 - Michael Chang diff --git a/grub2.spec b/grub2.spec index 3b00527..e85a7ec 100644 --- a/grub2.spec +++ b/grub2.spec @@ -390,6 +390,9 @@ Patch197: 0006-fs-ntfs-Make-code-more-readable.patch Patch198: 0001-luks2-Use-grub-tpm2-token-for-TPM2-protected-volume-.patch Patch199: Fix-the-size-calculation-for-the-synthesized-initrd.patch Patch200: 0001-kern-ieee1275-init-Restrict-high-memory-in-presence-.patch +Patch201: 0001-fs-xfs-Incorrect-short-form-directory-data-boundary-.patch +Patch202: 0002-fs-xfs-Fix-XFS-directory-extent-parsing.patch +Patch203: 0003-fs-xfs-add-large-extent-counters-incompat-feature-su.patch Requires: gettext-runtime %if 0%{?suse_version} >= 1140