rpms/grub2
7
0
Fork 0
Code Issues Pull Requests Releases Activity

import CS grub2-2.06-77.el9

Browse Source
This commit is contained in:
Andrew Lukoshko 2024-03-28 13:39:21 +00:00
parent 3f5d404f9b
commit a1db1ec7f5
18 changed files with 1576 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

226
SOURCES/0329-kern-ieee1275-init-ppc64-Restrict-high-memory-in-pre.patch Normal file
View File

@ -0,0 +1,226 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Stefan Berger <stefanb@linux.ibm.com>
Date: Tue, 25 Jul 2023 13:23:10 -0400
Subject: [PATCH] kern/ieee1275/init: ppc64: Restrict high memory in presence
of fadump
When a kernel dump is present then restrict the high memory regions to
avoid allocating memory where the kernel dump resides. Use the
ibm,kernel-dump node under /rtas to determine whether a kernel dump exists
and up to which limit grub can use available memory. Set the
upper_mem_limit to the size of the kernel dump section of type
'REAL_MODE_REGION' and therefore only allow grub's memory usage for high
addresses from RMO_ADDR_MAX to 'upper_mem_limit'. This means that grub can
use high memory in the range of RMO_ADDR_MAX (768MB) to upper_mem_limit and
the kernel-dump memory regions above 'upper_mem_limit' remain untouched.
This change has no effect on memory allocations below 'linux_rmo_save'
(typically at 640MB).
Also, fall back to allocating below rmo_linux_save in case the chunk of
memory there would be larger than the chunk of memory above RMO_ADDR_MAX.
This can for example occur if a free memory area is found starting at 300MB
extending up to 1GB but a kernel dump is located at 768MB and therefore
does not allow the allocation of the high memory area but requiring to use
the chunk starting at 300MB to avoid an unnecessary out-of-memory
condition.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Hari Bathini <hbathini@linux.ibm.com>
Cc: Pavithra Prakash <pavrampu@in.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Carolyn Scherrer <cpscherr@us.ibm.com>
Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Cc: Sourabh Jain <sourabhjain@linux.ibm.com>
---
grub-core/kern/ieee1275/init.c | 144 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 142 insertions(+), 2 deletions(-)
diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
index 3d4ad9d..8e7f742 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -17,6 +17,8 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
+#include <stddef.h> /* offsetof() */
+
#include <grub/kernel.h>
#include <grub/dl.h>
#include <grub/disk.h>
@@ -198,6 +200,96 @@ grub_claim_heap (void)
#else
/* Helpers for mm on powerpc. */
+/* ibm,kernel-dump data structures */
+struct kd_section
+{
+ grub_uint32_t flags;
+ grub_uint16_t src_datatype;
+#define KD_SRC_DATATYPE_REAL_MODE_REGION 0x0011
+ grub_uint16_t error_flags;
+ grub_uint64_t src_address;
+ grub_uint64_t num_bytes;
+ grub_uint64_t act_bytes;
+ grub_uint64_t dst_address;
+} GRUB_PACKED;
+
+#define MAX_KD_SECTIONS 10
+
+struct kernel_dump
+{
+ grub_uint32_t format;
+ grub_uint16_t num_sections;
+ grub_uint16_t status_flags;
+ grub_uint32_t offset_1st_section;
+ grub_uint32_t num_blocks;
+ grub_uint64_t start_block;
+ grub_uint64_t num_blocks_avail;
+ grub_uint32_t offet_path_string;
+ grub_uint32_t max_time_allowed;
+ struct kd_section kds[MAX_KD_SECTIONS]; /* offset_1st_section should point to kds[0] */
+} GRUB_PACKED;
+
+/*
+ * Determine if a kernel dump exists and if it does, then determine the highest
+ * address that grub can use for memory allocations.
+ * The caller must have initialized *highest to rmo_top. *highest will not
+ * be modified if no kernel dump is found.
+ */
+static void
+check_kernel_dump (grub_uint64_t *highest)
+{
+ struct kernel_dump kernel_dump;
+ grub_ssize_t kernel_dump_size;
+ grub_ieee1275_phandle_t rtas;
+ struct kd_section *kds;
+ grub_size_t i;
+
+ /* If there's a kernel-dump it must have at least one section */
+ if (grub_ieee1275_finddevice ("/rtas", &rtas) ||
+ grub_ieee1275_get_property (rtas, "ibm,kernel-dump", &kernel_dump,
+ sizeof (kernel_dump), &kernel_dump_size) ||
+ kernel_dump_size <= (grub_ssize_t) offsetof (struct kernel_dump, kds[1]))
+ return;
+
+ kernel_dump_size = grub_min (kernel_dump_size, (grub_ssize_t) sizeof (kernel_dump));
+
+ if (grub_be_to_cpu32 (kernel_dump.format) != 1)
+ {
+ grub_printf (_("Error: ibm,kernel-dump has an unexpected format version '%u'\n"),
+ grub_be_to_cpu32 (kernel_dump.format));
+ return;
+ }
+
+ if (grub_be_to_cpu16 (kernel_dump.num_sections) > MAX_KD_SECTIONS)
+ {
+ grub_printf (_("Error: Too many kernel dump sections: %d\n"),
+ grub_be_to_cpu32 (kernel_dump.num_sections));
+ return;
+ }
+
+ for (i = 0; i < grub_be_to_cpu16 (kernel_dump.num_sections); i++)
+ {
+ kds = (struct kd_section *) ((grub_addr_t) &kernel_dump +
+ grub_be_to_cpu32 (kernel_dump.offset_1st_section) +
+ i * sizeof (struct kd_section));
+ /* sanity check the address is within the 'kernel_dump' struct */
+ if ((grub_addr_t) kds > (grub_addr_t) &kernel_dump + kernel_dump_size + sizeof (*kds))
+ {
+ grub_printf (_("Error: 'kds' address beyond last available section\n"));
+ return;
+ }
+
+ if ((grub_be_to_cpu16 (kds->src_datatype) == KD_SRC_DATATYPE_REAL_MODE_REGION) &&
+ (grub_be_to_cpu64 (kds->src_address) == 0))
+ {
+ *highest = grub_min (*highest, grub_be_to_cpu64 (kds->num_bytes));
+ break;
+ }
+ }
+
+ return;
+}
+
/*
* How much memory does OF believe exists in total?
*
@@ -277,10 +369,31 @@ regions_claim (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
*
* Finally, we also want to make sure that when grub loads the kernel,
* it isn't going to use up all the memory we're trying to reserve! So
- * enforce our entire RUNTIME_MIN_SPACE here:
+ * enforce our entire RUNTIME_MIN_SPACE here (no fadump):
+ *
+ * | Top of memory == upper_mem_limit -|
+ * | |
+ * | available |
+ * | |
+ * |---------- 768 MB ----------|
+ * | |
+ * | reserved |
+ * | |
+ * |--- 768 MB - runtime min space ---|
+ * | |
+ * | available |
+ * | |
+ * |---------- 0 MB ----------|
+ *
+ * In case fadump is used, we allow the following:
*
* |---------- Top of memory ----------|
* | |
+ * | unavailable |
+ * | (kernel dump area) |
+ * | |
+ * |--------- upper_mem_limit ---------|
+ * | |
* | available |
* | |
* |---------- 768 MB ----------|
@@ -335,17 +448,44 @@ regions_claim (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
}
else
{
+ grub_uint64_t upper_mem_limit = rmo_top;
+ grub_uint64_t orig_addr = addr;
+
+ check_kernel_dump (&upper_mem_limit);
+
/*
* we order these cases to prefer higher addresses and avoid some
* splitting issues
+ * The following shows the order of variables:
+ * no kernel dump: linux_rmo_save < RMO_ADDR_MAX <= upper_mem_limit == rmo_top
+ * with kernel dump: liuxx_rmo_save < RMO_ADDR_MAX <= upper_mem_limit <= rmo_top
*/
- if (addr < RMO_ADDR_MAX && (addr + len) > RMO_ADDR_MAX)
+ if (addr < RMO_ADDR_MAX && (addr + len) > RMO_ADDR_MAX && upper_mem_limit >= RMO_ADDR_MAX)
{
grub_dprintf ("ieee1275",
"adjusting region for RUNTIME_MIN_SPACE: (%llx -> %llx) -> (%llx -> %llx)\n",
addr, addr + len, RMO_ADDR_MAX, addr + len);
len = (addr + len) - RMO_ADDR_MAX;
addr = RMO_ADDR_MAX;
+
+ /* We must not exceed the upper_mem_limit (assuming it's >= RMO_ADDR_MAX) */
+ if (addr + len > upper_mem_limit)
+ {
+ /* take the bigger chunk from either below linux_rmo_save or above upper_mem_limit */
+ len = upper_mem_limit - addr;
+ if (orig_addr < linux_rmo_save && linux_rmo_save - orig_addr > len)
+ {
+ /* lower part is bigger */
+ addr = orig_addr;
+ len = linux_rmo_save - addr;
+ }
+
+ grub_dprintf ("ieee1275", "re-adjusted region to: (%llx -> %llx)\n",
+ addr, addr + len);
+
+ if (len == 0)
+ return 0;
+ }
}
else if ((addr < linux_rmo_save) && ((addr + len) > linux_rmo_save))
{

27
SOURCES/0330-normal-Remove-grub_env_set-prefix-in-grub_try_normal.patch Normal file
View File

@ -0,0 +1,27 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Nicolas Frayer <nfrayer@redhat.com>
Date: Tue, 19 Dec 2023 16:52:05 +0100
Subject: [PATCH] normal: Remove grub_env_set prefix in grub_try_normal_prefix
Commit de735a453aa35 added a grub_env_set where the prefix contains
the arch name in the pathname. This create issues when trying to
load modules using this prefix as the pathname contains a "doubled"
arch name in it (ie .../powerpc-ieee1275/powerpc-ieee1275/).
Signed-off-by: Nicolas Frayer <nfrayer@redhat.com>
---
grub-core/normal/main.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/grub-core/normal/main.c b/grub-core/normal/main.c
index d59145f861d5..bac7b8a0e1d8 100644
--- a/grub-core/normal/main.c
+++ b/grub-core/normal/main.c
@@ -372,7 +372,6 @@ grub_try_normal_prefix (const char *prefix)
file = grub_file_open (config, GRUB_FILE_TYPE_CONFIG);
if (file)
{
- grub_env_set ("prefix", prefix);
grub_file_close (file);
err = GRUB_ERR_NONE;
}

159
SOURCES/0331-search-command-add-flag-to-only-search-root-dev.patch Normal file
View File

@ -0,0 +1,159 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Marta Lewandowska <mlewando@redhat.com>
Date: Mon, 9 Oct 2023 08:53:18 +0200
Subject: [PATCH] search command: add flag to only search root dev
bz#2223437
Signed-off-by: Marta Lewandowska <mlewando@redhat.com>
---
grub-core/commands/search.c | 36 ++++++++++++++++++++++++++++++++++++
grub-core/commands/search_wrap.c | 5 +++++
grub-core/kern/misc.c | 30 ++++++++++++++++++++++++++++++
include/grub/misc.h | 1 +
include/grub/search.h | 3 ++-
5 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/grub-core/commands/search.c b/grub-core/commands/search.c
index 57d26ced8a8e..94fe8b2872a1 100644
--- a/grub-core/commands/search.c
+++ b/grub-core/commands/search.c
@@ -85,6 +85,42 @@ iterate_device (const char *name, void *data)
grub_device_close (dev);
}
+ /* Skip it if it's not the root device when requested. */
+ if (ctx->flags & SEARCH_FLAGS_ROOTDEV_ONLY)
+ {
+ const char *root_dev;
+ root_dev = grub_env_get ("root");
+ if (root_dev != NULL && *root_dev != '\0')
+ {
+ char *root_disk = grub_malloc (grub_strlen(root_dev) + 1);
+ char *name_disk = grub_malloc (grub_strlen(name) + 1);
+ char *rem_1 = grub_malloc(grub_strlen(root_dev) + 1);
+ char *rem_2 = grub_malloc(grub_strlen(name) + 1);
+
+ if (root_disk != NULL && name_disk != NULL &&
+ rem_1 != NULL && rem_2 != NULL)
+ {
+ /* get just the disk name; partitions will be different. */
+ grub_str_sep (root_dev, root_disk, ',', rem_1);
+ grub_str_sep (name, name_disk, ',', rem_2);
+ if (root_disk != NULL && *root_disk != '\0' &&
+ name_disk != NULL && *name_disk != '\0')
+ if (grub_strcmp(root_disk, name_disk) != 0)
+ {
+ grub_free (root_disk);
+ grub_free (name_disk);
+ grub_free (rem_1);
+ grub_free (rem_2);
+ return 0;
+ }
+ }
+ grub_free (root_disk);
+ grub_free (name_disk);
+ grub_free (rem_1);
+ grub_free (rem_2);
+ }
+ }
+
#ifdef DO_SEARCH_FS_UUID
#define compare_fn grub_strcasecmp
#else
diff --git a/grub-core/commands/search_wrap.c b/grub-core/commands/search_wrap.c
index 0b62acf85359..06b5f51eefb5 100644
--- a/grub-core/commands/search_wrap.c
+++ b/grub-core/commands/search_wrap.c
@@ -41,6 +41,7 @@ static const struct grub_arg_option options[] =
ARG_TYPE_STRING},
{"no-floppy", 'n', 0, N_("Do not probe any floppy drive."), 0, 0},
{"efidisk-only", 0, 0, N_("Only probe EFI disks."), 0, 0},
+ {"root-dev-only", 'r', 0, N_("Only probe root device."), 0, 0},
{"hint", 'h', GRUB_ARG_OPTION_REPEATABLE,
N_("First try the device HINT. If HINT ends in comma, "
"also try subpartitions"), N_("HINT"), ARG_TYPE_STRING},
@@ -75,6 +76,7 @@ enum options
SEARCH_SET,
SEARCH_NO_FLOPPY,
SEARCH_EFIDISK_ONLY,
+ SEARCH_ROOTDEV_ONLY,
SEARCH_HINT,
SEARCH_HINT_IEEE1275,
SEARCH_HINT_BIOS,
@@ -189,6 +191,9 @@ grub_cmd_search (grub_extcmd_context_t ctxt, int argc, char **args)
if (state[SEARCH_EFIDISK_ONLY].set)
flags |= SEARCH_FLAGS_EFIDISK_ONLY;
+ if (state[SEARCH_ROOTDEV_ONLY].set)
+ flags |= SEARCH_FLAGS_ROOTDEV_ONLY;
+
if (state[SEARCH_LABEL].set)
grub_search_label (id, var, flags, hints, nhints);
else if (state[SEARCH_FS_UUID].set)
diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
index cb454614022f..50af9ee1bdd9 100644
--- a/grub-core/kern/misc.c
+++ b/grub-core/kern/misc.c
@@ -619,6 +619,36 @@ grub_reverse (char *str)
}
}
+/* Separate string into two parts, broken up by delimiter delim. */
+void
+grub_str_sep (const char *s, char *p, char delim, char *r)
+{
+ char* t = grub_strndup(s, grub_strlen(s));
+
+ if (t != NULL && *t != '\0')
+ {
+ char* tmp = t;
+
+ while (((*p = *t) != '\0') && ((*p = *t) != delim))
+ {
+ p++;
+ t++;
+ }
+ *p = '\0';
+
+ if (*t != '\0')
+ {
+ t++;
+ while ((*r++ = *t++) != '\0')
+ ;
+ *r = '\0';
+ }
+ grub_free (tmp);
+ }
+ else
+ grub_free (t);
+}
+
/* Divide N by D, return the quotient, and store the remainder in *R. */
grub_uint64_t
grub_divmod64 (grub_uint64_t n, grub_uint64_t d, grub_uint64_t *r)
diff --git a/include/grub/misc.h b/include/grub/misc.h
index faae0ae8606c..981526644d29 100644
--- a/include/grub/misc.h
+++ b/include/grub/misc.h
@@ -314,6 +314,7 @@ void *EXPORT_FUNC(grub_memset) (void *s, int c, grub_size_t n);
grub_size_t EXPORT_FUNC(grub_strlen) (const char *s) WARN_UNUSED_RESULT;
int EXPORT_FUNC(grub_printf) (const char *fmt, ...) __attribute__ ((format (GNU_PRINTF, 1, 2)));
int EXPORT_FUNC(grub_printf_) (const char *fmt, ...) __attribute__ ((format (GNU_PRINTF, 1, 2)));
+void EXPORT_FUNC(grub_str_sep) (const char *s, char *p, char delim, char *r);
/* Replace all `ch' characters of `input' with `with' and copy the
result into `output'; return EOS address of `output'. */
diff --git a/include/grub/search.h b/include/grub/search.h
index 4190aeb2cbf5..321d1400e451 100644
--- a/include/grub/search.h
+++ b/include/grub/search.h
@@ -22,7 +22,8 @@
enum search_flags
{
SEARCH_FLAGS_NO_FLOPPY = 1,
- SEARCH_FLAGS_EFIDISK_ONLY = 2
+ SEARCH_FLAGS_EFIDISK_ONLY = 2,
+ SEARCH_FLAGS_ROOTDEV_ONLY = 4
};
void grub_search_fs_file (const char *key, const char *var,

146
SOURCES/0332-grub-set-bootflag-Conservative-partial-fix-for-CVE-2.patch Normal file
View File

@ -0,0 +1,146 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Solar Designer <solar@openwall.com>
Date: Tue, 6 Feb 2024 21:39:41 +0100
Subject: [PATCH] grub-set-bootflag: Conservative partial fix for CVE-2024-1048
Following up on CVE-2019-14865 and taking a fresh look at
grub2-set-bootflag now (through my work at CIQ on Rocky Linux), I saw some
other ways in which users could still abuse this little program:
1. After CVE-2019-14865 fix, grub2-set-bootflag no longer rewrites the
grubenv file in-place, but writes into a temporary file and renames it
over the original, checking for error returns from each call first.
This prevents the original file truncation vulnerability, but it can
leave the temporary file around if the program is killed before it can
rename or remove the file. There are still many ways to get the program
killed, such as through RLIMIT_FSIZE triggering SIGXFSZ (tested,
reliable) or by careful timing (tricky) of signals sent by process group
leader, pty, pre-scheduled timers, SIGXCPU (probably not an exhaustive
list). Invoking the program multiple times fills up /boot (or if /boot
is not separate, then it can fill up the root filesystem). Since the
files are tiny, the filesystem is likely to run out of free inodes
before it'd run out of blocks, but the effect is similar - can't create
new files after this point (but still can add data to existing files,
such as logs).
2. After CVE-2019-14865 fix, grub2-set-bootflag naively tries to protect
itself from signals by becoming full root. (This does protect it from
signals sent by the user directly to the PID, but e.g. "kill -9 -1" by
the user still works.) A side effect of such "protection" is that it's
possible to invoke more concurrent instances of grub2-set-bootflag than
the user's RLIMIT_NPROC would normally permit (as specified e.g. in
/etc/security/limits.conf, or say in Apache httpd's RLimitNPROC if
grub2-set-bootflag would be abused by a website script), thereby
exhausting system resources (e.g., bypassing RAM usage limit if
RLIMIT_AS was also set).
3. umask is inherited. Again, due to how the CVE-2019-14865 fix creates
a new file, and due to how mkstemp() works, this affects grubenv's new
file permissions. Luckily, mkstemp() forces them to be no more relaxed
than 0600, but the user ends up being able to set them e.g. to 0.
Luckily, at least in my testing GRUB still works fine even when the file
has such (lack of) permissions.
This commit deals with the abuses above as follows:
1. RLIMIT_FSIZE is pre-checked, so this specific way to get the process
killed should no longer work. However, this isn't a complete fix
because there are other ways to get the process killed after it has
created the temporary file.
The commit also fixes bug 1975892 ("RFE: grub2-set-bootflag should not
write the grubenv when the flag being written is already set") and
similar for "menu_show_once", which further reduces the abuse potential.
2. RLIMIT_NPROC bypass should be avoided by not becoming full root (aka
dropping the partial "kill protection").
3. A safe umask is set.
This is a partial fix (temporary files can still accumulate, but this is
harder to trigger).
While at it, this commit also fixes potential 1- or 2-byte over-read of
env[] if its content is malformed - this was not a security issue since the
grubenv file is trusted input, and the fix is just for robustness.
Signed-off-by: Solar Designer <solar@openwall.com>
---
util/grub-set-bootflag.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/util/grub-set-bootflag.c b/util/grub-set-bootflag.c
index 3b4c25ca2ac6..5bbbef804391 100644
--- a/util/grub-set-bootflag.c
+++ b/util/grub-set-bootflag.c
@@ -33,6 +33,8 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/resource.h>
#include "progname.h"
@@ -57,12 +59,17 @@ static void usage(FILE *out)
int main(int argc, char *argv[])
{
/* NOTE buf must be at least the longest bootflag length + 4 bytes */
- char env[GRUBENV_SIZE + 1], buf[64], *s;
+ char env[GRUBENV_SIZE + 1 + 2], buf[64], *s;
/* +1 for 0 termination, +6 for "XXXXXX" in tmp filename */
char env_filename[PATH_MAX + 1], tmp_filename[PATH_MAX + 6 + 1];
const char *bootflag;
int i, fd, len, ret;
FILE *f;
+ struct rlimit rlim;
+
+ if (getrlimit(RLIMIT_FSIZE, &rlim) || rlim.rlim_cur < GRUBENV_SIZE || rlim.rlim_max < GRUBENV_SIZE)
+ return 1;
+ umask(077);
if (argc != 2)
{
@@ -94,20 +101,11 @@ int main(int argc, char *argv[])
len = strlen (bootflag);
/*
- * Really become root. setuid avoids an user killing us, possibly leaking
- * the tmpfile. setgid avoids the new grubenv's gid being that of the user.
+ * setegid avoids the new grubenv's gid being that of the user.
*/
- ret = setuid(0);
- if (ret)
+ if (setegid(0))
{
- perror ("Error setuid(0) failed");
- return 1;
- }
-
- ret = setgid(0);
- if (ret)
- {
- perror ("Error setgid(0) failed");
+ perror ("Error setegid(0) failed");
return 1;
}
@@ -136,6 +134,9 @@ int main(int argc, char *argv[])
/* 0 terminate env */
env[GRUBENV_SIZE] = 0;
+ /* not a valid flag value */
+ env[GRUBENV_SIZE + 1] = 0;
+ env[GRUBENV_SIZE + 2] = 0;
if (strncmp (env, GRUB_ENVBLK_SIGNATURE, strlen (GRUB_ENVBLK_SIGNATURE)))
{
@@ -171,6 +172,8 @@ int main(int argc, char *argv[])
/* The grubenv is not 0 terminated, so memcpy the name + '=' , '1', '\n' */
snprintf(buf, sizeof(buf), "%s=1\n", bootflag);
+ if (!memcmp(s, buf, len + 3))
+ return 0; /* nothing to do */
memcpy(s, buf, len + 3);

187
SOURCES/0333-grub-set-bootflag-More-complete-fix-for-CVE-2024-104.patch Normal file
View File

@ -0,0 +1,187 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Solar Designer <solar@openwall.com>
Date: Tue, 6 Feb 2024 21:56:21 +0100
Subject: [PATCH] grub-set-bootflag: More complete fix for CVE-2024-1048
Switch to per-user fixed temporary filenames along with a weird locking
mechanism, which is explained in source code comments. This is a more
complete fix than the previous commit (temporary files can't accumulate).
Unfortunately, it introduces new risks (by working on a temporary file
shared between the user's invocations), which are _hopefully_ avoided by
the patch's elaborate logic. I actually got it wrong at first, which
suggests that this logic is hard to reason about, and more errors or
omissions are possible. It also relies on the kernel's primitives' exact
semantics to a greater extent (nothing out of the ordinary, though).
Remaining issues that I think cannot reasonably be fixed without a
redesign (e.g., having per-flag files with nothing else in them) and
without introducing new issues:
A. A user can still revert a concurrent user's attempt of setting the
other flag - or of making other changes to grubenv by means other than
this program.
B. One leftover temporary file per user is still possible.
Signed-off-by: Solar Designer <solar@openwall.com>
---
util/grub-set-bootflag.c | 95 ++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 79 insertions(+), 16 deletions(-)
diff --git a/util/grub-set-bootflag.c b/util/grub-set-bootflag.c
index 5bbbef804391..514c4f9091ac 100644
--- a/util/grub-set-bootflag.c
+++ b/util/grub-set-bootflag.c
@@ -33,6 +33,7 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <sys/file.h>
#include <sys/stat.h>
#include <sys/resource.h>
@@ -60,15 +61,12 @@ int main(int argc, char *argv[])
{
/* NOTE buf must be at least the longest bootflag length + 4 bytes */
char env[GRUBENV_SIZE + 1 + 2], buf[64], *s;
- /* +1 for 0 termination, +6 for "XXXXXX" in tmp filename */
- char env_filename[PATH_MAX + 1], tmp_filename[PATH_MAX + 6 + 1];
+ /* +1 for 0 termination, +11 for ".%u" in tmp filename */
+ char env_filename[PATH_MAX + 1], tmp_filename[PATH_MAX + 11 + 1];
const char *bootflag;
int i, fd, len, ret;
FILE *f;
- struct rlimit rlim;
- if (getrlimit(RLIMIT_FSIZE, &rlim) || rlim.rlim_cur < GRUBENV_SIZE || rlim.rlim_max < GRUBENV_SIZE)
- return 1;
umask(077);
if (argc != 2)
@@ -105,7 +103,7 @@ int main(int argc, char *argv[])
*/
if (setegid(0))
{
- perror ("Error setegid(0) failed");
+ perror ("setegid(0) failed");
return 1;
}
@@ -176,19 +174,82 @@ int main(int argc, char *argv[])
return 0; /* nothing to do */
memcpy(s, buf, len + 3);
+ struct rlimit rlim;
+ if (getrlimit(RLIMIT_FSIZE, &rlim) || rlim.rlim_cur < GRUBENV_SIZE || rlim.rlim_max < GRUBENV_SIZE)
+ {
+ fprintf (stderr, "Resource limits undetermined or too low\n");
+ return 1;
+ }
+
+ /*
+ * Here we work under the premise that we shouldn't write into the target
+ * file directly because we might not be able to have all of our changes
+ * written completely and atomically. That was CVE-2019-14865, known to
+ * have been triggerable via RLIMIT_FSIZE. While we've dealt with that
+ * specific attack via the check above, there may be other possibilities.
+ */
/*
* Create a tempfile for writing the new env. Use the canonicalized filename
* for the template so that the tmpfile is in the same dir / on same fs.
+ *
+ * We now use per-user fixed temporary filenames, so that a user cannot cause
+ * multiple files to accumulate.
+ *
+ * We don't use O_EXCL so that a stale temporary file doesn't prevent further
+ * usage of the program by the user.
*/
- snprintf(tmp_filename, sizeof(tmp_filename), "%sXXXXXX", env_filename);
- fd = mkstemp(tmp_filename);
+ snprintf(tmp_filename, sizeof(tmp_filename), "%s.%u", env_filename, getuid());
+ fd = open(tmp_filename, O_CREAT | O_WRONLY, 0600);
if (fd == -1)
{
perror ("Creating tmpfile failed");
return 1;
}
+ /*
+ * The lock prevents the same user from reaching further steps ending in
+ * rename() concurrently, in which case the temporary file only partially
+ * written by one invocation could be renamed to the target file by another.
+ *
+ * The lock also guards the slow fsync() from concurrent calls. After the
+ * first time that and the rename() complete, further invocations for the
+ * same flag become no-ops.
+ *
+ * We lock the temporary file rather than the target file because locking the
+ * latter would allow any user having SIGSTOP'ed their process to make all
+ * other users' invocations fail (or lock up if we'd use blocking mode).
+ *
+ * We use non-blocking mode (LOCK_NB) because the lock having been taken by
+ * another process implies that the other process would normally have already
+ * renamed the file to target by the time it releases the lock (and we could
+ * acquire it), so we'd be working directly on the target if we proceeded,
+ * which is undesirable, and we'd kind of fail on the already-done rename.
+ */
+ if (flock(fd, LOCK_EX | LOCK_NB))
+ {
+ perror ("Locking tmpfile failed");
+ return 1;
+ }
+
+ /*
+ * Deal with the potential that another invocation proceeded all the way to
+ * rename() and process exit while we were between open() and flock().
+ */
+ {
+ struct stat st1, st2;
+ if (fstat(fd, &st1) || stat(tmp_filename, &st2))
+ {
+ perror ("stat of tmpfile failed");
+ return 1;
+ }
+ if (st1.st_dev != st2.st_dev || st1.st_ino != st2.st_ino)
+ {
+ fprintf (stderr, "Another invocation won race\n");
+ return 1;
+ }
+ }
+
f = fdopen (fd, "w");
if (!f)
{
@@ -213,6 +274,14 @@ int main(int argc, char *argv[])
return 1;
}
+ ret = ftruncate (fileno (f), GRUBENV_SIZE);
+ if (ret)
+ {
+ perror ("Error truncating tmpfile");
+ unlink(tmp_filename);
+ return 1;
+ }
+
ret = fsync (fileno (f));
if (ret)
{
@@ -221,15 +290,9 @@ int main(int argc, char *argv[])
return 1;
}
- ret = fclose (f);
- if (ret)
- {
- perror ("Error closing tmpfile");
- unlink(tmp_filename);
- return 1;
- }
-
/*
+ * We must not close the file before rename() as that would remove the lock.
+ *
* And finally rename the tmpfile with the new env over the old env, the
* linux kernel guarantees that this is atomic (from a syscall pov).
*/

36
SOURCES/0334-grub-set-bootflag-Exit-calmly-when-not-running-as-ro.patch Normal file
View File

@ -0,0 +1,36 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Solar Designer <solar@openwall.com>
Date: Tue, 6 Feb 2024 22:05:45 +0100
Subject: [PATCH] grub-set-bootflag: Exit calmly when not running as root
Exit calmly when not installed SUID root and invoked by non-root. This
allows installing user/grub-boot-success.service unconditionally while
supporting non-SUID installation of the program for some limited usage.
Signed-off-by: Solar Designer <solar@openwall.com>
---
util/grub-set-bootflag.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/util/grub-set-bootflag.c b/util/grub-set-bootflag.c
index 514c4f9091ac..31a868aeca8a 100644
--- a/util/grub-set-bootflag.c
+++ b/util/grub-set-bootflag.c
@@ -98,6 +98,17 @@ int main(int argc, char *argv[])
bootflag = bootflags[i];
len = strlen (bootflag);
+ /*
+ * Exit calmly when not installed SUID root and invoked by non-root. This
+ * allows installing user/grub-boot-success.service unconditionally while
+ * supporting non-SUID installation of the program for some limited usage.
+ */
+ if (geteuid())
+ {
+ printf ("grub-set-bootflag not running as root, no action taken\n");
+ return 0;
+ }
+
/*
* setegid avoids the new grubenv's gid being that of the user.
*/

93
SOURCES/0335-fs-ntfs-Fix-an-OOB-write-when-parsing-the-ATTRIBUTE_.patch Normal file
View File

@ -0,0 +1,93 @@
From 43651027d24e62a7a463254165e1e46e42aecdea Mon Sep 17 00:00:00 2001
From: Maxim Suhanov <dfirblog@gmail.com>
Date: Mon, 28 Aug 2023 16:31:57 +0300
Subject: [PATCH 1/6] fs/ntfs: Fix an OOB write when parsing the
$ATTRIBUTE_LIST attribute for the $MFT file
When parsing an extremely fragmented $MFT file, i.e., the file described
using the $ATTRIBUTE_LIST attribute, current NTFS code will reuse a buffer
containing bytes read from the underlying drive to store sector numbers,
which are consumed later to read data from these sectors into another buffer.
These sectors numbers, two 32-bit integers, are always stored at predefined
offsets, 0x10 and 0x14, relative to first byte of the selected entry within
the $ATTRIBUTE_LIST attribute. Usually, this won't cause any problem.
However, when parsing a specially-crafted file system image, this may cause
the NTFS code to write these integers beyond the buffer boundary, likely
causing the GRUB memory allocator to misbehave or fail. These integers contain
values which are controlled by on-disk structures of the NTFS file system.
Such modification and resulting misbehavior may touch a memory range not
assigned to the GRUB and owned by firmware or another EFI application/driver.
This fix introduces checks to ensure that these sector numbers are never
written beyond the boundary.
Fixes: CVE-2023-4692
Reported-by: Maxim Suhanov <dfirblog@gmail.com>
Signed-off-by: Maxim Suhanov <dfirblog@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/fs/ntfs.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index bbdbe24ada83..c3c4db117bba 100644
--- a/grub-core/fs/ntfs.c
+++ b/grub-core/fs/ntfs.c
@@ -184,7 +184,7 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
}
if (at->attr_end)
{
- grub_uint8_t *pa;
+ grub_uint8_t *pa, *pa_end;
at->emft_buf = grub_malloc (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR);
if (at->emft_buf == NULL)
@@ -209,11 +209,13 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
}
at->attr_nxt = at->edat_buf;
at->attr_end = at->edat_buf + u32at (pa, 0x30);
+ pa_end = at->edat_buf + n;
}
else
{
at->attr_nxt = at->attr_end + u16at (pa, 0x14);
at->attr_end = at->attr_end + u32at (pa, 4);
+ pa_end = at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR);
}
at->flags |= GRUB_NTFS_AF_ALST;
while (at->attr_nxt < at->attr_end)
@@ -230,6 +232,13 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
at->flags |= GRUB_NTFS_AF_GPOS;
at->attr_cur = at->attr_nxt;
pa = at->attr_cur;
+
+ if ((pa >= pa_end) || (pa_end - pa < 0x18))
+ {
+ grub_error (GRUB_ERR_BAD_FS, "can\'t parse attribute list");
+ return NULL;
+ }
+
grub_set_unaligned32 ((char *) pa + 0x10,
grub_cpu_to_le32 (at->mft->data->mft_start));
grub_set_unaligned32 ((char *) pa + 0x14,
@@ -240,6 +249,13 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
{
if (*pa != attr)
break;
+
+ if ((pa >= pa_end) || (pa_end - pa < 0x18))
+ {
+ grub_error (GRUB_ERR_BAD_FS, "can\'t parse attribute list");
+ return NULL;
+ }
+
if (read_attr
(at, pa + 0x10,
u32at (pa, 0x10) * (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR),
--
2.43.0

58
SOURCES/0336-fs-ntfs-Fix-an-OOB-read-when-reading-data-from-the-r.patch Normal file
View File

@ -0,0 +1,58 @@
From 0ed2458cc4eff6d9a9199527e2a0b6d445802f94 Mon Sep 17 00:00:00 2001
From: Maxim Suhanov <dfirblog@gmail.com>
Date: Mon, 28 Aug 2023 16:32:33 +0300
Subject: [PATCH 2/6] fs/ntfs: Fix an OOB read when reading data from the
resident $DATA attribute
When reading a file containing resident data, i.e., the file data is stored in
the $DATA attribute within the NTFS file record, not in external clusters,
there are no checks that this resident data actually fits the corresponding
file record segment.
When parsing a specially-crafted file system image, the current NTFS code will
read the file data from an arbitrary, attacker-chosen memory offset and of
arbitrary, attacker-chosen length.
This allows an attacker to display arbitrary chunks of memory, which could
contain sensitive information like password hashes or even plain-text,
obfuscated passwords from BS EFI variables.
This fix implements a check to ensure that resident data is read from the
corresponding file record segment only.
Fixes: CVE-2023-4693
Reported-by: Maxim Suhanov <dfirblog@gmail.com>
Signed-off-by: Maxim Suhanov <dfirblog@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/fs/ntfs.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index c3c4db117bba..a68e173d8285 100644
--- a/grub-core/fs/ntfs.c
+++ b/grub-core/fs/ntfs.c
@@ -401,7 +401,18 @@ read_data (struct grub_ntfs_attr *at, grub_uint8_t *pa, grub_uint8_t *dest,
{
if (ofs + len > u32at (pa, 0x10))
return grub_error (GRUB_ERR_BAD_FS, "read out of range");
- grub_memcpy (dest, pa + u32at (pa, 0x14) + ofs, len);
+
+ if (u32at (pa, 0x10) > (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR))
+ return grub_error (GRUB_ERR_BAD_FS, "resident attribute too large");
+
+ if (pa >= at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR))
+ return grub_error (GRUB_ERR_BAD_FS, "resident attribute out of range");
+
+ if (u16at (pa, 0x14) + u32at (pa, 0x10) >
+ (grub_addr_t) at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR) - (grub_addr_t) pa)
+ return grub_error (GRUB_ERR_BAD_FS, "resident attribute out of range");
+
+ grub_memcpy (dest, pa + u16at (pa, 0x14) + ofs, len);
return 0;
}
--
2.43.0

73
SOURCES/0337-fs-ntfs-Fix-an-OOB-read-when-parsing-directory-entri.patch Normal file
View File

@ -0,0 +1,73 @@
From 7e5f031a6a6a3decc2360a7b0c71abbe598e7354 Mon Sep 17 00:00:00 2001
From: Maxim Suhanov <dfirblog@gmail.com>
Date: Mon, 28 Aug 2023 16:33:17 +0300
Subject: [PATCH 3/6] fs/ntfs: Fix an OOB read when parsing directory entries
from resident and non-resident index attributes
This fix introduces checks to ensure that index entries are never read
beyond the corresponding directory index.
The lack of this check is a minor issue, likely not exploitable in any way.
Reported-by: Maxim Suhanov <dfirblog@gmail.com>
Signed-off-by: Maxim Suhanov <dfirblog@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/fs/ntfs.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index a68e173d8285..2d78b96e19fb 100644
--- a/grub-core/fs/ntfs.c
+++ b/grub-core/fs/ntfs.c
@@ -599,7 +599,7 @@ get_utf8 (grub_uint8_t *in, grub_size_t len)
}
static int
-list_file (struct grub_ntfs_file *diro, grub_uint8_t *pos,
+list_file (struct grub_ntfs_file *diro, grub_uint8_t *pos, grub_uint8_t *end_pos,
grub_fshelp_iterate_dir_hook_t hook, void *hook_data)
{
grub_uint8_t *np;
@@ -610,6 +610,9 @@ list_file (struct grub_ntfs_file *diro, grub_uint8_t *pos,
grub_uint8_t namespace;
char *ustr;
+ if ((pos >= end_pos) || (end_pos - pos < 0x52))
+ break;
+
if (pos[0xC] & 2) /* end signature */
break;
@@ -617,6 +620,9 @@ list_file (struct grub_ntfs_file *diro, grub_uint8_t *pos,
ns = *(np++);
namespace = *(np++);
+ if (2 * ns > end_pos - pos - 0x52)
+ break;
+
/*
* Ignore files in DOS namespace, as they will reappear as Win32
* names.
@@ -806,7 +812,9 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir,
}
cur_pos += 0x10; /* Skip index root */
- ret = list_file (mft, cur_pos + u16at (cur_pos, 0), hook, hook_data);
+ ret = list_file (mft, cur_pos + u16at (cur_pos, 0),
+ at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR),
+ hook, hook_data);
if (ret)
goto done;
@@ -893,6 +901,7 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir,
(const grub_uint8_t *) "INDX")))
goto done;
ret = list_file (mft, &indx[0x18 + u16at (indx, 0x18)],
+ indx + (mft->data->idx_size << GRUB_NTFS_BLK_SHR),
hook, hook_data);
if (ret)
goto done;
--
2.43.0

51
SOURCES/0338-fs-ntfs-Fix-an-OOB-read-when-parsing-bitmaps-for-ind.patch Normal file
View File

@ -0,0 +1,51 @@
From 7a5a116739fa6d8a625da7d6b9272c9a2462f967 Mon Sep 17 00:00:00 2001
From: Maxim Suhanov <dfirblog@gmail.com>
Date: Mon, 28 Aug 2023 16:33:44 +0300
Subject: [PATCH 4/6] fs/ntfs: Fix an OOB read when parsing bitmaps for index
attributes
This fix introduces checks to ensure that bitmaps for directory indices
are never read beyond their actual sizes.
The lack of this check is a minor issue, likely not exploitable in any way.
Reported-by: Maxim Suhanov <dfirblog@gmail.com>
Signed-off-by: Maxim Suhanov <dfirblog@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/fs/ntfs.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index 2d78b96e19fb..bb70c89fb803 100644
--- a/grub-core/fs/ntfs.c
+++ b/grub-core/fs/ntfs.c
@@ -843,6 +843,25 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir,
if (is_resident)
{
+ if (bitmap_len > (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR))
+ {
+ grub_error (GRUB_ERR_BAD_FS, "resident bitmap too large");
+ goto done;
+ }
+
+ if (cur_pos >= at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR))
+ {
+ grub_error (GRUB_ERR_BAD_FS, "resident bitmap out of range");
+ goto done;
+ }
+
+ if (u16at (cur_pos, 0x14) + u32at (cur_pos, 0x10) >
+ (grub_addr_t) at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR) - (grub_addr_t) cur_pos)
+ {
+ grub_error (GRUB_ERR_BAD_FS, "resident bitmap out of range");
+ goto done;
+ }
+
grub_memcpy (bmp, cur_pos + u16at (cur_pos, 0x14),
bitmap_len);
}
--
2.43.0

61
SOURCES/0339-fs-ntfs-Fix-an-OOB-read-when-parsing-a-volume-label.patch Normal file
View File

@ -0,0 +1,61 @@
From 1fe82c41e070385e273d7bb1cfb482627a3c28e8 Mon Sep 17 00:00:00 2001
From: Maxim Suhanov <dfirblog@gmail.com>
Date: Mon, 28 Aug 2023 16:38:19 +0300
Subject: [PATCH 5/6] fs/ntfs: Fix an OOB read when parsing a volume label
This fix introduces checks to ensure that an NTFS volume label is always
read from the corresponding file record segment.
The current NTFS code allows the volume label string to be read from an
arbitrary, attacker-chosen memory location. However, the bytes read are
always treated as UTF-16LE. So, the final string displayed is mostly
unreadable and it can't be easily converted back to raw bytes.
The lack of this check is a minor issue, likely not causing a significant
data leak.
Reported-by: Maxim Suhanov <dfirblog@gmail.com>
Signed-off-by: Maxim Suhanov <dfirblog@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/fs/ntfs.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index bb70c89fb803..ff5e3740f0dd 100644
--- a/grub-core/fs/ntfs.c
+++ b/grub-core/fs/ntfs.c
@@ -1213,13 +1213,29 @@ grub_ntfs_label (grub_device_t device, char **label)
init_attr (&mft->attr, mft);
pa = find_attr (&mft->attr, GRUB_NTFS_AT_VOLUME_NAME);
+
+ if (pa >= mft->buf + (mft->data->mft_size << GRUB_NTFS_BLK_SHR))
+ {
+ grub_error (GRUB_ERR_BAD_FS, "can\'t parse volume label");
+ goto fail;
+ }
+
+ if (mft->buf + (mft->data->mft_size << GRUB_NTFS_BLK_SHR) - pa < 0x16)
+ {
+ grub_error (GRUB_ERR_BAD_FS, "can\'t parse volume label");
+ goto fail;
+ }
+
if ((pa) && (pa[8] == 0) && (u32at (pa, 0x10)))
{
int len;
len = u32at (pa, 0x10) / 2;
pa += u16at (pa, 0x14);
- *label = get_utf8 (pa, len);
+ if (mft->buf + (mft->data->mft_size << GRUB_NTFS_BLK_SHR) - pa >= 2 * len)
+ *label = get_utf8 (pa, len);
+ else
+ grub_error (GRUB_ERR_BAD_FS, "can\'t parse volume label");
}
fail:
--
2.43.0

159
SOURCES/0340-fs-ntfs-Make-code-more-readable.patch Normal file
View File

@ -0,0 +1,159 @@
From e58b870ff926415e23fc386af41ff81b2f588763 Mon Sep 17 00:00:00 2001
From: Maxim Suhanov <dfirblog@gmail.com>
Date: Mon, 28 Aug 2023 16:40:07 +0300
Subject: [PATCH 6/6] fs/ntfs: Make code more readable
Move some calls used to access NTFS attribute header fields into
functions with human-readable names.
Suggested-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Maxim Suhanov <dfirblog@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/fs/ntfs.c | 48 +++++++++++++++++++++++++++++++--------------
1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index ff5e3740f0dd..de435aa14d85 100644
--- a/grub-core/fs/ntfs.c
+++ b/grub-core/fs/ntfs.c
@@ -52,6 +52,24 @@ u64at (void *ptr, grub_size_t ofs)
return grub_le_to_cpu64 (grub_get_unaligned64 ((char *) ptr + ofs));
}
+static grub_uint16_t
+first_attr_off (void *mft_buf_ptr)
+{
+ return u16at (mft_buf_ptr, 0x14);
+}
+
+static grub_uint16_t
+res_attr_data_off (void *res_attr_ptr)
+{
+ return u16at (res_attr_ptr, 0x14);
+}
+
+static grub_uint32_t
+res_attr_data_len (void *res_attr_ptr)
+{
+ return u32at (res_attr_ptr, 0x10);
+}
+
grub_ntfscomp_func_t grub_ntfscomp_func;
static grub_err_t
@@ -106,7 +124,7 @@ init_attr (struct grub_ntfs_attr *at, struct grub_ntfs_file *mft)
{
at->mft = mft;
at->flags = (mft == &mft->data->mmft) ? GRUB_NTFS_AF_MMFT : 0;
- at->attr_nxt = mft->buf + u16at (mft->buf, 0x14);
+ at->attr_nxt = mft->buf + first_attr_off (mft->buf);
at->attr_end = at->emft_buf = at->edat_buf = at->sbuf = NULL;
}
@@ -154,7 +172,7 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
return NULL;
}
- new_pos = &at->emft_buf[u16at (at->emft_buf, 0x14)];
+ new_pos = &at->emft_buf[first_attr_off (at->emft_buf)];
while (*new_pos != 0xFF)
{
if ((*new_pos == *at->attr_cur)
@@ -213,7 +231,7 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
}
else
{
- at->attr_nxt = at->attr_end + u16at (pa, 0x14);
+ at->attr_nxt = at->attr_end + res_attr_data_off (pa);
at->attr_end = at->attr_end + u32at (pa, 4);
pa_end = at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR);
}
@@ -399,20 +417,20 @@ read_data (struct grub_ntfs_attr *at, grub_uint8_t *pa, grub_uint8_t *dest,
if (pa[8] == 0)
{
- if (ofs + len > u32at (pa, 0x10))
+ if (ofs + len > res_attr_data_len (pa))
return grub_error (GRUB_ERR_BAD_FS, "read out of range");
- if (u32at (pa, 0x10) > (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR))
+ if (res_attr_data_len (pa) > (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR))
return grub_error (GRUB_ERR_BAD_FS, "resident attribute too large");
if (pa >= at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR))
return grub_error (GRUB_ERR_BAD_FS, "resident attribute out of range");
- if (u16at (pa, 0x14) + u32at (pa, 0x10) >
+ if (res_attr_data_off (pa) + res_attr_data_len (pa) >
(grub_addr_t) at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR) - (grub_addr_t) pa)
return grub_error (GRUB_ERR_BAD_FS, "resident attribute out of range");
- grub_memcpy (dest, pa + u16at (pa, 0x14) + ofs, len);
+ grub_memcpy (dest, pa + res_attr_data_off (pa) + ofs, len);
return 0;
}
@@ -556,7 +574,7 @@ init_file (struct grub_ntfs_file *mft, grub_uint64_t mftno)
(unsigned long long) mftno);
if (!pa[8])
- mft->size = u32at (pa, 0x10);
+ mft->size = res_attr_data_len (pa);
else
mft->size = u64at (pa, 0x30);
@@ -805,7 +823,7 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir,
(u32at (cur_pos, 0x18) != 0x490024) ||
(u32at (cur_pos, 0x1C) != 0x300033))
continue;
- cur_pos += u16at (cur_pos, 0x14);
+ cur_pos += res_attr_data_off (cur_pos);
if (*cur_pos != 0x30) /* Not filename index */
continue;
break;
@@ -834,7 +852,7 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir,
{
int is_resident = (cur_pos[8] == 0);
- bitmap_len = ((is_resident) ? u32at (cur_pos, 0x10) :
+ bitmap_len = ((is_resident) ? res_attr_data_len (cur_pos) :
u32at (cur_pos, 0x28));
bmp = grub_malloc (bitmap_len);
@@ -855,14 +873,14 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir,
goto done;
}
- if (u16at (cur_pos, 0x14) + u32at (cur_pos, 0x10) >
+ if (res_attr_data_off (cur_pos) + res_attr_data_len (cur_pos) >
(grub_addr_t) at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR) - (grub_addr_t) cur_pos)
{
grub_error (GRUB_ERR_BAD_FS, "resident bitmap out of range");
goto done;
}
- grub_memcpy (bmp, cur_pos + u16at (cur_pos, 0x14),
+ grub_memcpy (bmp, cur_pos + res_attr_data_off (cur_pos),
bitmap_len);
}
else
@@ -1226,12 +1244,12 @@ grub_ntfs_label (grub_device_t device, char **label)
goto fail;
}
- if ((pa) && (pa[8] == 0) && (u32at (pa, 0x10)))
+ if ((pa) && (pa[8] == 0) && (res_attr_data_len (pa)))
{
int len;
- len = u32at (pa, 0x10) / 2;
- pa += u16at (pa, 0x14);
+ len = res_attr_data_len (pa) / 2;
+ pa += res_attr_data_off (pa);
if (mft->buf + (mft->data->mft_size << GRUB_NTFS_BLK_SHR) - pa >= 2 * len)
*label = get_utf8 (pa, len);
else
--
2.43.0

38
SOURCES/0341-grub_dl_set_mem_attrs-fix-format-string.patch Normal file
View File

@ -0,0 +1,38 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Fri, 7 Apr 2023 14:54:35 +0200
Subject: [PATCH] grub_dl_set_mem_attrs(): fix format string
The grub_dprintf() call for printing the message
updating attributes for GOT and trampolines
passes the argument "mod->name", but the format string doesn't accept that
argument.
Print the module name too.
Example output:
> kern/dl.c:736: updating attributes for GOT and trampolines ("video_fb")
Fixes: ad1b904d325b (nx: set page permissions for loaded modules.)
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
grub-core/kern/dl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c
index ab9101a5ad1a..a97f4a8b1355 100644
--- a/grub-core/kern/dl.c
+++ b/grub-core/kern/dl.c
@@ -733,7 +733,8 @@ grub_dl_set_mem_attrs (grub_dl_t mod, void *ehdr)
{
tgsz = ALIGN_UP(tgsz, arch_addralign);
- grub_dprintf ("modules", "updating attributes for GOT and trampolines\n",
+ grub_dprintf ("modules",
+ "updating attributes for GOT and trampolines (\"%s\")\n",
mod->name);
grub_update_mem_attrs (tgaddr, tgsz, GRUB_MEM_ATTR_R|GRUB_MEM_ATTR_X,
GRUB_MEM_ATTR_W);

140
SOURCES/0342-grub_dl_set_mem_attrs-add-self-check-for-the-tramp-G.patch Normal file
View File

@ -0,0 +1,140 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Fri, 7 Apr 2023 16:21:54 +0200
Subject: [PATCH] grub_dl_set_mem_attrs(): add self-check for the tramp/GOT
sizes
On aarch64 UEFI, we currently have a crasher:
grub_dl_load_core()
grub_dl_load_core_noinit()
/* independent allocation: must remain writable */
mod = grub_zalloc();
/* allocates module image with incorrect tail alignment */
grub_dl_load_segments()
/* write-protecting the module image makes "mod" read-only! */
grub_dl_set_mem_attrs()
grub_update_mem_attrs()
grub_dl_init()
/* page fault, crash */
mod->next = ...;
- Commit 887f1d8fa976 ("modules: load module sections at page-aligned
addresses", 2023-02-08) forgot to page-align the allocation of the
trampolines and GOT areas of grub2 modules, in grub_dl_load_segments().
- Commit ad1b904d325b ("nx: set page permissions for loaded modules.",
2023-02-08) calculated a common bounding box for the trampolines and GOT
areas in grub_dl_set_mem_attrs(), rounded the box size up to a whole
multiple of EFI page size ("arch_addralign"), and write-protected the
resultant page range.
Consequently, grub_dl_load_segments() places the module image in memory
such that its tail -- the end of the trampolines and GOT areas -- lands at
the head of a page whose tail in turn contains independent memory
allocations, such as "mod". grub_dl_set_mem_attrs() will then unwittingly
write-protect these other allocations too.
But "mod" must remain writable: we assign "mod->next" in grub_dl_init()
subsequently. Currently we crash there with a page fault / permission
fault.
(The crash is not trivial to hit: the tramp/GOT areas are irrelevant on
x86_64, plus the page protection depends on the UEFI platform firmware
providing EFI_MEMORY_ATTRIBUTE_PROTOCOL. In practice, the crash is
restricted to aarch64 edk2 (ArmVirtQemu) builds containing commit
1c4dfadb4611, "ArmPkg/CpuDxe: Implement EFI memory attributes protocol",
2023-03-16.)
Example log before the patch:
> kern/dl.c:736: updating attributes for GOT and trampolines ("video_fb")
> kern/efi/mm.c:927: set +rx -w on 0x13b88b000-0x13b88bfff before:rwx after:r-x
> kern/dl.c:744: done updating module memory attributes for "video_fb"
> kern/dl.c:639: flushing 0xe4f0 bytes at 0x13b87d000
> kern/arm64/cache.c:42: D$ line size: 64
> kern/arm64/cache.c:43: I$ line size: 64
> kern/dl.c:839: module name: video_fb
> kern/dl.c:840: init function: 0x0
> kern/dl.c:865: Initing module video_fb
>
> Synchronous Exception at 0x000000013B8A76EC
> PC 0x00013B8A76EC
>
> X0 0x000000013B88B960 X1 0x0000000000000000 X2 0x000000013F93587C X3 0x0000000000000075
>
> SP 0x00000000470745C0 ELR 0x000000013B8A76EC SPSR 0x60000205 FPSR 0x00000000
> ESR 0x9600004F FAR 0x000000013B88B9D0
>
> ESR : EC 0x25 IL 0x1 ISS 0x0000004F
>
> Data abort: Permission fault, third level
Note the following:
- The whole 4K page at 0x1_3B88_B000 is write-protected.
- The "video_fb" module actually lives at [0x1_3B87_D000, 0x1_3B88_B4F0)
-- left-inclusive, right-exclusive --; that is, in the last page (at
0x1_3B88_B000), it only occupies the first 0x4F0 bytes.
- The instruction at 0x1_3B8A_76EC faults. Not shown here, but it is a
store instruction, which writes to the field at offset 0x70 of the
structure pointed-to by the X0 register. This is the "mod->next"
assignment from grub_dl_init().
- The faulting address is therefore (X0 + 0x70), i.e., 0x1_3B88_B9D0. This
is indeed the value held in the FAR register.
- The faulting address 0x1_3B88_B9D0 falls in the above-noted page (at
0x1_3B88_B000), namely at offset 0x9D0. This is *beyond* the first 0x4F0
bytes that the very tail of the "video_fb" module occupies at the front
of that page.
For now, add a self-check that reports this bug (and prevents the crash by
skipping the write protection).
Example log after the patch:
> kern/dl.c:742:BUG: trying to protect pages outside of module allocation
> ("video_fb"): module base 0x13b87d000, size 0xe4f0; tramp/GOT base
> 0x13b88b000, size 0x1000
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
grub-core/kern/dl.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c
index a97f4a8b1355..3b66fa410e80 100644
--- a/grub-core/kern/dl.c
+++ b/grub-core/kern/dl.c
@@ -682,7 +682,7 @@ grub_dl_set_mem_attrs (grub_dl_t mod, void *ehdr)
#if !defined (__i386__) && !defined (__x86_64__) && !defined(__riscv)
grub_size_t arch_addralign = grub_arch_dl_min_alignment ();
grub_addr_t tgaddr;
- grub_uint64_t tgsz;
+ grub_size_t tgsz;
#endif
grub_dprintf ("modules", "updating memory attributes for \"%s\"\n",
@@ -736,6 +736,15 @@ grub_dl_set_mem_attrs (grub_dl_t mod, void *ehdr)
grub_dprintf ("modules",
"updating attributes for GOT and trampolines (\"%s\")\n",
mod->name);
+ if (tgaddr < (grub_addr_t)mod->base ||
+ tgsz > (grub_addr_t)-1 - tgaddr ||
+ tgaddr + tgsz > (grub_addr_t)mod->base + mod->sz)
+ return grub_error (GRUB_ERR_BUG,
+ "BUG: trying to protect pages outside of module "
+ "allocation (\"%s\"): module base %p, size 0x%"
+ PRIxGRUB_SIZE "; tramp/GOT base 0x%" PRIxGRUB_ADDR
+ ", size 0x%" PRIxGRUB_SIZE,
+ mod->name, mod->base, mod->sz, tgaddr, tgsz);
grub_update_mem_attrs (tgaddr, tgsz, GRUB_MEM_ATTR_R|GRUB_MEM_ATTR_X,
GRUB_MEM_ATTR_W);
}

70
SOURCES/0343-grub_dl_load_segments-page-align-the-tramp-GOT-areas.patch Normal file
View File

@ -0,0 +1,70 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Fri, 7 Apr 2023 16:56:09 +0200
Subject: [PATCH] grub_dl_load_segments(): page-align the tramp/GOT areas too
The tramp/GOT write-protection in grub_dl_set_mem_attrs() requires that
the tramp/GOT areas of the module image *not* share a page with any other
memory allocations. Page-align the tramp/GOT areas, while satisfying their
intrinsic alignment requirements too.
Fixes: 887f1d8fa976 (modules: load module sections at page-aligned addresses)
Fixes: ad1b904d325b (nx: set page permissions for loaded modules.)
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
grub-core/kern/dl.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c
index 3b66fa410e80..f3cdb9e0bacf 100644
--- a/grub-core/kern/dl.c
+++ b/grub-core/kern/dl.c
@@ -280,7 +280,9 @@ grub_dl_load_segments (grub_dl_t mod, const Elf_Ehdr *e)
grub_size_t tsize = 0, talign = 1, arch_addralign = 1;
#if !defined (__i386__) && !defined (__x86_64__) && !defined(__riscv)
grub_size_t tramp;
+ grub_size_t tramp_align;
grub_size_t got;
+ grub_size_t got_align;
grub_err_t err;
#endif
char *ptr;
@@ -311,12 +313,18 @@ grub_dl_load_segments (grub_dl_t mod, const Elf_Ehdr *e)
err = grub_arch_dl_get_tramp_got_size (e, &tramp, &got);
if (err)
return err;
- tsize += ALIGN_UP (tramp, GRUB_ARCH_DL_TRAMP_ALIGN);
- if (talign < GRUB_ARCH_DL_TRAMP_ALIGN)
- talign = GRUB_ARCH_DL_TRAMP_ALIGN;
- tsize += ALIGN_UP (got, GRUB_ARCH_DL_GOT_ALIGN);
- if (talign < GRUB_ARCH_DL_GOT_ALIGN)
- talign = GRUB_ARCH_DL_GOT_ALIGN;
+ tramp_align = GRUB_ARCH_DL_TRAMP_ALIGN;
+ if (tramp_align < arch_addralign)
+ tramp_align = arch_addralign;
+ tsize += ALIGN_UP (tramp, tramp_align);
+ if (talign < tramp_align)
+ talign = tramp_align;
+ got_align = GRUB_ARCH_DL_GOT_ALIGN;
+ if (got_align < arch_addralign)
+ got_align = arch_addralign;
+ tsize += ALIGN_UP (got, got_align);
+ if (talign < got_align)
+ talign = got_align;
#endif
#ifdef GRUB_MACHINE_EMU
@@ -376,11 +384,11 @@ grub_dl_load_segments (grub_dl_t mod, const Elf_Ehdr *e)
}
}
#if !defined (__i386__) && !defined (__x86_64__) && !defined(__riscv)
- ptr = (char *) ALIGN_UP ((grub_addr_t) ptr, GRUB_ARCH_DL_TRAMP_ALIGN);
+ ptr = (char *) ALIGN_UP ((grub_addr_t) ptr, tramp_align);
mod->tramp = ptr;
mod->trampptr = ptr;
ptr += tramp;
- ptr = (char *) ALIGN_UP ((grub_addr_t) ptr, GRUB_ARCH_DL_GOT_ALIGN);
+ ptr = (char *) ALIGN_UP ((grub_addr_t) ptr, got_align);
mod->got = ptr;
mod->gotptr = ptr;
ptr += got;

3
SOURCES/20-grub.install
View File

@ -19,6 +19,9 @@ MACHINE_ID=$KERNEL_INSTALL_MACHINE_ID
# If ${BOOT_DIR_ABS} exists, some other boot loader is active. # If ${BOOT_DIR_ABS} exists, some other boot loader is active.
[[ -d "${BOOT_DIR_ABS}" ]] && exit 0 [[ -d "${BOOT_DIR_ABS}" ]] && exit 0
# UKIs are BLS type 2 entries, 90-uki-copy.install takes care of them
[ "x$KERNEL_INSTALL_LAYOUT" != "xuki" ] || exit 0
BLS_DIR="/boot/loader/entries" BLS_DIR="/boot/loader/entries"
mkbls() { mkbls() {

15
SOURCES/grub.patches
View File

@ -326,3 +326,18 @@ Patch0325: 0325-kern-ieee1275-init-Extended-support-in-Vec5.patch
Patch0326: 0326-efi-http-change-uint32_t-to-uintn_t.patch Patch0326: 0326-efi-http-change-uint32_t-to-uintn_t.patch
Patch0327: 0327-grub-mkconfig-dont-overwrite-BLS-cmdline-if-BLSCFG.patch Patch0327: 0327-grub-mkconfig-dont-overwrite-BLS-cmdline-if-BLSCFG.patch
Patch0328: 0328-grub2-mkconfig-Pass-all-boot-params-when-used-by-ana.patch Patch0328: 0328-grub2-mkconfig-Pass-all-boot-params-when-used-by-ana.patch
Patch0329: 0329-kern-ieee1275-init-ppc64-Restrict-high-memory-in-pre.patch
Patch0330: 0330-normal-Remove-grub_env_set-prefix-in-grub_try_normal.patch
Patch0331: 0331-search-command-add-flag-to-only-search-root-dev.patch
Patch0332: 0332-grub-set-bootflag-Conservative-partial-fix-for-CVE-2.patch
Patch0333: 0333-grub-set-bootflag-More-complete-fix-for-CVE-2024-104.patch
Patch0334: 0334-grub-set-bootflag-Exit-calmly-when-not-running-as-ro.patch
Patch0335: 0335-fs-ntfs-Fix-an-OOB-write-when-parsing-the-ATTRIBUTE_.patch
Patch0336: 0336-fs-ntfs-Fix-an-OOB-read-when-reading-data-from-the-r.patch
Patch0337: 0337-fs-ntfs-Fix-an-OOB-read-when-parsing-directory-entri.patch
Patch0338: 0338-fs-ntfs-Fix-an-OOB-read-when-parsing-bitmaps-for-ind.patch
Patch0339: 0339-fs-ntfs-Fix-an-OOB-read-when-parsing-a-volume-label.patch
Patch0340: 0340-fs-ntfs-Make-code-more-readable.patch
Patch0341: 0341-grub_dl_set_mem_attrs-fix-format-string.patch
Patch0342: 0342-grub_dl_set_mem_attrs-add-self-check-for-the-tramp-G.patch
Patch0343: 0343-grub_dl_load_segments-page-align-the-tramp-GOT-areas.patch

36
SPECS/grub2.spec
View File

@ -16,7 +16,7 @@
Name: grub2 Name: grub2
Epoch: 1 Epoch: 1
Version: 2.06 Version: 2.06
Release: 70%{?dist}.1 Release: 77%{?dist}
Summary: Bootloader with support for Linux, Multiboot and more Summary: Bootloader with support for Linux, Multiboot and more
License: GPLv3+ License: GPLv3+
URL: http://www.gnu.org/software/grub/ URL: http://www.gnu.org/software/grub/
@ -349,7 +349,7 @@ BOOT_UUID=$(%{name}-probe --target=fs_uuid ${GRUB_HOME})
GRUB_DIR=$(%{name}-mkrelpath ${GRUB_HOME}) GRUB_DIR=$(%{name}-mkrelpath ${GRUB_HOME})
cat << EOF > ${EFI_HOME}/grub.cfg.stb cat << EOF > ${EFI_HOME}/grub.cfg.stb
search --no-floppy --fs-uuid --set=dev ${BOOT_UUID} search --no-floppy --root-dev-only --fs-uuid --set=dev ${BOOT_UUID}
set prefix=(\$dev)${GRUB_DIR} set prefix=(\$dev)${GRUB_DIR}
export \$prefix export \$prefix
configfile \$prefix/grub.cfg configfile \$prefix/grub.cfg
@ -533,6 +533,38 @@ mv ${EFI_HOME}/grub.cfg.stb ${EFI_HOME}/grub.cfg
%endif %endif
%changelog %changelog
* Thu Feb 22 2024 Nicolas Frayer <nfrayer@redhat.com> - 2.06-77
- kern/dl: grub_dl_set_mem_attrs()/grub_dl_load_segments() fixes
- Resolves: #RHEL-26322
* Tue Feb 20 2024 Nicolas Frayer <nfrayer@redhat.com> - 2.06-76
- fs/ntfs: OOB write fix
- (CVE-2023-4692)
- Resolves: #RHEL-11567
* Wed Feb 7 2024 Nicolas Frayer <nfrayer@redhat.com> - 2.06-75
- grub-set-bootflag: Fix for CVE-2024-1048
- (CVE-2024-1048)
- Resolves: #RHEL-20747
* Mon Feb 5 2024 Nicolas Frayer <nfrayer@redhat.com> - 2.06-74
- Don't run 20-grub.install for UKIs
- Resolves: #RHEL-21368
* Thu Jan 4 2024 Nicolas Frayer <nfrayer@redhat.com> - 2.06-73
- search command: add flag to only search root dev
- Resolves: #RHEL-20526
- Resolves: #CVE-2023-4001
* Thu Jan 4 2024 Nicolas Frayer <nfrayer@redhat.com> - 2.06-72
- normal: Remove grub_env_set prefix in grub_try_normal_prefix
- Resolves: #RHEL-1601
* Thu Oct 19 2023 Nicolas Frayer <nfrayer@redhat.com> - 2.06-71
- kern/ieee1275/init: ppc64: Restrict high memory in presence
of fadump
- Resolves: #RHEL-14282
* Tue Aug 29 2023 Nicolas Frayer <nfrayer@redhat.com> - 2.06-70 * Tue Aug 29 2023 Nicolas Frayer <nfrayer@redhat.com> - 2.06-70
- grub2-mkconfig: Pass all boot params when used by anaconda - grub2-mkconfig: Pass all boot params when used by anaconda
- Resolves: #RHEL-2185 - Resolves: #RHEL-2185