[PATCH v7 0/5] x86/KASLR: Add parameter kaslr_mem=nn[KMG]@ss[KMG]

[PATCH v7 0/5] x86/KASLR: Add parameter kaslr_mem=nn[KMG]@ss[KMG]

[Chao Fan]

***Background:
People reported that kaslr may randomly chooses some positions
which are located in movable memory regions. This will break memory
hotplug feature. 

And also on kvm guest with 4GB meory, the good unfragmented 1GB could
be occupied by randomized kernel. It will cause hugetlb failing to
allocate 1GB page. While kernel with 'nokaslr' has not such issue.
This causes regression. Please see the discussion mail:
	https://lkml.org/lkml/2018/1/4/236

***Solutions:
Introduce a new kernel parameter 'kaslr_mem=nn@ss' to let users to
specify the memory regions where kernel can be allowed to randomize
safely.

E.g if 'movable_node' is spedified, we can use 'kaslr_mem=nn@ss' to
tell KASLR where we can put kernel safely. Then KASLR code can avoid
those movable regions and only choose those immovable regions
specified.

For hugetlb case, users can always add 'kaslr_mem=1G' in kernel
cmdline since the 0~1G is always fragmented region because of BIOS
reserved area. Surely users can specify regions more precisely if
they know system memory very well.

*** Issues need be discussed
There are several issues I am not quite sure, please help review and
give suggestions:

1) Since there's already mem_avoid[] which stores the memory regions
KASLR need avoid. For the regions KASLR can safely use, I name it as
mem_usable[], not sure if it's appropriate. Or kaslr_mem[] directly?

2) In v6, I made 'kaslr_mem=' as a kernel parameter which users can use
to specify memory regions where kenrel can be extracted safely by
'kaslr_mem=nn@ss', or regions where we need avoid to extract kernel by
'kaslr_mem=nn!ss'. While later I rethink about it, seems
'kaslr_mem=nn@ss' can satisfy the current requirement, there's no need
to introduce the 'kaslr_mem=nn!ss'. So I just take that
'kaslr_mem=nn!ss' handling patch off, may add it later if anyone think
it's necessary. Any suggestions?
	https://www.spinics.net/lists/kernel/msg2698457.html

***Test results:
 - I did some tests for the memory hotplug issues. I specify the memory
   region in one node, then I found every time the kernel will be
   extracted to the memory of this node.
 - Luiz said he will do some tests for the 1G huge page issue.

***History
v6->v7:
 - Drop the unnecessary avoid part for now.
 - Add document for the new parameter.

v5->v6:
 - Add the last patch to save the avoid memory regions.

v4->v5:
 - Change the problem reported by LKP
Follow Dou's suggestion:
 - Also return if match "movable_node" when parsing kernel commandline
   in handle_mem_filter without define CONFIG_MEMORY_HOTPLUG

v3->v4:
Follow Kees's suggestion:
 - Put the functions variables of immovable_mem to #ifdef
   CONFIG_MEMORY_HOTPLUG and change some code place
 - Change the name of "process_mem_region" to "slots_count"
 - Reanme the new function "process_immovable_mem" to "process_mem_region"
Follow Baoquan's suggestion:
 - Fail KASLR if "movable_node" specified without "immovable_mem"
 - Ajust the code place of handling mem_region directely if no
   immovable_mem specified
Follow Randy's suggestion:
 - Change the mistake and add detailed description for the document.

v2->v3:
Follow Baoquan He's suggestion:
 - Change names of several functions.
 - Add a new parameter "immovable_mem" instead of extending mvoable_node
 - Use the clamp to calculate the memory intersecting, which makes
   logical more clear.
 - Disable memory mirror if movable_node specified

v1->v2:
Follow Dou Liyang's suggestion:
 - Add the parse for movable_node=nn[KMG] without @ss[KMG]
 - Fix the bug for more than one "movable_node=" specified
 - Drop useless variables and use mem_vector region directely
 - Add more comments.

Chao Fan (5):
  x86/KASLR: Add kaslr_mem=nn[KMG]@ss[KMG]
  x86/KASLR: Handle the memory regions specified in kaslr_mem
  x86/KASLR: Give a warning if movable_node specified without kaslr_mem=
  x86/KASLR: Skip memory mirror handling if movable_node specified
  document: add document for kaslr_mem

 Documentation/admin-guide/kernel-parameters.txt |  10 ++
 arch/x86/boot/compressed/kaslr.c                | 154 +++++++++++++++++++++---
 2 files changed, 150 insertions(+), 14 deletions(-)

-- 
2.14.3

[PATCH v7 1/5] x86/KASLR: Add kaslr_mem=nn[KMG]@ss[KMG]

[Chao Fan]

Introduce a new kernel parameter kaslr_mem=nn[KMG]@ss[KMG] which is used
by KASLR only during kernel decompression stage.

