parent
5999506124
commit
f39a17434a
@ -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))
|
||||||
|
{
|
@ -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;
|
||||||
|
}
|
@ -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,
|
@ -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);
|
||||||
|
|
||||||
|
|
@ -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).
|
||||||
|
*/
|
@ -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.
|
||||||
|
*/
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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);
|
@ -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);
|
||||||
|
}
|
@ -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;
|
Loading…
Reference in new issue