diff mbox series

x86/kaslr: support separate multiple memmaps parameter parsing

Message ID 20201222113124.35367-1-zhangpan26@huawei.com (mailing list archive)
State New, archived
Headers show
Series x86/kaslr: support separate multiple memmaps parameter parsing | expand

Commit Message

Pan Zhang Dec. 22, 2020, 11:31 a.m. UTC
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@huawei.com>
---
 arch/x86/boot/compressed/kaslr.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Comments

Arvind Sankar Jan. 5, 2021, 8:22 p.m. UTC | #1
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@huawei.com>
> ---
>  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
>
diff mbox series

Patch

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)
 		memmap_too_large = true;
 }