Users can use it to specify memory regions where kernel can be randomized
into. E.g if movable_node specified in kernel cmdline, kernel could be
extracted into those movable regions, this will make memory hotplug fail.
With the help of 'kaslr_mem=', limit kernel in those immovable regions
specified.

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
---
 arch/x86/boot/compressed/kaslr.c | 73 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 70 insertions(+), 3 deletions(-)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 8199a6187251..b21741135673 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -108,6 +108,15 @@ enum mem_avoid_index {
 
 static struct mem_vector mem_avoid[MEM_AVOID_MAX];
 
+/* Only support at most 4 usable memory regions specified for kaslr */
+#define MAX_KASLR_MEM_USABLE	4
+
+/* Store the usable memory regions for kaslr */
+static struct mem_vector mem_usable[MAX_KASLR_MEM_USABLE];
+
+/* The amount of usable regions for kaslr user specify, not more than 4 */
+static int num_usable_region;
+
 static bool mem_overlaps(struct mem_vector *one, struct mem_vector *two)
 {
 	/* Item one is entirely before item two. */
@@ -206,7 +215,62 @@ static void mem_avoid_memmap(char *str)
 		memmap_too_large = true;
 }
 
-static int handle_mem_memmap(void)
+static int parse_kaslr_mem(char *p,
+			   unsigned long long *start,
+			   unsigned long long *size)
+{
+	char *oldp;
+
+	if (!p)
+		return -EINVAL;
+
+	oldp = p;
+	*size = memparse(p, &p);
+	if (p == oldp)
+		return -EINVAL;
+
+	switch (*p) {
+	case '@':
+		*start = memparse(p + 1, &p);
+		return 0;
+	default:
+		/*
+		 * If w/o offset, only size specified, kaslr_mem=nn[KMG]
+		 * has the same behaviour as kaslr_mem=nn[KMG]@0. It means
+		 * the region starts from 0.
+		 */
+		*start = 0;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static void parse_kaslr_mem_regions(char *str)
+{
+	static int i;
+
+	while (str && (i < MAX_KASLR_MEM_USABLE)) {
+		int rc;
+		unsigned long long start, size;
+		char *k = strchr(str, ',');
+
+		if (k)
+			*k++ = 0;
+
+		rc = parse_kaslr_mem(str, &start, &size);
+		if (rc < 0)
+			break;
+		str = k;
+
+		mem_usable[i].start = start;
+		mem_usable[i].size = size;
+		i++;
+	}
+	num_usable_region = i;
+}
+
+static int handle_mem_filter(void)
 {
 	char *args = (char *)get_cmd_line_ptr();
 	size_t len = strlen((char *)args);
@@ -214,7 +278,8 @@ static int handle_mem_memmap(void)
 	char *param, *val;
 	u64 mem_size;
 
-	if (!strstr(args, "memmap=") && !strstr(args, "mem="))
+	if (!strstr(args, "memmap=") && !strstr(args, "mem=") &&
+	    !strstr(args, "kaslr_mem="))
 		return 0;
 
 	tmp_cmdline = malloc(len + 1);
@@ -239,6 +304,8 @@ static int handle_mem_memmap(void)
 
 		if (!strcmp(param, "memmap")) {
 			mem_avoid_memmap(val);
+		} else if (!strcmp(param, "kaslr_mem")) {
+			parse_kaslr_mem_regions(val);
 		} else if (!strcmp(param, "mem")) {
 			char *p = val;
 
@@ -378,7 +445,7 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
 	/* We don't need to set a mapping for setup_data. */
 
 	/* Mark the memmap regions we need to avoid */
-	handle_mem_memmap();
+	handle_mem_filter();
 
 #ifdef CONFIG_X86_VERBOSE_BOOTUP
 	/* Make sure video RAM can be used. */
-- 
2.14.3

[PATCH v7 2/5] x86/KASLR: Handle the memory regions specified in kaslr_mem

[Chao Fan]

If no 'kaslr_mem=' specified, just handle the e820/efi entries directly
as before. Otherwise, limit kernel to memory regions specified in
'kaslr_mem=' commandline.

Rename process_mem_region to slots_count to match
slots_fetch_random, and name new function as process_mem_region.

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
---
 arch/x86/boot/compressed/kaslr.c | 64 +++++++++++++++++++++++++++++++++-------
 1 file changed, 53 insertions(+), 11 deletions(-)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index b21741135673..b200a7ceafc1 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -548,9 +548,9 @@ static unsigned long slots_fetch_random(void)
 	return 0;
 }
 
-static void process_mem_region(struct mem_vector *entry,
-			       unsigned long minimum,
-			       unsigned long image_size)
+static void slots_count(struct mem_vector *entry,
+			unsigned long minimum,
+			unsigned long image_size)
 {
 	struct mem_vector region, overlap;
 	struct slot_area slot_area;
@@ -627,6 +627,52 @@ static void process_mem_region(struct mem_vector *entry,
 	}
 }
 
+static bool process_mem_region(struct mem_vector region,
+			       unsigned long long minimum,
+			       unsigned long long image_size)
+{
+	/*
+	 * If kaslr_mem= specified, walk all the regions, and
+	 * filter the intersection to slots_count.
+	 */
+	if (num_usable_region > 0) {
+		int i;
+
+		for (i = 0; i < num_usable_region; i++) {
+			struct mem_vector entry;
+			unsigned long long start, end, entry_end, region_end;
+
+			start = mem_usable[i].start;
+			end = start + mem_usable[i].size;
+			region_end = region.start + region.size;
+
+			entry.start = clamp(region.start, start, end);
+			entry_end = clamp(region_end, start, end);
+
+			if (entry.start < entry_end) {
+				entry.size = entry_end - entry.start;
+				slots_count(&entry, minimum, image_size);
+			}
+
+			if (slot_area_index == MAX_SLOT_AREA) {
+				debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
+				return 1;
+			}
+		}
+		return 0;
+	}
+
+	/*
+	 * If no kaslr_mem stored, use region directly
+	 */
+	slots_count(&region, minimum, image_size);
+	if (slot_area_index == MAX_SLOT_AREA) {
+		debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
+		return 1;
+	}
+	return 0;
+}
+
 #ifdef CONFIG_EFI
 /*
  * Returns true if mirror region found (and must have been processed
@@ -692,11 +738,9 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
 
 		region.start = md->phys_addr;
 		region.size = md->num_pages << EFI_PAGE_SHIFT;
-		process_mem_region(&region, minimum, image_size);
-		if (slot_area_index == MAX_SLOT_AREA) {
-			debug_putstr("Aborted EFI scan (slot_areas full)!\n");
+
+		if (process_mem_region(region, minimum, image_size))
 			break;
-		}
 	}
 	return true;
 }
@@ -723,11 +767,9 @@ static void process_e820_entries(unsigned long minimum,
 			continue;
 		region.start = entry->addr;
 		region.size = entry->size;
-		process_mem_region(&region, minimum, image_size);
-		if (slot_area_index == MAX_SLOT_AREA) {
-			debug_putstr("Aborted e820 scan (slot_areas full)!\n");
+
+		if (process_mem_region(region, minimum, image_size))
 			break;
-		}
 	}
 }
 
-- 
2.14.3

[PATCH v7 3/5] x86/KASLR: Give a warning if movable_node specified without kaslr_mem=

[Chao Fan]

Since only 'movable_node' specified without 'kaslr_mem=' may break
memory hotplug, so reconmmend users using 'kaslr_mem=' when
'movable_node' specified..

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
---
 arch/x86/boot/compressed/kaslr.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index b200a7ceafc1..dca846b522fc 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -282,6 +282,16 @@ static int handle_mem_filter(void)
 	    !strstr(args, "kaslr_mem="))
 		return 0;
 
+#ifdef CONFIG_MEMORY_HOTPLUG
+	/*
+	 * Check if "kaslr_mem=" specified when "movable_node" found. If not,
+	 * just give warrning. Otherwise memory hotplug could be
+	 * affected if kernel put on movable memory regions.
+	 */
+	if (strstr(args, "movable_node") && !strstr(args, "kaslr_mem="))
+		warn("kaslr_mem= should specified when using movable_node.\n");
+#endif
+
 	tmp_cmdline = malloc(len + 1);
 	if (!tmp_cmdline)
 		error("Failed to allocate space for tmp_cmdline");
-- 
2.14.3

[PATCH v7 4/5] x86/KASLR: Skip memory mirror handling if movable_node specified

[Chao Fan]

In kernel code, if movable_node specified, it will skip the mirror
feature. So we should also skip mirror feature in KASLR.

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
---
 arch/x86/boot/compressed/kaslr.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index dca846b522fc..84b9db26d026 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -692,6 +692,7 @@ static bool
 process_efi_entries(unsigned long minimum, unsigned long image_size)
 {
 	struct efi_info *e = &boot_params->efi_info;
+	char *args = (char *)get_cmd_line_ptr();
 	bool efi_mirror_found = false;
 	struct mem_vector region;
 	efi_memory_desc_t *md;
@@ -725,6 +726,12 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
 		}
 	}
 
+#ifdef CONFIG_MEMORY_HOTPLUG
+	/* Skip memory mirror if movabale_node or immovable_mem specified */
+	if (strstr(args, "movable_node"))
+		efi_mirror_found = false;
+#endif
+
 	for (i = 0; i < nr_desc; i++) {
 		md = efi_early_memdesc_ptr(pmap, e->efi_memdesc_size, i);
 
-- 
2.14.3

[PATCH v7 5/5] document: add document for kaslr_mem

[Chao Fan]

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index e2de7c006a74..f6d5adde1a73 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2350,6 +2350,16 @@
 			allocations which rules out almost all kernel
 			allocations. Use with caution!
 
+	kaslr_mem=nn[KMG][@ss[KMG]]
+			[KNL] Force usage of a specific region of memory.
+			Make some features, like memory hotplug and 1G huge
+			page work well with KASLR. Region of usable memory is
+			from ss to ss+nn. If ss is omitted, it defaults to 0.
+			Multiple regions can be specified, comma delimited.
+			Notice: we support 4 regions at most now.
+			Example:
+			kaslr_mem=1G,500M@2G,1G@4G
+
 	MTD_Partition=	[MTD]
 			Format: <name>,<region-number>,<size>,<offset>
 
-- 
2.14.3

Re: [PATCH v7 3/5] x86/KASLR: Give a warning if movable_node specified without kaslr_mem=

[Baoquan He]

On 01/17/18 at 06:53pm, Chao Fan wrote:
> Since only 'movable_node' specified without 'kaslr_mem=' may break
> memory hotplug, so reconmmend users using 'kaslr_mem=' when
> 'movable_node' specified..
> 
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  arch/x86/boot/compressed/kaslr.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index b200a7ceafc1..dca846b522fc 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -282,6 +282,16 @@ static int handle_mem_filter(void)
>  	    !strstr(args, "kaslr_mem="))
>  		return 0;
>  
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +	/*
> +	 * Check if "kaslr_mem=" specified when "movable_node" found. If not,
> +	 * just give warrning. Otherwise memory hotplug could be
> +	 * affected if kernel put on movable memory regions.
> +	 */
> +	if (strstr(args, "movable_node") && !strstr(args, "kaslr_mem="))
> +		warn("kaslr_mem= should specified when using movable_node.\n");
		      'kaslr_mem='     ^be
> +#endif
> +
>  	tmp_cmdline = malloc(len + 1);
>  	if (!tmp_cmdline)
>  		error("Failed to allocate space for tmp_cmdline");
> -- 
> 2.14.3
> 
> 
> 

Re: [PATCH v7 4/5] x86/KASLR: Skip memory mirror handling if movable_node specified

[Baoquan He]

On 01/17/18 at 06:53pm, Chao Fan wrote:
> In kernel code, if movable_node specified, it will skip the mirror
> feature. So we should also skip mirror feature in KASLR.
> 
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  arch/x86/boot/compressed/kaslr.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index dca846b522fc..84b9db26d026 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -692,6 +692,7 @@ static bool
>  process_efi_entries(unsigned long minimum, unsigned long image_size)
>  {
>  	struct efi_info *e = &boot_params->efi_info;
> +	char *args = (char *)get_cmd_line_ptr();
>  	bool efi_mirror_found = false;
>  	struct mem_vector region;
>  	efi_memory_desc_t *md;
> @@ -725,6 +726,12 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
>  		}
>  	}
>  
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +	/* Skip memory mirror if movabale_node or immovable_mem specified */
						  ~~~~~~~~~~~~~
						  ^^ need be removed?
> +	if (strstr(args, "movable_node"))
> +		efi_mirror_found = false;
> +#endif
> +
>  	for (i = 0; i < nr_desc; i++) {
>  		md = efi_early_memdesc_ptr(pmap, e->efi_memdesc_size, i);
>  
> -- 
> 2.14.3
> 
> 
> 

Re: [PATCH v7 3/5] x86/KASLR: Give a warning if movable_node specified without kaslr_mem=

[Baoquan He]

On 01/17/18 at 06:53pm, Chao Fan wrote:
> Since only 'movable_node' specified without 'kaslr_mem=' may break
> memory hotplug, so reconmmend users using 'kaslr_mem=' when
> 'movable_node' specified..
> 
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  arch/x86/boot/compressed/kaslr.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index b200a7ceafc1..dca846b522fc 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -282,6 +282,16 @@ static int handle_mem_filter(void)
>  	    !strstr(args, "kaslr_mem="))
>  		return 0;
>  
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +	/*
> +	 * Check if "kaslr_mem=" specified when "movable_node" found. If not,
> +	 * just give warrning. Otherwise memory hotplug could be
> +	 * affected if kernel put on movable memory regions.
			     ^is put
> +	 */
> +	if (strstr(args, "movable_node") && !strstr(args, "kaslr_mem="))
> +		warn("kaslr_mem= should specified when using movable_node.\n");
> +#endif
> +
>  	tmp_cmdline = malloc(len + 1);
>  	if (!tmp_cmdline)
>  		error("Failed to allocate space for tmp_cmdline");
> -- 
> 2.14.3
> 
> 
> 

Re: [PATCH v7 0/5] x86/KASLR: Add parameter kaslr_mem=nn[KMG]@ss[KMG]

[Luiz Capitulino]

On Wed, 17 Jan 2018 18:53:46 +0800
Chao Fan <fanc.fnst@cn.fujitsu.com> wrote:

> ***Background:
> People reported that kaslr may randomly chooses some positions
> which are located in movable memory regions. This will break memory
> hotplug feature. 
> 
> And also on kvm guest with 4GB meory, the good unfragmented 1GB could
> be occupied by randomized kernel. It will cause hugetlb failing to
> allocate 1GB page. While kernel with 'nokaslr' has not such issue.
> This causes regression. Please see the discussion mail:
> 	https://lkml.org/lkml/2018/1/4/236
> 
> ***Solutions:
> Introduce a new kernel parameter 'kaslr_mem=nn@ss' to let users to
> specify the memory regions where kernel can be allowed to randomize
> safely.

I've tested this series with a 4GB KVM guest. With kaslr_mem=1G, I
got one 1GB page allocated 100% of the time in 85 boots. Without
kaslr_mem=, I got 3 failures in only 10 boots (that is, in 3 boots
I had no 1GB page allocated).

So, this series solves the 1GB page problem for me.

> 
> E.g if 'movable_node' is spedified, we can use 'kaslr_mem=nn@ss' to
> tell KASLR where we can put kernel safely. Then KASLR code can avoid
> those movable regions and only choose those immovable regions
> specified.
> 
> For hugetlb case, users can always add 'kaslr_mem=1G' in kernel
> cmdline since the 0~1G is always fragmented region because of BIOS
> reserved area. Surely users can specify regions more precisely if
> they know system memory very well.
> 
> *** Issues need be discussed
> There are several issues I am not quite sure, please help review and
> give suggestions:
> 
> 1) Since there's already mem_avoid[] which stores the memory regions
> KASLR need avoid. For the regions KASLR can safely use, I name it as
> mem_usable[], not sure if it's appropriate. Or kaslr_mem[] directly?
> 
> 2) In v6, I made 'kaslr_mem=' as a kernel parameter which users can use
> to specify memory regions where kenrel can be extracted safely by
> 'kaslr_mem=nn@ss', or regions where we need avoid to extract kernel by
> 'kaslr_mem=nn!ss'. While later I rethink about it, seems
> 'kaslr_mem=nn@ss' can satisfy the current requirement, there's no need
> to introduce the 'kaslr_mem=nn!ss'. So I just take that
> 'kaslr_mem=nn!ss' handling patch off, may add it later if anyone think
> it's necessary. Any suggestions?
> 	https://www.spinics.net/lists/kernel/msg2698457.html
> 
> ***Test results:
>  - I did some tests for the memory hotplug issues. I specify the memory
>    region in one node, then I found every time the kernel will be
>    extracted to the memory of this node.
>  - Luiz said he will do some tests for the 1G huge page issue.
> 
> ***History
> v6->v7:
>  - Drop the unnecessary avoid part for now.
>  - Add document for the new parameter.
> 
> v5->v6:
>  - Add the last patch to save the avoid memory regions.
> 
> v4->v5:
>  - Change the problem reported by LKP
> Follow Dou's suggestion:
>  - Also return if match "movable_node" when parsing kernel commandline
>    in handle_mem_filter without define CONFIG_MEMORY_HOTPLUG
> 
> v3->v4:
> Follow Kees's suggestion:
>  - Put the functions variables of immovable_mem to #ifdef
>    CONFIG_MEMORY_HOTPLUG and change some code place
>  - Change the name of "process_mem_region" to "slots_count"
>  - Reanme the new function "process_immovable_mem" to "process_mem_region"
> Follow Baoquan's suggestion:
>  - Fail KASLR if "movable_node" specified without "immovable_mem"
>  - Ajust the code place of handling mem_region directely if no
>    immovable_mem specified
> Follow Randy's suggestion:
>  - Change the mistake and add detailed description for the document.
> 
> v2->v3:
> Follow Baoquan He's suggestion:
>  - Change names of several functions.
>  - Add a new parameter "immovable_mem" instead of extending mvoable_node
>  - Use the clamp to calculate the memory intersecting, which makes
>    logical more clear.
>  - Disable memory mirror if movable_node specified
> 
> v1->v2:
> Follow Dou Liyang's suggestion:
>  - Add the parse for movable_node=nn[KMG] without @ss[KMG]
>  - Fix the bug for more than one "movable_node=" specified
>  - Drop useless variables and use mem_vector region directely
>  - Add more comments.
> 
> Chao Fan (5):
>   x86/KASLR: Add kaslr_mem=nn[KMG]@ss[KMG]
>   x86/KASLR: Handle the memory regions specified in kaslr_mem
>   x86/KASLR: Give a warning if movable_node specified without kaslr_mem=
>   x86/KASLR: Skip memory mirror handling if movable_node specified
>   document: add document for kaslr_mem
> 
>  Documentation/admin-guide/kernel-parameters.txt |  10 ++
>  arch/x86/boot/compressed/kaslr.c                | 154 +++++++++++++++++++++---
>  2 files changed, 150 insertions(+), 14 deletions(-)
> 

