Re: [PATCH] x86/kaslr: support separate multiple memmaps parameter parsing

[Arvind Sankar]

On Tue, Dec 22, 2020 at 07:31:24PM +0800, Pan Zhang wrote:
> When the kernel is loading,
> the load address of the kernel needs to be calculated firstly.
> 
> If the kernel address space layout randomization(`kaslr`) is enabled,
> the memory range reserved by the memmap parameter will be excluded
> to avoid loading the kernel address into the memmap reserved area.
> 
> Currently, this is what the manual says:
> 	`memmap = nn [KMG] @ss [KMG]
> 		[KNL] Force usage of a specific region of memory.
>     	Region of memory to be used is from ss to ss + nn.
>    		If @ss [KMG] is omitted, it is equivalent to mem = nn [KMG],
>     	which limits max address to nn [KMG].
> 		Multiple different regions can be specified,
> 		comma delimited.
> 		Example:
> 		memmap=100M@2G, 100M#3G, 1G!1024G
> 	`
> 
> Can we relax the use of memmap?
> In our production environment we see many people who use it like this:
> Separate multiple memmaps parameters to reserve memory,
> memmap=xx\$xxx memmap=xx\$xxx memmap=xx\$xxx memmap=xx\$xxx memmap=xx\$xxx
> 
> If this format is used, and the reserved memory segment is greater than 4,
> there is no way to parse the 5th and subsequent memmaps and the kaslr cannot be disabled by `memmap_too_large`
> so the kernel loading address may fall within the memmap range
> (reserved memory area from memmap after fourth segment),
> which will have bad consequences for use of reserved memory.
> 
> Signed-off-by: Pan Zhang <zhangpan26@xxxxxxxxxx>
> ---
>  arch/x86/boot/compressed/kaslr.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index d7408af..24a2778 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -203,9 +203,6 @@ static void mem_avoid_memmap(enum parse_mode mode, char *str)
>  {
>  	static int i;
>  
> -	if (i >= MAX_MEMMAP_REGIONS)
> -		return;
> -
>  	while (str && (i < MAX_MEMMAP_REGIONS)) {
>  		int rc;
>  		unsigned long long start, size;
> @@ -233,7 +230,7 @@ static void mem_avoid_memmap(enum parse_mode mode, char *str)
>  	}
>  
>  	/* More than 4 memmaps, fail kaslr */
> -	if ((i >= MAX_MEMMAP_REGIONS) && str)
> +	if ((i >= MAX_MEMMAP_REGIONS) && !memmap_too_large)

I think this should stay the way it was, otherwise KASLR will be
disabled even if exactly MAX_MEMMAP_REGIONS were specified. Removing the
early return as you did above should be enough to cause the flag to be
set if a 5th memmap is specified in a separate parameter, right?

>  		memmap_too_large = true;
>  }
>  
> -- 
> 2.7.4
>