On Wed, Feb 05, 2025 at 12:31:33PM +0200, Ilias Apalodimas wrote: > Hi Jerome, > > On Wed, 5 Feb 2025 at 11:57, Jerome Forissier > <[email protected]> wrote: > > > > > > > > On 2/5/25 08:16, 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/[email protected]/ > > > Link: > > > https://lore.kernel.org/u-boot/[email protected]/ > > > Signed-off-by: Ilias Apalodimas <[email protected]> > > > --- > > > common/Kconfig | 13 +++++++++++++ > > > common/board_r.c | 20 ++++++++++++++++++++ > > > 2 files changed, 33 insertions(+) > > > > > > diff --git a/common/Kconfig b/common/Kconfig > > > index 7685914fa6fd..dbae7e062b0a 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 > > > > Space before ( > > Sure > > > > > > + we relocate. Since code sections needs to be page aligned > > > + the final binary size will increase. > > > + The mapping can be dumped using the 'meminfo' command. > > > > OK > > > > > + We should make this default 'y' in the future, but currently > > > + we have code defining const variables that are later written. > > > + Enabling this will crash U-Boot if that code path runs, so keep > > > + it off by default until we fix invalid accesses. > > > > Not sure this needs to be documented in the Kconfig help. Perhaps just > > keep a patch ready and send it early in the next release cycle for people > > to test and debug? > > I'd like people to see it and try to debug when they see random > crashes. I think it's easier if we document that here until things are > fixed. > OTOH i don't really mind whatever people think it's best
We should change the tone / emphasis I think. Something like: 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. And then we should encourage our more active SoC maintainers to default this option to being enabled after testing out a few platforms as long term it should be on by default but initial testing has shown that assuming most platforms are OK isn't a good idea. -- Tom
signature.asc
Description: PGP signature