Re: [PATCH v7 0/5] x86/KASLR: Add parameter kaslr_mem=nn[KMG]@ss[KMG]

[Chao Fan]

On Wed, Jan 17, 2018 at 12:32:35PM -0500, Luiz Capitulino wrote:
>On Wed, 17 Jan 2018 18:53:46 +0800
>Chao Fan <fanc.fnst@cn.fujitsu.com> wrote:
>
>> ***Background:
>> People reported that kaslr may randomly chooses some positions
>> which are located in movable memory regions. This will break memory
>> hotplug feature. 
>> 
>> And also on kvm guest with 4GB meory, the good unfragmented 1GB could
>> be occupied by randomized kernel. It will cause hugetlb failing to
>> allocate 1GB page. While kernel with 'nokaslr' has not such issue.
>> This causes regression. Please see the discussion mail:
>> 	https://lkml.org/lkml/2018/1/4/236
>> 
>> ***Solutions:
>> Introduce a new kernel parameter 'kaslr_mem=nn@ss' to let users to
>> specify the memory regions where kernel can be allowed to randomize
>> safely.
>
>I've tested this series with a 4GB KVM guest. With kaslr_mem=1G, I
>got one 1GB page allocated 100% of the time in 85 boots. Without
>kaslr_mem=, I got 3 failures in only 10 boots (that is, in 3 boots
>I had no 1GB page allocated).
>
>So, this series solves the 1GB page problem for me.
>


