[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <70dd035f58199cb933526303792ef238080a235c.1666705333.git.baskov@ispras.ru>
Date: Tue, 25 Oct 2022 17:12:48 +0300
From: Evgeniy Baskov <baskov@...ras.ru>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: Evgeniy Baskov <baskov@...ras.ru>, Borislav Petkov <bp@...en8.de>,
Andy Lutomirski <luto@...nel.org>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Ingo Molnar <mingo@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
Alexey Khoroshilov <khoroshilov@...ras.ru>,
Peter Jones <pjones@...hat.com>, lvc-project@...uxtesting.org,
x86@...nel.org, linux-efi@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: [PATCH v2 10/23] efi/libstub: Move helper function to related file
efi_adjust_memory_range_protection() can be useful outside x86-stub.c.
Move it to mem.c, where memory related code resides and make it
non-static.
Change its behavior to setup exact attributes and disallow making
memory regions readable and writable simultaneously for supported
configurations.
Signed-off-by: Evgeniy Baskov <baskov@...ras.ru>
---
drivers/firmware/efi/libstub/efistub.h | 4 +
drivers/firmware/efi/libstub/mem.c | 102 ++++++++++++++++++++++++
drivers/firmware/efi/libstub/x86-stub.c | 66 ++-------------
3 files changed, 112 insertions(+), 60 deletions(-)
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index a30fb5d8ef05..63f5157341fe 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -917,6 +917,10 @@ efi_status_t efi_relocate_kernel(unsigned long *image_addr,
unsigned long alignment,
unsigned long min_addr);
+efi_status_t efi_adjust_memory_range_protection(unsigned long start,
+ unsigned long size,
+ unsigned long attributes);
+
efi_status_t efi_parse_options(char const *cmdline);
void efi_parse_option_graphics(char *option);
diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c
index 45841ef55a9f..cdf1e6fb6430 100644
--- a/drivers/firmware/efi/libstub/mem.c
+++ b/drivers/firmware/efi/libstub/mem.c
@@ -125,3 +125,105 @@ void efi_free(unsigned long size, unsigned long addr)
nr_pages = round_up(size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
efi_bs_call(free_pages, addr, nr_pages);
}
+
+/**
+ * efi_adjust_memory_range_protection() - change memory range protection attributes
+ * @start: memory range start address
+ * @size: memory range size
+ *
+ * Actual memory range for which memory attributes are modified is
+ * the smallest ranged with start address and size aligned to EFI_PAGE_SIZE
+ * that includes [start, start + size].
+ *
+ * @return: status code
+ */
+efi_status_t efi_adjust_memory_range_protection(unsigned long start,
+ unsigned long size,
+ unsigned long attributes)
+{
+ efi_status_t status;
+ efi_gcd_memory_space_desc_t desc;
+ efi_physical_addr_t end, next;
+ efi_physical_addr_t rounded_start, rounded_end;
+ efi_physical_addr_t unprotect_start, unprotect_size;
+
+ if (efi_dxe_table == NULL)
+ return EFI_UNSUPPORTED;
+
+ /*
+ * This function should not be used to modify attributes
+ * other than writable/executable.
+ */
+
+ if ((attributes & ~(EFI_MEMORY_RO | EFI_MEMORY_XP)) != 0)
+ return EFI_INVALID_PARAMETER;
+
+ /*
+ * Disallow simultaniously executable and writable memory
+ * to inforce W^X policy if direct extraction code is enabled.
+ */
+
+ if ((attributes & (EFI_MEMORY_RO | EFI_MEMORY_XP)) == 0) {
+ efi_warn("W^X violation at [%08lx,%08lx]\n",
+ (unsigned long)rounded_start,
+ (unsigned long)rounded_end);
+ }
+
+ rounded_start = rounddown(start, EFI_PAGE_SIZE);
+ rounded_end = roundup(start + size, EFI_PAGE_SIZE);
+
+ /*
+ * Don't modify memory region attributes, they are
+ * already suitable, to lower the possibility to
+ * encounter firmware bugs.
+ */
+
+ for (end = start + size; start < end; start = next) {
+
+ status = efi_dxe_call(get_memory_space_descriptor,
+ start, &desc);
+
+ if (status != EFI_SUCCESS) {
+ efi_warn("Unable to get memory descriptor at %lx\n",
+ start);
+ return status;
+ }
+
+ next = desc.base_address + desc.length;
+
+ /*
+ * Only system memory is suitable for trampoline/kernel image
+ * placement, so only this type of memory needs its attributes
+ * to be modified.
+ */
+
+ if (desc.gcd_memory_type != EfiGcdMemoryTypeSystemMemory) {
+ efi_warn("Attempted to change protection of special memory range\n");
+ return EFI_UNSUPPORTED;
+ }
+
+ if (((desc.attributes ^ attributes) &
+ (EFI_MEMORY_RO | EFI_MEMORY_XP)) == 0)
+ continue;
+
+ desc.attributes &= ~(EFI_MEMORY_RO | EFI_MEMORY_XP);
+ desc.attributes |= attributes;
+
+ unprotect_start = max(rounded_start, desc.base_address);
+ unprotect_size = min(rounded_end, next) - unprotect_start;
+
+ status = efi_dxe_call(set_memory_space_attributes,
+ unprotect_start, unprotect_size,
+ desc.attributes);
+
+ if (status != EFI_SUCCESS) {
+ efi_warn("Unable to unprotect memory range [%08lx,%08lx]: %lx\n",
+ (unsigned long)unprotect_start,
+ (unsigned long)(unprotect_start + unprotect_size),
+ status);
+ return status;
+ }
+ }
+
+ return EFI_SUCCESS;
+}
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 33a7811e12c6..2fddb88613cd 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -212,61 +212,6 @@ static void retrieve_apple_device_properties(struct boot_params *boot_params)
}
}
-static void
-adjust_memory_range_protection(unsigned long start, unsigned long size)
-{
- efi_status_t status;
- efi_gcd_memory_space_desc_t desc;
- unsigned long end, next;
- unsigned long rounded_start, rounded_end;
- unsigned long unprotect_start, unprotect_size;
-
- if (efi_dxe_table == NULL)
- return;
-
- rounded_start = rounddown(start, EFI_PAGE_SIZE);
- rounded_end = roundup(start + size, EFI_PAGE_SIZE);
-
- /*
- * Don't modify memory region attributes, they are
- * already suitable, to lower the possibility to
- * encounter firmware bugs.
- */
-
- for (end = start + size; start < end; start = next) {
-
- status = efi_dxe_call(get_memory_space_descriptor, start, &desc);
-
- if (status != EFI_SUCCESS)
- return;
-
- next = desc.base_address + desc.length;
-
- /*
- * Only system memory is suitable for trampoline/kernel image placement,
- * so only this type of memory needs its attributes to be modified.
- */
-
- if (desc.gcd_memory_type != EfiGcdMemoryTypeSystemMemory ||
- (desc.attributes & (EFI_MEMORY_RO | EFI_MEMORY_XP)) == 0)
- continue;
-
- unprotect_start = max(rounded_start, (unsigned long)desc.base_address);
- unprotect_size = min(rounded_end, next) - unprotect_start;
-
- status = efi_dxe_call(set_memory_space_attributes,
- unprotect_start, unprotect_size,
- EFI_MEMORY_WB);
-
- if (status != EFI_SUCCESS) {
- efi_warn("Unable to unprotect memory range [%08lx,%08lx]: %lx\n",
- unprotect_start,
- unprotect_start + unprotect_size,
- status);
- }
- }
-}
-
/*
* Trampoline takes 2 pages and can be loaded in first megabyte of memory
* with its end placed between 128k and 640k where BIOS might start.
@@ -290,12 +235,12 @@ setup_memory_protection(unsigned long image_base, unsigned long image_size)
* and relocated kernel image.
*/
- adjust_memory_range_protection(TRAMPOLINE_PLACEMENT_BASE,
- TRAMPOLINE_PLACEMENT_SIZE);
+ efi_adjust_memory_range_protection(TRAMPOLINE_PLACEMENT_BASE,
+ TRAMPOLINE_PLACEMENT_SIZE, 0);
#ifdef CONFIG_64BIT
if (image_base != (unsigned long)startup_32)
- adjust_memory_range_protection(image_base, image_size);
+ efi_adjust_memory_range_protection(image_base, image_size, 0);
#else
/*
* Clear protection flags on a whole range of possible
@@ -305,8 +250,9 @@ setup_memory_protection(unsigned long image_base, unsigned long image_size)
* need to remove possible protection on relocated image
* itself disregarding further relocations.
*/
- adjust_memory_range_protection(LOAD_PHYSICAL_ADDR,
- KERNEL_IMAGE_SIZE - LOAD_PHYSICAL_ADDR);
+ efi_adjust_memory_range_protection(LOAD_PHYSICAL_ADDR,
+ KERNEL_IMAGE_SIZE - LOAD_PHYSICAL_ADDR,
+ 0);
#endif
}
--
2.37.4
Powered by blists - more mailing lists