[RFC PATCH 0/4] Fix page permission on arm64 architectures
Ilias Apalodimas
ilias.apalodimas at linaro.org
Tue Feb 4 13:12:14 CET 2025
Hi Love,
On Tue, 4 Feb 2025 at 13:53, Love Kumar <love.kumar at amd.com> wrote:
>
> Hi Ilias,
>
>
> On 30/01/25 12:50 pm, Ilias Apalodimas wrote:
> > U-Boot maps all pages as RWX. Sadly it's not not 1990 anymore and we are
> > better off mapping binaries with proper permissions.
> >
> > This is an attempt to map the U-Boot binary properly, but leave the
> > area we load binaries unaffected and RWX.
> >
> > patch #1 adds printing capabilities for page permissions in meminfo
> > patch #2 prepares our linker scripts, aligns sections in page boundaries etc
> > patch #3 is adding a function to change page tables
> > patch #4 sets the permissions for .rodata, .text and .data
> >
> > There's a few problems problem in this RFC ...
> > - U-Boot has no MMU APIs in place. As a result I just wired up the function
> > changing the page permissions in board_r.c but only for arm64. In the long
> > term we should add an API that other archs can use if they wish/can
> > - meminfo doesn't align all results properly (Caleb?)
> > - I am not setting the permissions of EFI runtime services section on purpose.
> > Since the OS is allowed to call SetVirtualAddressMap, we need to reset the
> > pages to RWX just before ExitBootServices, so the OS can relocate that code.
> > In arm64 this isn't a huge problem, because we explicitly disable SVAM if
> > if VA_BITS > 39, but other OSes/archs will have a problem.
> >
> > # TODO:
> > - Long term. Add an MMU API and use that instead of exporting arch specific
> > functions
> > - Add an enum in mmu_set_attrs() instead of 1,2,3 arguments
> > - Set the permissions of EFI runtime services anbd reset them before EBS()
> > - Fix alignment when printing pages & information
> > - Add a Kconfig option so the feature can be turned off.
> > The reason is that QEMU CI and some boards, fail because they are trying to
> > write RO pages. This usually happens due to variables being wrongly defined as
> > const, but with the updated page permissions this leads to a crash [0] [1].
> > In both cases the reported ESR is 0x9600004f which translates to
> > "Abort caused by writing to memory" "Permission fault, level 3.". On top of
> > that not setting pages as RO (and only setting .data and .text sections) works
> > fine. So until we find and fix the bugs above we can't turn this on
> > unconditionally.
> >
> > # Open questions
> > - Is initr_reloc_global_data() the right place to change permissions? Or is there
> > a better/safer place to do that?
> >
> > # How to test patches
> > - Aplly them an enable
> > CONFIG_CMD_MEMINFO=y
> > CONFIG_CMD_MEMINFO_MAP=y
> > 'meminfo' should print something along the lines of
> > => meminfo
> > DRAM: 8 GiB
> > Walking pagetable at 000000023ffe0000, va_bits: 40. Using 4 levels
> > [0x23ffe1000] | Table | |
> > [0x23ffe2000] | Table | |
> > [0x000000000 - 0x008000000] | Block | RWX | Normal | Inner-shareable
> > [0x008000000 - 0x040000000] | Block | RW | Device-nGnRnE | Non-shareable
> > [0x040000000 - 0x200000000] | Block | RWX | Normal | Inner-shareable
> > [0x23ffea000] | Table | |
> > [0x200000000 - 0x23f600000] | Block | RWX | Normal | Inner-shareable
> > [0x23ffeb000] | Table | |
> > [0x23f600000 - 0x23f68b000] | Pages | RWX | Normal | Inner-shareable
> > [0x23f68b000 - 0x23f74e000] | Pages | RX | Normal | Inner-shareable
> > [0x23f74e000 - 0x23f793000] | Pages | RO | Normal | Inner-shareable
> > [0x23f793000 - 0x23f794000] | Pages | RWX | Normal | Inner-shareable
> > [0x23f794000 - 0x23f79d000] | Pages | RW | Normal | Inner-shareable
> > [0x23f79d000 - 0x23f800000] | Pages | RWX | Normal | Inner-shareable
> > [0x23f800000 - 0x240000000] | Block | RWX | Normal | Inner-shareable
> > [0x240000000 - 0x4000000000] | Block | RWX | Normal | Inner-shareable
> > [0x23ffe3000] | Table | |
> > [0x4010000000 - 0x4020000000] | Block | RW | Device-nGnRnE | Non-shareable
> > [0x23ffe4000] | Table | |
> > [0x8000000000 - 0x10000000000] | Block | RW | Device-nGnRnE | Non-shareable
> >
> > [0] https://lore.kernel.org/u-boot/20250129-rockchip-pinctrl-const-v1-0-450ccdadfa7e@cherry.de/
> > [1] https://source.denx.de/u-boot/custodians/u-boot-tpm/-/jobs/1008714
> >
> > Ilias Apalodimas (4):
> > meminfo: add memory details for armv8
> > arm: Prepare linker scripts for memory permissions
> > arm64: mmu_change_region_attr() add an option not to break PTEs
> > arm64: Change mapping for data/rodata/text
> >
> > Makefile | 15 +++++----
> > arch/arm/cpu/armv8/cache_v8.c | 45 +++++++++++++++++++++++--
> > arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 10 +++---
> > arch/arm/cpu/armv8/u-boot.lds | 32 +++++++++++-------
> > arch/arm/include/asm/armv8/mmu.h | 2 ++
> > arch/arm/include/asm/system.h | 3 +-
> > arch/arm/mach-snapdragon/board.c | 2 +-
> > cmd/meminfo.c | 5 +++
> > common/board_r.c | 7 ++++
> > include/asm-generic/sections.h | 2 ++
> > lib/efi_loader/efi_runtime.c | 2 ++
> > 11 files changed, 97 insertions(+), 28 deletions(-)
>
> This patch series is increasing u-boot size by 13kB which is breaking
> our mini u-boot configurations.
>
There's not much I can do about it, since sections need to be
page-aligned to be able to set permissions.
On v1 I'll have the option under a Kconfig, so you need to keep it disabled.
/Ilias
> Without patch:
> text data bss dec hex filename
> 0 115632 0 115632 1c3b0 u-boot.elf
>
> With patch:
> text data bss dec hex filename
> 0 128688 0 128688 1f6b0 u-boot.elf
>
> Regards,
> Love Kumar
>
> >
> > --
> > 2.43.0
> >
More information about the U-Boot
mailing list