Thanks for Luiz's test.

Thanks,
Chao Fan

>> 
>> E.g if 'movable_node' is spedified, we can use 'kaslr_mem=nn@ss' to
>> tell KASLR where we can put kernel safely. Then KASLR code can avoid
>> those movable regions and only choose those immovable regions
>> specified.
>> 
>> For hugetlb case, users can always add 'kaslr_mem=1G' in kernel
>> cmdline since the 0~1G is always fragmented region because of BIOS
>> reserved area. Surely users can specify regions more precisely if
>> they know system memory very well.
>> 
>> *** Issues need be discussed
>> There are several issues I am not quite sure, please help review and
>> give suggestions:
>> 
>> 1) Since there's already mem_avoid[] which stores the memory regions
>> KASLR need avoid. For the regions KASLR can safely use, I name it as
>> mem_usable[], not sure if it's appropriate. Or kaslr_mem[] directly?
>> 
>> 2) In v6, I made 'kaslr_mem=' as a kernel parameter which users can use
>> to specify memory regions where kenrel can be extracted safely by
>> 'kaslr_mem=nn@ss', or regions where we need avoid to extract kernel by
>> 'kaslr_mem=nn!ss'. While later I rethink about it, seems
>> 'kaslr_mem=nn@ss' can satisfy the current requirement, there's no need
>> to introduce the 'kaslr_mem=nn!ss'. So I just take that
>> 'kaslr_mem=nn!ss' handling patch off, may add it later if anyone think
>> it's necessary. Any suggestions?
>> 	https://www.spinics.net/lists/kernel/msg2698457.html
>> 
>> ***Test results:
>>  - I did some tests for the memory hotplug issues. I specify the memory
>>    region in one node, then I found every time the kernel will be
>>    extracted to the memory of this node.
>>  - Luiz said he will do some tests for the 1G huge page issue.
>> 
>> ***History
>> v6->v7:
>>  - Drop the unnecessary avoid part for now.
>>  - Add document for the new parameter.
>> 
>> v5->v6:
>>  - Add the last patch to save the avoid memory regions.
>> 
>> v4->v5:
>>  - Change the problem reported by LKP
>> Follow Dou's suggestion:
>>  - Also return if match "movable_node" when parsing kernel commandline
>>    in handle_mem_filter without define CONFIG_MEMORY_HOTPLUG
>> 
>> v3->v4:
>> Follow Kees's suggestion:
>>  - Put the functions variables of immovable_mem to #ifdef
>>    CONFIG_MEMORY_HOTPLUG and change some code place
>>  - Change the name of "process_mem_region" to "slots_count"
>>  - Reanme the new function "process_immovable_mem" to "process_mem_region"
>> Follow Baoquan's suggestion:
>>  - Fail KASLR if "movable_node" specified without "immovable_mem"
>>  - Ajust the code place of handling mem_region directely if no
>>    immovable_mem specified
>> Follow Randy's suggestion:
>>  - Change the mistake and add detailed description for the document.
>> 
>> v2->v3:
>> Follow Baoquan He's suggestion:
>>  - Change names of several functions.
>>  - Add a new parameter "immovable_mem" instead of extending mvoable_node
>>  - Use the clamp to calculate the memory intersecting, which makes
>>    logical more clear.
>>  - Disable memory mirror if movable_node specified
>> 
>> v1->v2:
>> Follow Dou Liyang's suggestion:
>>  - Add the parse for movable_node=nn[KMG] without @ss[KMG]
>>  - Fix the bug for more than one "movable_node=" specified
>>  - Drop useless variables and use mem_vector region directely
>>  - Add more comments.
>> 
>> Chao Fan (5):
>>   x86/KASLR: Add kaslr_mem=nn[KMG]@ss[KMG]
>>   x86/KASLR: Handle the memory regions specified in kaslr_mem
>>   x86/KASLR: Give a warning if movable_node specified without kaslr_mem=
>>   x86/KASLR: Skip memory mirror handling if movable_node specified
>>   document: add document for kaslr_mem
>> 
>>  Documentation/admin-guide/kernel-parameters.txt |  10 ++
>>  arch/x86/boot/compressed/kaslr.c                | 154 +++++++++++++++++++++---
>>  2 files changed, 150 insertions(+), 14 deletions(-)
>> 
>
>
>

Re: [PATCH v7 4/5] x86/KASLR: Skip memory mirror handling if movable_node specified

[Chao Fan]

