mbox series

[RFC,0/4] Fix page permission on arm64 architectures

Message ID 20250130072100.27297-1-ilias.apalodimas@linaro.org
Headers show
Series Fix page permission on arm64 architectures | expand

Message

Ilias Apalodimas Jan. 30, 2025, 7:20 a.m. UTC 
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(-)

--
2.43.0