From ad9716664326dbd610b3905d7b22558b1c759255 Mon Sep 17 00:00:00 2001 From: Tobias Powalowski Date: Fri, 6 Oct 2023 20:59:39 +0200 Subject: [PATCH] upgpkg: 2:2.12rc1-4: fix XFS boundary check #79857 --- .SRCINFO | 6 +- 0005-fix-xfs-boundary-check.patch | 214 ++++++++++++++++++++++++++++++ PKGBUILD | 11 +- 3 files changed, 228 insertions(+), 3 deletions(-) create mode 100644 0005-fix-xfs-boundary-check.patch diff --git a/.SRCINFO b/.SRCINFO index ff8e9c9..297ed02 100644 --- a/.SRCINFO +++ b/.SRCINFO @@ -1,12 +1,12 @@ pkgbase = grub pkgdesc = GNU GRand Unified Bootloader (2) pkgver = 2.12rc1 - pkgrel = 3 + pkgrel = 4 epoch = 2 url = https://www.gnu.org/software/grub/ install = grub.install arch = x86_64 - license = GPL3 + license = GPL-3.0-or-later makedepends = git makedepends = rsync makedepends = xz @@ -55,6 +55,7 @@ pkgbase = grub source = 0002-10_linux-detect-archlinux-initramfs.patch source = 0003-support-dropins-for-default-configuration.patch source = 0004-ntfs-module-security.patch + source = 0005-fix-xfs-boundary-check.patch source = grub.default source = sbat.csv validpgpkeys = E53D497F3FA42AD8C9B4D1E835A93B74E82E4209 @@ -68,6 +69,7 @@ pkgbase = grub sha256sums = 8488aec30a93e8fe66c23ef8c23aefda39c38389530e9e73ba3fbcc8315d244d sha256sums = b5d9fcd62ffb3c3950fdeb7089ec2dc2294ac52e9861980ad90a437dedbd3d47 sha256sums = 4bdd5ceb13dbd4c41fde24163f16a0ba05447d821e74d938a0b9e5fce0431140 + sha256sums = 9f8921b2bacd69bde7ab0c3aff88c678d52c2a625c89264fb92184e7427b819b sha256sums = 7df3f5cb5df7d2dfb17f4c9b5c5dedc9519ddce6f8d2c6cd43d1be17cecb65cb sha256sums = 98b23d41e223bdc0a6e20bdcb3aa77e642f29b64081b1fd2f575314172fc89df diff --git a/0005-fix-xfs-boundary-check.patch b/0005-fix-xfs-boundary-check.patch new file mode 100644 index 0000000..f829ebd --- /dev/null +++ b/0005-fix-xfs-boundary-check.patch @@ -0,0 +1,214 @@ +[PATCH v3] Fix XFS directory extent parsing +From: Jon DeVree +Subject: [PATCH v3] Fix XFS directory extent parsing +Date: Wed, 27 Sep 2023 20:43:55 -0400 + +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 diretory 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. + +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 +--- + +Notes: + Changes from v2: + * Fix bounds check on filename + + Changes from v1: + * Address review feedback + + grub-core/fs/xfs.c | 51 +++++++++++++++++++++++++++++++++------------- + 1 file changed, 37 insertions(+), 14 deletions(-) + +diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c +index b91cd32b4..acdfb1a7b 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; +@@ -877,9 +883,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, +@@ -890,14 +895,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; + +- if (!entries) +- continue; ++ /* 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; +@@ -917,7 +935,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 +@@ -931,11 +949,16 @@ 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.40.1 + +[PATCH 1/1] fs/xfs: Incorrect short form directory data boundary check +From: Lidong Chen +Subject: [PATCH 1/1] fs/xfs: Incorrect short form directory data boundary check +Date: Thu, 28 Sep 2023 22:33:44 +0000 + +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 +--- + 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 +@@ -816,7 +816,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); +@@ -852,10 +852,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.30.2 diff --git a/PKGBUILD b/PKGBUILD index 4aefa8d..0ba8324 100644 --- a/PKGBUILD +++ b/PKGBUILD @@ -22,7 +22,7 @@ _tag='bb59f566e1e5c387dbfd342bb3767f761422c744' # git rev-parse grub-${_pkgver} _pkgver=2.12rc1 _unifont_ver='15.1.02' pkgver=${_pkgver/-/} -pkgrel=3 +pkgrel=4 url='https://www.gnu.org/software/grub/' arch=('x86_64') license=('GPL-3.0-or-later') @@ -64,6 +64,7 @@ source=("git+https://git.savannah.gnu.org/git/grub.git#tag=${_tag}?signed" '0002-10_linux-detect-archlinux-initramfs.patch' '0003-support-dropins-for-default-configuration.patch' '0004-ntfs-module-security.patch' + '0005-fix-xfs-boundary-check.patch' 'grub.default' 'sbat.csv') @@ -75,6 +76,7 @@ sha256sums=('SKIP' '8488aec30a93e8fe66c23ef8c23aefda39c38389530e9e73ba3fbcc8315d244d' 'b5d9fcd62ffb3c3950fdeb7089ec2dc2294ac52e9861980ad90a437dedbd3d47' '4bdd5ceb13dbd4c41fde24163f16a0ba05447d821e74d938a0b9e5fce0431140' + '9f8921b2bacd69bde7ab0c3aff88c678d52c2a625c89264fb92184e7427b819b' '7df3f5cb5df7d2dfb17f4c9b5c5dedc9519ddce6f8d2c6cd43d1be17cecb65cb' '98b23d41e223bdc0a6e20bdcb3aa77e642f29b64081b1fd2f575314172fc89df') @@ -134,9 +136,16 @@ prepare() { echo "Patch to support dropins for default configuration..." patch -Np1 -i "${srcdir}/0003-support-dropins-for-default-configuration.patch" + # #79857 + # https://lists.gnu.org/archive/html/grub-devel/2023-09/msg00113.html + # https://savannah.gnu.org/bugs/?64514 + echo "Patch to fo fix XFS incorrect short form directory data boundary check" + patch -Np1 -i "${srcdir}/0005-fix-xfs-boundary-check.patch" + echo "Patch to fix ntfs module security vulnerabilities" patch -Np1 -i "${srcdir}/0004-ntfs-module-security.patch" + echo "Fix DejaVuSans.ttf location so that grub-mkfont can create *.pf2 files for starfield theme..." sed 's|/usr/share/fonts/dejavu|/usr/share/fonts/dejavu /usr/share/fonts/TTF|g' -i "configure.ac"