On Wed, Jan 17, 2018 at 10:03:54PM +0800, Baoquan He wrote:
>On 01/17/18 at 06:53pm, Chao Fan wrote:
>> In kernel code, if movable_node specified, it will skip the mirror
>> feature. So we should also skip mirror feature in KASLR.
>> 
>> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
>> ---
>>  arch/x86/boot/compressed/kaslr.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>> 
>> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
>> index dca846b522fc..84b9db26d026 100644
>> --- a/arch/x86/boot/compressed/kaslr.c
>> +++ b/arch/x86/boot/compressed/kaslr.c
>> @@ -692,6 +692,7 @@ static bool
>>  process_efi_entries(unsigned long minimum, unsigned long image_size)
>>  {
>>  	struct efi_info *e = &boot_params->efi_info;
>> +	char *args = (char *)get_cmd_line_ptr();
>>  	bool efi_mirror_found = false;
>>  	struct mem_vector region;
>>  	efi_memory_desc_t *md;
>> @@ -725,6 +726,12 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
>>  		}
>>  	}
>>  
>> +#ifdef CONFIG_MEMORY_HOTPLUG
>> +	/* Skip memory mirror if movabale_node or immovable_mem specified */
>						  ~~~~~~~~~~~~~
>						  ^^ need be removed?

Yes, sorry for the mistake, I change the old versions and miss some
points. Will change that.

Thanks,
Chao Fan

>> +	if (strstr(args, "movable_node"))
>> +		efi_mirror_found = false;
>> +#endif
>> +
>>  	for (i = 0; i < nr_desc; i++) {
>>  		md = efi_early_memdesc_ptr(pmap, e->efi_memdesc_size, i);
>>  
>> -- 
>> 2.14.3
>> 
>> 
>> 
>
>

Re: [PATCH v7 3/5] x86/KASLR: Give a warning if movable_node specified without kaslr_mem=

[Chao Fan]

On Wed, Jan 17, 2018 at 10:02:48PM +0800, Baoquan He wrote:
>On 01/17/18 at 06:53pm, Chao Fan wrote:
>> Since only 'movable_node' specified without 'kaslr_mem=' may break
>> memory hotplug, so reconmmend users using 'kaslr_mem=' when
>> 'movable_node' specified..
>> 
>> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
>> ---
>>  arch/x86/boot/compressed/kaslr.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>> 
>> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
>> index b200a7ceafc1..dca846b522fc 100644
>> --- a/arch/x86/boot/compressed/kaslr.c
>> +++ b/arch/x86/boot/compressed/kaslr.c
>> @@ -282,6 +282,16 @@ static int handle_mem_filter(void)
>>  	    !strstr(args, "kaslr_mem="))
>>  		return 0;
>>  
>> +#ifdef CONFIG_MEMORY_HOTPLUG
>> +	/*
>> +	 * Check if "kaslr_mem=" specified when "movable_node" found. If not,
>> +	 * just give warrning. Otherwise memory hotplug could be
>> +	 * affected if kernel put on movable memory regions.
>> +	 */
>> +	if (strstr(args, "movable_node") && !strstr(args, "kaslr_mem="))
>> +		warn("kaslr_mem= should specified when using movable_node.\n");
>		      'kaslr_mem='     ^be

You have given me a large help for my poor English.

Thanks,
Chao Fan

>> +#endif
>> +
>>  	tmp_cmdline = malloc(len + 1);
>>  	if (!tmp_cmdline)
>>  		error("Failed to allocate space for tmp_cmdline");
>> -- 
>> 2.14.3
>> 
>> 
>> 
>
>

Re: [PATCH v7 0/5] x86/KASLR: Add parameter kaslr_mem=nn[KMG]@ss[KMG]

[Luiz Capitulino]

On Thu, 18 Jan 2018 09:11:14 +0800
Chao Fan <fanc.fnst@cn.fujitsu.com> wrote:

> On Wed, Jan 17, 2018 at 12:32:35PM -0500, Luiz Capitulino wrote:
> >On Wed, 17 Jan 2018 18:53:46 +0800
> >Chao Fan <fanc.fnst@cn.fujitsu.com> wrote:
> >  
> >> ***Background:
> >> People reported that kaslr may randomly chooses some positions
> >> which are located in movable memory regions. This will break memory
> >> hotplug feature. 
> >> 
> >> And also on kvm guest with 4GB meory, the good unfragmented 1GB could
> >> be occupied by randomized kernel. It will cause hugetlb failing to
> >> allocate 1GB page. While kernel with 'nokaslr' has not such issue.
> >> This causes regression. Please see the discussion mail:
> >> 	https://lkml.org/lkml/2018/1/4/236
> >> 
> >> ***Solutions:
> >> Introduce a new kernel parameter 'kaslr_mem=nn@ss' to let users to
> >> specify the memory regions where kernel can be allowed to randomize
> >> safely.  
> >
> >I've tested this series with a 4GB KVM guest. With kaslr_mem=1G, I
> >got one 1GB page allocated 100% of the time in 85 boots. Without
> >kaslr_mem=, I got 3 failures in only 10 boots (that is, in 3 boots
> >I had no 1GB page allocated).
> >
> >So, this series solves the 1GB page problem for me.
> >  
> 
> 
> Thanks for Luiz's test.

Btw, my test tested a simple single case, but I think you can add:

Tested-by: Luiz Capitulino <lcapitulino@redhat.com>

