Re: [PATCH v2 6/6] ARM: MMU64: map text segment ro and data segments execute never

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 6/17/25 16:28, Sascha Hauer wrote:
> With this all segments in the DRAM except the text segment are mapped
> execute-never so that only the barebox code can actually be executed.
> Also map the readonly data segment readonly so that it can't be
> modified.
> 
> The mapping is only implemented in barebox proper. The PBL still maps
> the whole DRAM rwx.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Let's see what breaks!

Cheers,
Ahmad

> ---
>  arch/arm/cpu/mmu_64.c            | 34 ++++++++++++++++++++++++++++++----
>  arch/arm/include/asm/pgtable64.h |  1 +
>  arch/arm/lib64/barebox.lds.S     |  5 +++--
>  3 files changed, 34 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
> index 7e46201bbaae06dd4f2f3bd194db93d83401bfc9..4c23cb3056d24799e2c0aec9ee567165fca06ce0 100644
> --- a/arch/arm/cpu/mmu_64.c
> +++ b/arch/arm/cpu/mmu_64.c
> @@ -292,13 +292,19 @@ static unsigned long get_pte_attrs(unsigned flags)
>  {
>  	switch (flags) {
>  	case MAP_CACHED:
> -		return CACHED_MEM;
> +		return attrs_xn() | CACHED_MEM;
>  	case MAP_UNCACHED:
>  		return attrs_xn() | UNCACHED_MEM;
>  	case MAP_FAULT:
>  		return 0x0;
>  	case ARCH_MAP_WRITECOMBINE:
>  		return attrs_xn() | MEM_ALLOC_WRITECOMBINE;
> +	case MAP_CODE:
> +		return CACHED_MEM | PTE_BLOCK_RO;
> +	case ARCH_MAP_CACHED_RO:
> +		return attrs_xn() | CACHED_MEM | PTE_BLOCK_RO;
> +	case ARCH_MAP_CACHED_RWX:
> +		return CACHED_MEM;
>  	default:
>  		return ~0UL;
>  	}
> @@ -316,7 +322,11 @@ static void early_remap_range(uint64_t addr, size_t size, unsigned flags, bool f
>  
>  int arch_remap_range(void *virt_addr, phys_addr_t phys_addr, size_t size, unsigned flags)
>  {
> -	unsigned long attrs = get_pte_attrs(flags);
> +	unsigned long attrs;
> +
> +	flags = arm_mmu_maybe_skip_permissions(flags);
> +
> +	attrs = get_pte_attrs(flags);
>  
>  	if (attrs == ~0UL)
>  		return -EINVAL;
> @@ -357,6 +367,12 @@ void __mmu_init(bool mmu_on)
>  {
>  	uint64_t *ttb = get_ttb();
>  	struct memory_bank *bank;
> +	unsigned long text_start = (unsigned long)&_stext;
> +	unsigned long code_start = text_start;
> +	unsigned long code_size = (unsigned long)&__start_rodata - (unsigned long)&_stext;
> +	unsigned long text_size = (unsigned long)&_etext - text_start;
> +	unsigned long rodata_start = (unsigned long)&__start_rodata;
> +	unsigned long rodata_size = (unsigned long)&__end_rodata - rodata_start;
>  
>  	// TODO: remap writable only while remapping?
>  	// TODO: What memtype for ttb when barebox is EFI loader?
> @@ -383,9 +399,19 @@ void __mmu_init(bool mmu_on)
>  			pos = rsv->end + 1;
>  		}
>  
> +		if (region_overlap_size(pos, bank->start + bank->size - pos,
> +		    text_start, text_size)) {
> +			remap_range((void *)pos, text_start - pos, MAP_CACHED);
> +			/* skip barebox segments here, will be mapped below */
> +			pos = text_start + text_size;
> +		}
> +
>  		remap_range((void *)pos, bank->start + bank->size - pos, MAP_CACHED);
>  	}
>  
> +	remap_range((void *)code_start, code_size, MAP_CODE);
> +	remap_range((void *)rodata_start, rodata_size, ARCH_MAP_CACHED_RO);
> +
>  	/* Make zero page faulting to catch NULL pointer derefs */
>  	zero_page_faulting();
>  	create_guard_page();
> @@ -465,7 +491,7 @@ void mmu_early_enable(unsigned long membase, unsigned long memsize, unsigned lon
>  	 */
>  	init_range(2);
>  
> -	early_remap_range(membase, memsize, MAP_CACHED, false);
> +	early_remap_range(membase, memsize, ARCH_MAP_CACHED_RWX, false);
>  
>  	if (optee_get_membase(&optee_membase)) {
>                  optee_membase = membase + memsize - OPTEE_SIZE;
> @@ -484,7 +510,7 @@ void mmu_early_enable(unsigned long membase, unsigned long memsize, unsigned lon
>  	early_remap_range(optee_membase, OPTEE_SIZE, MAP_FAULT, false);
>  
>  	early_remap_range(PAGE_ALIGN_DOWN((uintptr_t)_stext), PAGE_ALIGN(_etext - _stext),
> -			  MAP_CACHED, false);
> +			  ARCH_MAP_CACHED_RWX, false);
>  
>  	mmu_enable();
>  }
> diff --git a/arch/arm/include/asm/pgtable64.h b/arch/arm/include/asm/pgtable64.h
> index b88ffe6be5254e1b9d3968573d5e9b7a37828a55..6f6ef22717b76baaf7857b12d38c6074871ce143 100644
> --- a/arch/arm/include/asm/pgtable64.h
> +++ b/arch/arm/include/asm/pgtable64.h
> @@ -59,6 +59,7 @@
>  #define PTE_BLOCK_NG            (1 << 11)
>  #define PTE_BLOCK_PXN           (UL(1) << 53)
>  #define PTE_BLOCK_UXN           (UL(1) << 54)
> +#define PTE_BLOCK_RO            (UL(1) << 7)
>  
>  /*
>   * AttrIndx[2:0] encoding (mapping attributes defined in the MAIR* registers).
> diff --git a/arch/arm/lib64/barebox.lds.S b/arch/arm/lib64/barebox.lds.S
> index 50e4b6f42cb8d4de92b7450e5b864b9056b61916..caddbedd610f68658b7ecf7616947ce02a84e5e8 100644
> --- a/arch/arm/lib64/barebox.lds.S
> +++ b/arch/arm/lib64/barebox.lds.S
> @@ -28,18 +28,19 @@ SECTIONS
>  	}
>  	BAREBOX_BARE_INIT_SIZE
>  
> -	. = ALIGN(4);
> +	. = ALIGN(4096);
>  	__start_rodata = .;
>  	.rodata : {
>  		*(.rodata*)
>  		RO_DATA_SECTION
>  	}
>  
> +	. = ALIGN(4096);
> +
>  	__end_rodata = .;
>  	_etext = .;
>  	_sdata = .;
>  
> -	. = ALIGN(4);
>  	.data : { *(.data*) }
>  
>  	.barebox_imd : { BAREBOX_IMD }
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |