lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 26 Apr 2024 09:52:30 -0700
From: "H. Peter Anvin" <hpa@...or.com>
To: Byungchul Park <byungchul@...com>, tglx@...utronix.de, mingo@...hat.com,
        bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org,
        jbohac@...e.cz, dyoung@...hat.com
CC: kernel_team@...ynix.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] x86/e820: apply 'mem=' boot command while reserving memory using boot_params

On April 25, 2024 9:40:18 PM PDT, Byungchul Park <byungchul@...com> wrote:
>On Wed, Apr 24, 2024 at 10:03:13AM +0900, Byungchul Park wrote:
>> I might miss something.  Please lemme know if I go wrong.  Thanks.
>
>I started to work on it since I wanted to limit memory boundary using
>'mem=' boot command but it doesn't work.  However, while looking around
>the code in more detail, I found the issue is about which one should
>have higher priority between:
>
>   1. boot command limiting memory boundary e.g. 'mem=',
>   2. setup data of memory map from bootloader, boot_params.
>
>Based on the current code, setup data from bootloader has higher
>priority than boot command so the setup data can overwrite the user
>defined limit specified in boot command.  Is it inteded?
>
>   If yes, I should stop posting.
>   If not, I will keep posting with the following - v3.
>
>	Byungchul
>
>---
>
>diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
>index 6f1b379e3b38..3bc593235b76 100644
>--- a/arch/x86/kernel/e820.c
>+++ b/arch/x86/kernel/e820.c
>@@ -879,6 +879,7 @@ static void __init early_panic(char *msg)
> }
> 
> static int userdef __initdata;
>+static u64 userdef_mem_limit;
> 
> /* The "mem=nopentium" boot option disables 4MB page tables on 32-bit kernels: */
> static int __init parse_memopt(char *p)
>@@ -905,7 +906,10 @@ static int __init parse_memopt(char *p)
> 	if (mem_size == 0)
> 		return -EINVAL;
> 
>-	e820__range_remove(mem_size, ULLONG_MAX - mem_size, E820_TYPE_RAM, 1);
>+	if (userdef_mem_limit)
>+		userdef_mem_limit = min(userdef_mem_limit, mem_size);
>+	else
>+		userdef_mem_limit = mem_size;
> 
> #ifdef CONFIG_MEMORY_HOTPLUG
> 	max_mem_size = mem_size;
>@@ -966,7 +970,10 @@ static int __init parse_memmap_one(char *p)
> 		else
> 			e820__range_remove(start_at, mem_size, 0, 0);
> 	} else {
>-		e820__range_remove(mem_size, ULLONG_MAX - mem_size, E820_TYPE_RAM, 1);
>+		if (userdef_mem_limit)
>+			userdef_mem_limit = min(userdef_mem_limit, mem_size);
>+		else
>+			userdef_mem_limit = mem_size;
> 	}
> 
> 	return *p == '\0' ? 0 : -EINVAL;
>@@ -1050,6 +1057,11 @@ void __init e820__reserve_setup_data(void)
> void __init e820__finish_early_params(void)
> {
> 	if (userdef) {
>+		if (userdef_mem_limit)
>+			e820__range_remove(userdef_mem_limit,
>+					ULLONG_MAX - userdef_mem_limit,
>+					E820_TYPE_RAM, 1);
>+
> 		if (e820__update_table(e820_table) < 0)
> 			early_panic("Invalid user supplied memory map");
> 
>---
>> 	Byungchul
>> 
>> Changes from v1
>> 	1. before - handle boot_mem_limit assuming the default is U64_MAX.
>> 	   after  - handle boot_mem_limit assuming the default is 0.
>> 
>> --->8---
>> >From e8bf247d6024b35af5300914dcff9135df9c1d66 Mon Sep 17 00:00:00 2001
>> From: Byungchul Park <byungchul@...com>
>> Date: Wed, 24 Apr 2024 09:55:25 +0900
>> Subject: [PATCH v2] x86/e820: apply 'mem=' boot command while reserving memory using boot_params
>> 
>> When a user specifies 'mem=' boot command, it's expected to limit the
>> maximum address of usable memory for the kernel no matter what the
>> memory map source is.  However, 'mem=' boot command doesn't work since
>> it doesn't respect it when reserving memory using boot_params.
>> 
>> Applied the restriction when reserving memory using boot_params.  While
>> at it, renamed mem_size to a more specific name, boot_mem_limit.
>> 
>> Signed-off-by: Byungchul Park <byungchul@...com>
>> ---
>>  arch/x86/kernel/e820.c | 15 +++++++++------
>>  1 file changed, 9 insertions(+), 6 deletions(-)
>> 
>> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
>> index 6f1b379e3b38..e3f716128caf 100644
>> --- a/arch/x86/kernel/e820.c
>> +++ b/arch/x86/kernel/e820.c
>> @@ -880,11 +880,11 @@ static void __init early_panic(char *msg)
>>  
>>  static int userdef __initdata;
>>  
>> +static u64 boot_mem_limit;
>> +
>>  /* The "mem=nopentium" boot option disables 4MB page tables on 32-bit kernels: */
>>  static int __init parse_memopt(char *p)
>>  {
>> -	u64 mem_size;
>> -
>>  	if (!p)
>>  		return -EINVAL;
>>  
>> @@ -899,16 +899,16 @@ static int __init parse_memopt(char *p)
>>  	}
>>  
>>  	userdef = 1;
>> -	mem_size = memparse(p, &p);
>> +	boot_mem_limit = memparse(p, &p);
>>  
>>  	/* Don't remove all memory when getting "mem={invalid}" parameter: */
>> -	if (mem_size == 0)
>> +	if (boot_mem_limit == 0)
>>  		return -EINVAL;
>>  
>> -	e820__range_remove(mem_size, ULLONG_MAX - mem_size, E820_TYPE_RAM, 1);
>> +	e820__range_remove(boot_mem_limit, ULLONG_MAX - boot_mem_limit, E820_TYPE_RAM, 1);
>>  
>>  #ifdef CONFIG_MEMORY_HOTPLUG
>> -	max_mem_size = mem_size;
>> +	max_mem_size = boot_mem_limit;
>>  #endif
>>  
>>  	return 0;
>> @@ -1036,6 +1036,9 @@ void __init e820__reserve_setup_data(void)
>>  		early_memunmap(data, len);
>>  	}
>>  
>> +	if (boot_mem_limit)
>> +		e820__range_remove(boot_mem_limit, ULLONG_MAX - boot_mem_limit,
>> +				E820_TYPE_RESERVED_KERN, 1);
>>  	e820__update_table(e820_table);
>>  
>>  	pr_info("extended physical RAM map:\n");
>> -- 
>> 2.17.1

mem= typically should cap the usable memory at that address. At one point in history we also allowed it to add memory at the top, but modern systems have too complex memory maps; the memmap= option can be used for that, however.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