> 
> Thanks,
> Chao Fan
> 
> >> 
> >> E.g if 'movable_node' is spedified, we can use 'kaslr_mem=nn@ss' to
> >> tell KASLR where we can put kernel safely. Then KASLR code can avoid
> >> those movable regions and only choose those immovable regions
> >> specified.
> >> 
> >> For hugetlb case, users can always add 'kaslr_mem=1G' in kernel
> >> cmdline since the 0~1G is always fragmented region because of BIOS
> >> reserved area. Surely users can specify regions more precisely if
> >> they know system memory very well.
> >> 
> >> *** Issues need be discussed
> >> There are several issues I am not quite sure, please help review and
> >> give suggestions:
> >> 
> >> 1) Since there's already mem_avoid[] which stores the memory regions
> >> KASLR need avoid. For the regions KASLR can safely use, I name it as
> >> mem_usable[], not sure if it's appropriate. Or kaslr_mem[] directly?
> >> 
> >> 2) In v6, I made 'kaslr_mem=' as a kernel parameter which users can use
> >> to specify memory regions where kenrel can be extracted safely by
> >> 'kaslr_mem=nn@ss', or regions where we need avoid to extract kernel by
> >> 'kaslr_mem=nn!ss'. While later I rethink about it, seems
> >> 'kaslr_mem=nn@ss' can satisfy the current requirement, there's no need
> >> to introduce the 'kaslr_mem=nn!ss'. So I just take that
> >> 'kaslr_mem=nn!ss' handling patch off, may add it later if anyone think
> >> it's necessary. Any suggestions?
> >> 	https://www.spinics.net/lists/kernel/msg2698457.html
> >> 
> >> ***Test results:
> >>  - I did some tests for the memory hotplug issues. I specify the memory
> >>    region in one node, then I found every time the kernel will be
> >>    extracted to the memory of this node.
> >>  - Luiz said he will do some tests for the 1G huge page issue.
> >> 
> >> ***History
> >> v6->v7:
> >>  - Drop the unnecessary avoid part for now.
> >>  - Add document for the new parameter.
> >> 
> >> v5->v6:
> >>  - Add the last patch to save the avoid memory regions.
> >> 
> >> v4->v5:
> >>  - Change the problem reported by LKP
> >> Follow Dou's suggestion:
> >>  - Also return if match "movable_node" when parsing kernel commandline
> >>    in handle_mem_filter without define CONFIG_MEMORY_HOTPLUG
> >> 
> >> v3->v4:
> >> Follow Kees's suggestion:
> >>  - Put the functions variables of immovable_mem to #ifdef
> >>    CONFIG_MEMORY_HOTPLUG and change some code place
> >>  - Change the name of "process_mem_region" to "slots_count"
> >>  - Reanme the new function "process_immovable_mem" to "process_mem_region"
> >> Follow Baoquan's suggestion:
> >>  - Fail KASLR if "movable_node" specified without "immovable_mem"
> >>  - Ajust the code place of handling mem_region directely if no
> >>    immovable_mem specified
> >> Follow Randy's suggestion:
> >>  - Change the mistake and add detailed description for the document.
> >> 
> >> v2->v3:
> >> Follow Baoquan He's suggestion:
> >>  - Change names of several functions.
> >>  - Add a new parameter "immovable_mem" instead of extending mvoable_node
> >>  - Use the clamp to calculate the memory intersecting, which makes
> >>    logical more clear.
> >>  - Disable memory mirror if movable_node specified
> >> 
> >> v1->v2:
> >> Follow Dou Liyang's suggestion:
> >>  - Add the parse for movable_node=nn[KMG] without @ss[KMG]
> >>  - Fix the bug for more than one "movable_node=" specified
> >>  - Drop useless variables and use mem_vector region directely
> >>  - Add more comments.
> >> 
> >> Chao Fan (5):
> >>   x86/KASLR: Add kaslr_mem=nn[KMG]@ss[KMG]
> >>   x86/KASLR: Handle the memory regions specified in kaslr_mem
> >>   x86/KASLR: Give a warning if movable_node specified without kaslr_mem=
> >>   x86/KASLR: Skip memory mirror handling if movable_node specified
> >>   document: add document for kaslr_mem
> >> 
> >>  Documentation/admin-guide/kernel-parameters.txt |  10 ++
> >>  arch/x86/boot/compressed/kaslr.c                | 154 +++++++++++++++++++++---
> >>  2 files changed, 150 insertions(+), 14 deletions(-)
> >>   
> >
> >
> >  
> 
> 

Re: [PATCH v7 1/5] x86/KASLR: Add kaslr_mem=nn[KMG]@ss[KMG]

[Baoquan He]

On 01/17/18 at 06:53pm, Chao Fan wrote:
> Introduce a new kernel parameter kaslr_mem=nn[KMG]@ss[KMG] which is used
> by KASLR only during kernel decompression stage.
> 
> Users can use it to specify memory regions where kernel can be randomized
> into. E.g if movable_node specified in kernel cmdline, kernel could be
  ~ remove 'into'
