diff mbox series

[v2,6/6] arm64: Enable RW, RX and RO mappings for the relocated binary

Message ID 20250220135506.151894-7-ilias.apalodimas@linaro.org
State New
Headers show
Series Fix page permission on arm64 architectures | expand

Commit Message

Ilias Apalodimas Feb. 20, 2025, 1:54 p.m. UTC
Now that we have everything in place switch the page permissions for
.rodata, .text and .data just after we relocate everything in top of the
RAM.

Unfortunately we can't enable this by default, since we have examples of
U-Boot crashing due to invalid access. This usually happens because code
defines const variables that it later writes. So hide it behind a Kconfig
option until we sort it out.

It's worth noting that EFI runtime services are not covered by this
patch on purpose. Since the OS can call SetVirtualAddressMap which can
relocate runtime services, we need to set them to RX initially but remap
them as RWX right before ExitBootServices.

Link: https://lore.kernel.org/u-boot/20250129-rockchip-pinctrl-const-v1-0-450ccdadfa7e@cherry.de/
Link: https://lore.kernel.org/u-boot/20250130133646.2177194-1-andre.przywara@arm.com/
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 common/Kconfig   | 13 +++++++++++++
 common/board_r.c | 20 ++++++++++++++++++++
 2 files changed, 33 insertions(+)

Comments

Richard Henderson Feb. 20, 2025, 6:33 p.m. UTC | #1
On 2/20/25 05:54, Ilias Apalodimas wrote:
> Now that we have everything in place switch the page permissions for
> .rodata, .text and .data just after we relocate everything in top of the
> RAM.
> 
> Unfortunately we can't enable this by default, since we have examples of
> U-Boot crashing due to invalid access. This usually happens because code
> defines const variables that it later writes. So hide it behind a Kconfig
> option until we sort it out.
> 
> It's worth noting that EFI runtime services are not covered by this
> patch on purpose. Since the OS can call SetVirtualAddressMap which can
> relocate runtime services, we need to set them to RX initially but remap
> them as RWX right before ExitBootServices.
> 
> Link:https://lore.kernel.org/u-boot/20250129-rockchip-pinctrl-const-v1-0-450ccdadfa7e@cherry.de/
> Link:https://lore.kernel.org/u-boot/20250130133646.2177194-1-andre.przywara@arm.com/
> Reviewed-by: Jerome Forissier<jerome.forissier@linaro.org>
> Signed-off-by: Ilias Apalodimas<ilias.apalodimas@linaro.org>
> ---
>   common/Kconfig   | 13 +++++++++++++
>   common/board_r.c | 20 ++++++++++++++++++++
>   2 files changed, 33 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
diff mbox series

Patch

diff --git a/common/Kconfig b/common/Kconfig
index 7685914fa6fd..d23e845ee471 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -914,6 +914,19 @@  config STACKPROTECTOR
 	  Enable stack smash detection through compiler's stack-protector
 	  canary logic
 
+config MMU_PGPROT
+	bool "Enable RO, RW and RX mappings"
+	help
+	  U-Boot maps all pages as RWX. If selected pages will
+	  be marked as RO(.rodata), RX(.text), RW(.data) right after
+	  we relocate. Since code sections needs to be page aligned
+	  the final binary size will increase. The mappings can be dumped
+	  using the 'meminfo' command.
+
+	  Enabling this feature can expose bugs in U-Boot where we have
+	  code that violates read-only permissions for example. Use this
+	  feature with caution.
+
 config SPL_STACKPROTECTOR
 	bool "Stack Protector buffer overflow detection for SPL"
 	depends on STACKPROTECTOR && SPL
diff --git a/common/board_r.c b/common/board_r.c
index 179259b00de8..65111e2fc97a 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -170,7 +170,27 @@  static int initr_reloc_global_data(void)
 	efi_save_gd();
 
 	efi_runtime_relocate(gd->relocaddr, NULL);
+
 #endif
+	/*
+	 * We are done with all relocations change the permissions of the binary
+	 * NOTE: __start_rodata etc are defined in arm64 linker scripts and
+	 * sections.h. If you want to add support for your platform you need to
+	 * add the symbols on your linker script, otherwise they will point to
+	 * random addresses.
+	 *
+	 */
+	if (IS_ENABLED(CONFIG_MMU_PGPROT)) {
+		pgprot_set_attrs((phys_addr_t)(uintptr_t)(__start_rodata),
+				 (size_t)(uintptr_t)(__end_rodata - __start_rodata),
+				 MMU_ATTR_RO);
+		pgprot_set_attrs((phys_addr_t)(uintptr_t)(__start_data),
+				 (size_t)(uintptr_t)(__end_data - __start_data),
+				 MMU_ATTR_RW);
+		pgprot_set_attrs((phys_addr_t)(uintptr_t)(__text_start),
+				 (size_t)(uintptr_t)(__text_end - __text_start),
+				 MMU_ATTR_RX);
+	}
 
 	return 0;
 }