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

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

[Chao Fan]

This is v8 resend. There's no code change. Just improve code comments
and document accordingly. So add Baoquan's Acked-by and Luiz's
Tested-by.

***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 tested this series with a 4GB KVM guest. With kaslr_mem=1G,
   got one 1GB page allocated 100% of the time in 85 boots. Without
   kaslr_mem=, got 3 failures in only 10 boots (that is, in 3 boots
   no 1GB page allocated). So this series solves the 1GB page problem.

***History
v7->v8:
 - Just improve some comments.
 - Change the wrong spelling.
 - Add the Tested-by and Acked-by.

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 v8 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.

Tested-by: Luiz Capitulino <lcapitulino@redhat.com>
Acked-by: Baoquan He <bhe@redhat.com>
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 v8 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.

Tested-by: Luiz Capitulino <lcapitulino@redhat.com>
Acked-by: Baoquan He <bhe@redhat.com>
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 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.

Acked-by: Baoquan He <bhe@redhat.com>
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 a 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 also skip mirror feature in KASLR.

Acked-by: Baoquan He <bhe@redhat.com>
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]

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..e6de15715c4c 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 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
+
 	MTD_Partition=	[MTD]
 			Format: <name>,<region-number>,<size>,<offset>
 
-- 
2.14.3

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

[Chao Fan]

Hi Kees,

Since no reply for one week, any suggestions for this version?

Thanks,
Chao Fan

On Tue, Jan 23, 2018 at 04:35:41PM +0800, Chao Fan wrote:
>This is v8 resend. There's no code change. Just improve code comments
>and document accordingly. So add Baoquan's Acked-by and Luiz's
>Tested-by.
>
>***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 tested this series with a 4GB KVM guest. With kaslr_mem=1G,
>   got one 1GB page allocated 100% of the time in 85 boots. Without
>   kaslr_mem=, got 3 failures in only 10 boots (that is, in 3 boots
>   no 1GB page allocated). So this series solves the 1GB page problem.
>
>***History
>v7->v8:
> - Just improve some comments.
> - Change the wrong spelling.
> - Add the Tested-by and Acked-by.
>
>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
>