> extracted into those movable regions, this will make memory hotplug fail.
> With the help of 'kaslr_mem=', limit kernel in those immovable regions
> specified.
> 
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  arch/x86/boot/compressed/kaslr.c | 73 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 70 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 8199a6187251..b21741135673 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -108,6 +108,15 @@ enum mem_avoid_index {
>  
>  static struct mem_vector mem_avoid[MEM_AVOID_MAX];
>  
> +/* Only support at most 4 usable memory regions specified for kaslr */
> +#define MAX_KASLR_MEM_USABLE	4
> +
> +/* Store the usable memory regions for kaslr */
> +static struct mem_vector mem_usable[MAX_KASLR_MEM_USABLE];

The name xx_usable sounds not so good, while I don't know what
is better. Otherwise this patch looks good to me.

Ack it.

Acked-by: Baoquan He <bhe@redhat.com>

> +
> +/* The amount of usable regions for kaslr user specify, not more than 4 */
> +static int num_usable_region;
> +
>  static bool mem_overlaps(struct mem_vector *one, struct mem_vector *two)
>  {
>  	/* Item one is entirely before item two. */
> @@ -206,7 +215,62 @@ static void mem_avoid_memmap(char *str)
>  		memmap_too_large = true;
>  }
>  
> -static int handle_mem_memmap(void)
> +static int parse_kaslr_mem(char *p,
> +			   unsigned long long *start,
> +			   unsigned long long *size)
> +{
> +	char *oldp;
> +
> +	if (!p)
> +		return -EINVAL;
> +
> +	oldp = p;
> +	*size = memparse(p, &p);
> +	if (p == oldp)
> +		return -EINVAL;
> +
> +	switch (*p) {
> +	case '@':
> +		*start = memparse(p + 1, &p);
> +		return 0;
> +	default:
> +		/*
> +		 * If w/o offset, only size specified, kaslr_mem=nn[KMG]
> +		 * has the same behaviour as kaslr_mem=nn[KMG]@0. It means
> +		 * the region starts from 0.
> +		 */
> +		*start = 0;
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static void parse_kaslr_mem_regions(char *str)
> +{
> +	static int i;
> +
> +	while (str && (i < MAX_KASLR_MEM_USABLE)) {
> +		int rc;
> +		unsigned long long start, size;
> +		char *k = strchr(str, ',');
> +
> +		if (k)
> +			*k++ = 0;
> +
> +		rc = parse_kaslr_mem(str, &start, &size);
> +		if (rc < 0)
> +			break;
> +		str = k;
> +
> +		mem_usable[i].start = start;
> +		mem_usable[i].size = size;
> +		i++;
> +	}
> +	num_usable_region = i;
> +}
> +
> +static int handle_mem_filter(void)
>  {
>  	char *args = (char *)get_cmd_line_ptr();
>  	size_t len = strlen((char *)args);
> @@ -214,7 +278,8 @@ static int handle_mem_memmap(void)
>  	char *param, *val;
>  	u64 mem_size;
>  
> -	if (!strstr(args, "memmap=") && !strstr(args, "mem="))
> +	if (!strstr(args, "memmap=") && !strstr(args, "mem=") &&
> +	    !strstr(args, "kaslr_mem="))
>  		return 0;
>  
>  	tmp_cmdline = malloc(len + 1);
> @@ -239,6 +304,8 @@ static int handle_mem_memmap(void)
>  
>  		if (!strcmp(param, "memmap")) {
>  			mem_avoid_memmap(val);
> +		} else if (!strcmp(param, "kaslr_mem")) {
> +			parse_kaslr_mem_regions(val);
>  		} else if (!strcmp(param, "mem")) {
>  			char *p = val;
>  
> @@ -378,7 +445,7 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
>  	/* We don't need to set a mapping for setup_data. */
>  
>  	/* Mark the memmap regions we need to avoid */
> -	handle_mem_memmap();
> +	handle_mem_filter();
>  
>  #ifdef CONFIG_X86_VERBOSE_BOOTUP
>  	/* Make sure video RAM can be used. */
> -- 
> 2.14.3
> 
> 
> 

Re: [PATCH v7 2/5] x86/KASLR: Handle the memory regions specified in kaslr_mem

[Baoquan He]

On 01/17/18 at 06:53pm, Chao Fan wrote:
> If no 'kaslr_mem=' specified, just handle the e820/efi entries directly
> as before. Otherwise, limit kernel to memory regions specified in
> 'kaslr_mem=' commandline.
> 
> Rename process_mem_region to slots_count to match
> slots_fetch_random, and name new function as process_mem_region.
> 
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  arch/x86/boot/compressed/kaslr.c | 64 +++++++++++++++++++++++++++++++++-------
>  1 file changed, 53 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index b21741135673..b200a7ceafc1 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -548,9 +548,9 @@ static unsigned long slots_fetch_random(void)
>  	return 0;
>  }

Looks good, ack.

Acked-by: Baoquan He <bhe@redhat.com>

>  
> -static void process_mem_region(struct mem_vector *entry,
> -			       unsigned long minimum,
> -			       unsigned long image_size)
> +static void slots_count(struct mem_vector *entry,
> +			unsigned long minimum,
> +			unsigned long image_size)
>  {
>  	struct mem_vector region, overlap;
>  	struct slot_area slot_area;
> @@ -627,6 +627,52 @@ static void process_mem_region(struct mem_vector *entry,
>  	}
>  }
>  
> +static bool process_mem_region(struct mem_vector region,
> +			       unsigned long long minimum,
> +			       unsigned long long image_size)
> +{
> +	/*
> +	 * If kaslr_mem= specified, walk all the regions, and
> +	 * filter the intersection to slots_count.
> +	 */
> +	if (num_usable_region > 0) {
> +		int i;
> +
> +		for (i = 0; i < num_usable_region; i++) {
> +			struct mem_vector entry;
> +			unsigned long long start, end, entry_end, region_end;
> +
> +			start = mem_usable[i].start;
> +			end = start + mem_usable[i].size;
> +			region_end = region.start + region.size;
> +
> +			entry.start = clamp(region.start, start, end);
> +			entry_end = clamp(region_end, start, end);
> +
> +			if (entry.start < entry_end) {
> +				entry.size = entry_end - entry.start;
> +				slots_count(&entry, minimum, image_size);
> +			}
> +
> +			if (slot_area_index == MAX_SLOT_AREA) {
> +				debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
> +				return 1;
> +			}
> +		}
> +		return 0;
> +	}
> +
> +	/*
> +	 * If no kaslr_mem stored, use region directly
> +	 */
> +	slots_count(&region, minimum, image_size);
> +	if (slot_area_index == MAX_SLOT_AREA) {
> +		debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
> +		return 1;
> +	}
> +	return 0;
> +}
> +
>  #ifdef CONFIG_EFI
>  /*
>   * Returns true if mirror region found (and must have been processed
> @@ -692,11 +738,9 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
>  
>  		region.start = md->phys_addr;
>  		region.size = md->num_pages << EFI_PAGE_SHIFT;
> -		process_mem_region(&region, minimum, image_size);
> -		if (slot_area_index == MAX_SLOT_AREA) {
> -			debug_putstr("Aborted EFI scan (slot_areas full)!\n");
> +
> +		if (process_mem_region(region, minimum, image_size))
>  			break;
> -		}
>  	}
>  	return true;
>  }
> @@ -723,11 +767,9 @@ static void process_e820_entries(unsigned long minimum,
>  			continue;
>  		region.start = entry->addr;
>  		region.size = entry->size;
> -		process_mem_region(&region, minimum, image_size);
> -		if (slot_area_index == MAX_SLOT_AREA) {
> -			debug_putstr("Aborted e820 scan (slot_areas full)!\n");
> +
> +		if (process_mem_region(region, minimum, image_size))
>  			break;
> -		}
>  	}
>  }
>  
> -- 
> 2.14.3
> 
> 
> 

Re: [PATCH v7 5/5] document: add document for kaslr_mem

[Baoquan He]

On 01/17/18 at 06:53pm, Chao Fan wrote:
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index e2de7c006a74..f6d5adde1a73 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2350,6 +2350,16 @@
>  			allocations which rules out almost all kernel
>  			allocations. Use with caution!
>  
> +	kaslr_mem=nn[KMG][@ss[KMG]]
> +			[KNL] Force usage of a specific region of memory.
			[KNL] Force usage of a specific region of memory
			for KASLR during kernel decompression stage.
			Region of memory to be used is from ss to ss+nn.
			If ss is omitted, it is equivalent to kaslr_mem=nn[KMG]@0.
			Multiple regions can be specified, comma delimited.
			Notice: only support 4 regions at most now.
			Example:
			kaslr_mem=1G,500M@2G,1G@4G

Try to rewrite the doc, just for reference.

> +			Make some features, like memory hotplug and 1G huge
> +			page work well with KASLR. Region of usable memory is
> +			from ss to ss+nn. If ss is omitted, it defaults to 0.
> +			Multiple regions can be specified, comma delimited.
> +			Notice: we support 4 regions at most now.
> +			Example:
> +			kaslr_mem=1G,500M@2G,1G@4G
> +
>  	MTD_Partition=	[MTD]
>  			Format: <name>,<region-number>,<size>,<offset>
>  
> -- 
> 2.14.3
> 
> 
> 

[PATCH v8 3/5] x86/KASLR: Give a warning if movable_node specified without kaslr_mem=

[Chao Fan]

Since only 'movable_node' specified without 'kaslr_mem=' may break
memory hotplug, so reconmmend users using 'kaslr_mem=' when
'movable_node' specified.

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
---
 arch/x86/boot/compressed/kaslr.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index b200a7ceafc1..8703cc764306 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -282,6 +282,16 @@ static int handle_mem_filter(void)
 	    !strstr(args, "kaslr_mem="))
 		return 0;
 
+#ifdef CONFIG_MEMORY_HOTPLUG
+	/*
+	 * Check if 'kaslr_mem=' specified when 'movable_node' found. If not,
+	 * just give warrning. Otherwise memory hotplug could be
+	 * affected if kernel is put on movable memory regions.
+	 */
+	if (strstr(args, "movable_node") && !strstr(args, "kaslr_mem="))
+		warn("'kaslr_mem=' should be specified when using 'movable_node'.\n");
+#endif
+
 	tmp_cmdline = malloc(len + 1);
 	if (!tmp_cmdline)
 		error("Failed to allocate space for tmp_cmdline");
-- 
2.14.3

[PATCH v8 4/5] x86/KASLR: Skip memory mirror handling if movable_node specified

[Chao Fan]

In kernel code, if movable_node specified, it will skip the mirror
feature. So we should also skip mirror feature in KASLR.

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
---
 arch/x86/boot/compressed/kaslr.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 8703cc764306..e4b487f0b7af 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -692,6 +692,7 @@ static bool
 process_efi_entries(unsigned long minimum, unsigned long image_size)
 {
 	struct efi_info *e = &boot_params->efi_info;
+	char *args = (char *)get_cmd_line_ptr();
 	bool efi_mirror_found = false;
 	struct mem_vector region;
 	efi_memory_desc_t *md;
@@ -725,6 +726,12 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
 		}
 	}
 
+#ifdef CONFIG_MEMORY_HOTPLUG
+	/* Skip memory mirror if 'movabale_node' specified */
+	if (strstr(args, "movable_node"))
+		efi_mirror_found = false;
+#endif
+
 	for (i = 0; i < nr_desc; i++) {
 		md = efi_early_memdesc_ptr(pmap, e->efi_memdesc_size, i);
 
-- 
2.14.3

[PATCH v8 5/5] document: add document for kaslr_mem

[Chao Fan]

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index e2de7c006a74..28a879f62560 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2350,6 +2350,16 @@
 			allocations which rules out almost all kernel
 			allocations. Use with caution!
 
+	kaslr_mem=nn[KMG][@ss[KMG]]
+			[KNL] Force usage of a specific region of memory
+			for KASLR during kernel decompression stage.
+			Region of usable memory is from ss to ss+nn. If ss
+			is omitted, it is qeuivalent to kaslr_mem=nn[KMG]@0.
+			Multiple regions can be specified, comma delimited.
+			Notice: we support 4 regions at most now.
+			Example:
+			kaslr_mem=1G,500M@2G,1G@4G
+
 	MTD_Partition=	[MTD]
 			Format: <name>,<region-number>,<size>,<offset>
 
-- 
2.14.3

Re: [PATCH v8 4/5] x86/KASLR: Skip memory mirror handling if movable_node specified

[Baoquan He]

On 01/19/18 at 11:33am, Chao Fan wrote:
> In kernel code, if movable_node specified, it will skip the mirror
> feature. So we should also skip mirror feature in KASLR.
> 
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  arch/x86/boot/compressed/kaslr.c | 7 +++++++
>  1 file changed, 7 insertions(+)

Ack.

Acked-by: Baoquan He <bhe@redhat.com>

> 
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 8703cc764306..e4b487f0b7af 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -692,6 +692,7 @@ static bool
>  process_efi_entries(unsigned long minimum, unsigned long image_size)
>  {
>  	struct efi_info *e = &boot_params->efi_info;
> +	char *args = (char *)get_cmd_line_ptr();
>  	bool efi_mirror_found = false;
>  	struct mem_vector region;
>  	efi_memory_desc_t *md;
> @@ -725,6 +726,12 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
>  		}
>  	}
>  
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +	/* Skip memory mirror if 'movabale_node' specified */
> +	if (strstr(args, "movable_node"))
> +		efi_mirror_found = false;
> +#endif
> +
>  	for (i = 0; i < nr_desc; i++) {
>  		md = efi_early_memdesc_ptr(pmap, e->efi_memdesc_size, i);
>  
> -- 
> 2.14.3
> 
> 
> 

Re: [PATCH v8 3/5] x86/KASLR: Give a warning if movable_node specified without kaslr_mem=

[Baoquan He]

On 01/19/18 at 11:31am, Chao Fan wrote:
> Since only 'movable_node' specified without 'kaslr_mem=' may break
> memory hotplug, so reconmmend users using 'kaslr_mem=' when
> 'movable_node' specified.
> 
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  arch/x86/boot/compressed/kaslr.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index b200a7ceafc1..8703cc764306 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -282,6 +282,16 @@ static int handle_mem_filter(void)
>  	    !strstr(args, "kaslr_mem="))
>  		return 0;

Looks good to me.

Acked-by: Baoquan He <bhe@redhat.com>

>  
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +	/*
> +	 * Check if 'kaslr_mem=' specified when 'movable_node' found. If not,
> +	 * just give warrning. Otherwise memory hotplug could be
> +	 * affected if kernel is put on movable memory regions.
> +	 */
> +	if (strstr(args, "movable_node") && !strstr(args, "kaslr_mem="))
> +		warn("'kaslr_mem=' should be specified when using 'movable_node'.\n");
> +#endif
> +
>  	tmp_cmdline = malloc(len + 1);
>  	if (!tmp_cmdline)
>  		error("Failed to allocate space for tmp_cmdline");
> -- 
> 2.14.3
> 
> 
> 

Re: [PATCH v8 5/5] document: add document for kaslr_mem

[Baoquan He]

On 01/19/18 at 11:36am, Chao Fan wrote:
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index e2de7c006a74..28a879f62560 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2350,6 +2350,16 @@
>  			allocations which rules out almost all kernel
>  			allocations. Use with caution!
>  
> +	kaslr_mem=nn[KMG][@ss[KMG]]
> +			[KNL] Force usage of a specific region of memory
> +			for KASLR during kernel decompression stage.
> +			Region of usable memory is from ss to ss+nn. If ss
> +			is omitted, it is qeuivalent to kaslr_mem=nn[KMG]@0.
> +			Multiple regions can be specified, comma delimited.
> +			Notice: we support 4 regions at most now.

Better not use 'we' here. You can refer to kernel-parameter.txt.

> +			Example:
> +			kaslr_mem=1G,500M@2G,1G@4G
> +
>  	MTD_Partition=	[MTD]
>  			Format: <name>,<region-number>,<size>,<offset>
>  
> -- 
> 2.14.3
> 
> 
> 

[RESEND PATCH v8 5/5] document: add document for kaslr_mem

[Chao Fan]

Cc: linux-doc@vger.kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index e2de7c006a74..2e3d5fb13f7f 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2350,6 +2350,16 @@
 			allocations which rules out almost all kernel
 			allocations. Use with caution!
 
+	kaslr_mem=nn[KMG][@ss[KMG]]
+			[KNL] Force usage of a specific region of memory
+			for KASLR during kernel decompression stage.
+			Region of usable memory is from ss to ss+nn. If ss
+			is omitted, it is qeuivalent to kaslr_mem=nn[KMG]@0.
+			Multiple regions can be specified, comma delimited.
+			Notice: only support 4 regions at most now.
+			Example:
+			kaslr_mem=1G,500M@2G,1G@4G
+
 	MTD_Partition=	[MTD]
 			Format: <name>,<region-number>,<size>,<offset>
 
-- 
2.14.3

Re: [PATCH v8 5/5] document: add document for kaslr_mem

[Chao Fan]

On Fri, Jan 19, 2018 at 11:53:31AM +0800, Baoquan He wrote:
>On 01/19/18 at 11:36am, Chao Fan wrote:
>> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
>> ---
>>  Documentation/admin-guide/kernel-parameters.txt | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>> 
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index e2de7c006a74..28a879f62560 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -2350,6 +2350,16 @@
>>  			allocations which rules out almost all kernel
>>  			allocations. Use with caution!
>>  
>> +	kaslr_mem=nn[KMG][@ss[KMG]]
>> +			[KNL] Force usage of a specific region of memory
>> +			for KASLR during kernel decompression stage.
>> +			Region of usable memory is from ss to ss+nn. If ss
>> +			is omitted, it is qeuivalent to kaslr_mem=nn[KMG]@0.
>> +			Multiple regions can be specified, comma delimited.
>> +			Notice: we support 4 regions at most now.
>
>Better not use 'we' here. You can refer to kernel-parameter.txt.

You are right, so I resend this part, and add several Cc.

Thanks,
Chao Fan
>
>> +			Example:
>> +			kaslr_mem=1G,500M@2G,1G@4G
>> +
>>  	MTD_Partition=	[MTD]
>>  			Format: <name>,<region-number>,<size>,<offset>
>>  
>> -- 
>> 2.14.3
>> 
>> 
>> 
>
>

Re: [RESEND PATCH v8 5/5] document: add document for kaslr_mem

[Randy Dunlap]

On 01/18/2018 09:23 PM, Chao Fan wrote:
> Cc: linux-doc@vger.kernel.org
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index e2de7c006a74..2e3d5fb13f7f 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2350,6 +2350,16 @@
>  			allocations which rules out almost all kernel
>  			allocations. Use with caution!
>  
> +	kaslr_mem=nn[KMG][@ss[KMG]]
> +			[KNL] Force usage of a specific region of memory
> +			for KASLR during kernel decompression stage.
> +			Region of usable memory is from ss to ss+nn. If ss
> +			is omitted, it is qeuivalent to kaslr_mem=nn[KMG]@0.

			                  equivalent

> +			Multiple regions can be specified, comma delimited.
> +			Notice: only support 4 regions at most now.
> +			Example:
> +			kaslr_mem=1G,500M@2G,1G@4G
> +
>  	MTD_Partition=	[MTD]
>  			Format: <name>,<region-number>,<size>,<offset>
>  
> 


-- 
~Randy