Re: [RFC/RFT PATCH 2/5] memblock: introduce generic memblock_setup_resources()

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Wed, Jun 02, 2021 at 09:43:32PM +0300, Mike Rapoport wrote:
> Back then when __ex_table was moved from .data section, _sdata and _edata
> were part of the .data section. Today they are not. So something like the
> patch below will ensure for instance that __ex_table would be a part of
> "Kernel data" in /proc/iomem without moving it to the .data section:
> 
> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> index f7f4620d59c3..2991feceab31 100644
> --- a/arch/arm/kernel/vmlinux.lds.S
> +++ b/arch/arm/kernel/vmlinux.lds.S
> @@ -72,13 +72,6 @@ SECTIONS
>  
>  	RO_DATA(PAGE_SIZE)
>  
> -	. = ALIGN(4);
> -	__ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) {
> -		__start___ex_table = .;
> -		ARM_MMU_KEEP(*(__ex_table))
> -		__stop___ex_table = .;
> -	}
> -
>  #ifdef CONFIG_ARM_UNWIND
>  	ARM_UNWIND_SECTIONS
>  #endif
> @@ -143,6 +136,14 @@ SECTIONS
>  	__init_end = .;
>  
>  	_sdata = .;
> +
> +	. = ALIGN(4);
> +	__ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) {
> +		__start___ex_table = .;
> +		ARM_MMU_KEEP(*(__ex_table))
> +		__stop___ex_table = .;
> +	}
> +
>  	RW_DATA(L1_CACHE_BYTES, PAGE_SIZE, THREAD_SIZE)
>  	_edata = .;

This example has undesirable security implications. It moves the
exception table out of the read-only mappings into the read-write
mappings, thereby providing a way for an attacker to bypass the
read-only protection on the kernel and manipulate code pointers at
potentially known addresses for distro built kernels.

> I agree there is a risk but I don't think it's high. It does not look like
> the minor changes in "reserved" reporting in /proc/iomem will break kexec
> tooling.

What makes you come to that conclusion? The kexec tools architecture
backends get to decide what they do when parsing /proc/iomem.
Currently, only firmware areas are marked reserved in /proc/iomem on
32-bit ARM.

This is read by kexec, and entered into its memory_range[] table as
either RAM, or RESERVED.

kexec uses this to search for a suitable hole in the memory map to
place the kernel in physical memory. The addition of what I will call
ficticious "reserved" areas by the host kernel because the host kernel
happened to use them _will_ have an impact on this.

They _are_ ficticious, because they are purely an artifact of the host
kernel being run, and are of no consequence to tooling such as kexec.
What such tooling is interested in is which areas it needs to avoid
because of firmware.

I think what isn't helping here is that you haven't adequately
described what your overall objective actually is. Framing it in
terms of wanting the reserved memory to be consistent between the
various kernel "interfaces" such as /proc/iomem, the memblock debugfs
and firmware is very ambiguous and open to different interpretations,
whcih I think is what the problem is here.

> Anyway the amount of reserved and free memory depends on a
> particular system, kernel version, configuration and command line.
> I have no intention to report kernel boot time reservations
> to /proc/iomem on architectures that do not report them there today,
> although this also does not seem like a significant factor.

You seem to be missing the point I've tried to make. The areas in
memblock that are marked "reserved" are the areas of reserved memory
from the firmware _plus_ the areas that the kernel has made during
boot which are of no consequence to userspace.

Wanting /proc/iomem, memblock and firmware to all agree on the values
that they mark as "reserved" is IMHO unrealistic.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!