[PATCH v4 0/4] aslr: add parameter immovable_mem=nn[KMG]@ss[KMG] to make memory hotplug work well with kaslr

[PATCH v4 0/4] aslr: add parameter immovable_mem=nn[KMG]@ss[KMG] to make memory hotplug work well with kaslr

[Chao Fan]

Here is a problem:
Here is a machine with several NUMA nodes and some of them are
hot-pluggable. It's not good for kernel to be extracted in the memory
region of movable node. But in current code, I print the address chosen by
kaslr and found it may be placed in movable node sometimes.
To solve this problem, it's better to limit the memory region chosen by
kaslr to immovable node in kaslr.c. But the memory information about if
it's hot-pluggable is stored in ACPI SRAT table, which is parsed after
kernel is extracted. So we can't get the detail memory information
before extracting kernel.

So add the new parameter immovable_mem=nn@ss, in which nn means
the size of memory in *immovable* node, and ss means the start position of
this memory region. Then limit kaslr choose memory in these regions.

There are two policies:
1. Specify the memory region in *movable* node to avoid:
   Then we can use the existing mem_avoid to handle. But if the memory
   on movable node was separated by memory hole or different movable nodes
   are discontinuous, we don't know how many regions need to avoid.
   OTOH, we must avoid all of the movable memory, otherwise, kaslr may
   choose the wrong place.
2. Specify the memory region in *immovable* node to select:
   Only support 4 regions in this parameter. Then user can use two nodes
   at least for kaslr to choose, it's enough for the kernel to extract.
   At the same time, because we need only 4 new mem_vector, the usage
   of memory here is not too big. So I think this way is better, and this
   patchset is based on this policy.

PATCH 1/4 parse the new parameter immovable_mem=nn[KMG]@ss[KMG], then
          store the memory regions.
PATCH 2/4 select the memory region in immovable node when process
          memmap.
PATCH 3/4 skip mirror feature if movable_node or immovable_mem specified.
PATCH 4/4 add document.

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.

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

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.

Chao Fan (4):
  kaslr: add immovable_mem=nn[KMG]@ss[KMG] to specify extracting memory
  kaslr: calculate the memory region in immovable node
  kaslr: disable memory mirror feature when movable_node
  document: change the document for immovable_mem

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

-- 
2.14.3

[PATCH v4 1/4] kaslr: add immovable_mem=nn[KMG]@ss[KMG] to specify extracting memory

[Chao Fan]

In current code, kaslr may choose the memory region in movable
nodes to extract kernel, which will make the nodes can't be hot-removed.
To solve it, we can specify the memory region in immovable node.
Create immovable_mem to store the regions in immovable_mem, where should
be chosen by kaslr.

Also change the "handle_mem_memmap" to "handle_mem_filter", since
it will not only handle memmap parameter now.
Since "immovable_mem=" only works with "movable_node", so "immovable_mem="
doesn't work alone. If specify "movable_node" without "immovable_mem=",
disable KASLR.

Multiple regions can be specified, comma delimited.
Considering the usage of memory, only support for 4 regions.
4 regions contains 2 nodes at least, enough for kernel to extract.

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

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 8199a6187251..40f299d8cd34 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -108,6 +108,19 @@ enum mem_avoid_index {
 
 static struct mem_vector mem_avoid[MEM_AVOID_MAX];
 
+#ifdef CONFIG_MEMORY_HOTPLUG
+/* Only supporting at most 4 immovable memory regions with kaslr */
+#define MAX_IMMOVABLE_MEM	4
+
+static bool lack_immovable_mem;
+
+/* Store the memory regions in immovable node */
+static struct mem_vector immovable_mem[MAX_IMMOVABLE_MEM];
+
+/* The immovable regions user specify, not more than 4 */
+static int num_immovable_region;
+#endif
+
 static bool mem_overlaps(struct mem_vector *one, struct mem_vector *two)
 {
 	/* Item one is entirely before item two. */
@@ -206,17 +219,99 @@ static void mem_avoid_memmap(char *str)
 		memmap_too_large = true;
 }
 
-static int handle_mem_memmap(void)
+#ifdef CONFIG_MEMORY_HOTPLUG
+static int parse_immovable_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, immovable_mem=nn[KMG]
+		 * has the same behaviour as immovable_mem=nn[KMG]@0. It means
+		 * the region starts from 0.
+		 */
+		*start = 0;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static void parse_immovable_mem_regions(char *str)
+{
+	static int i;
+
+	while (str && (i < MAX_IMMOVABLE_MEM)) {
+		int rc;
+		unsigned long long start, size;
+		char *k = strchr(str, ',');
+
+		if (k)
+			*k++ = 0;
+
+		rc = parse_immovable_mem(str, &start, &size);
+		if (rc < 0)
+			break;
+		str = k;
+
+		immovable_mem[i].start = start;
+		immovable_mem[i].size = size;
+		i++;
+	}
+	num_immovable_region = i;
+}
+#else
+static inline void parse_immovable_mem_regions(char *str)
+{
+}
+#endif
+
+static int handle_mem_filter(void)
 {
 	char *args = (char *)get_cmd_line_ptr();
 	size_t len = strlen((char *)args);
+	bool exist_movable_node = false;
 	char *tmp_cmdline;
 	char *param, *val;
 	u64 mem_size;
 
-	if (!strstr(args, "memmap=") && !strstr(args, "mem="))
+	if (!strstr(args, "memmap=") && !strstr(args, "mem=") &&
+	    !strstr(args, "movable_node"))
 		return 0;
 
+#ifdef CONFIG_MEMORY_HOTPLUG
+	if (strstr(args, "movable_node")) {
+		/*
+		 * Confirm "movable_node" specified, otherwise
+		 * "immovable_mem=" doesn't work.
+		 */
+		exist_movable_node = true;
+
+		/*
+		 * If only specify "movable_node" without "immovable_mem=",
+		 * disable KASLR.
+		 */
+		if (!strstr(args, "immovable_mem=")) {
+			lack_immovable_mem = true;
+			return 0;
+		}
+	}
+#endif
+
 	tmp_cmdline = malloc(len + 1);
 	if (!tmp_cmdline)
 		error("Failed to allocate space for tmp_cmdline");
@@ -239,6 +334,9 @@ static int handle_mem_memmap(void)
 
 		if (!strcmp(param, "memmap")) {
 			mem_avoid_memmap(val);
+		} else if (!strcmp(param, "immovable_mem=") &&
+			   exist_movable_node) {
+			parse_immovable_mem_regions(val);
 		} else if (!strcmp(param, "mem")) {
 			char *p = val;
 
@@ -378,7 +476,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. */
@@ -673,6 +771,14 @@ static unsigned long find_random_phys_addr(unsigned long minimum,
 		return 0;
 	}
 
+#ifdef CONFIG_MEMORY_HOTPLUG
+	/* Check if specify "movable_node" without "immovable_mem=". */
+	if (lack_immovable_mem) {
+		debug_putstr("Fail KASLR when movable_node specified without immovable_mem=.\n");
+		return 0;
+	}
+#endif
+
 	/* Make sure minimum is aligned. */
 	minimum = ALIGN(minimum, CONFIG_PHYSICAL_ALIGN);
 
-- 
2.14.3

[PATCH v4 2/4] kaslr: calculate the memory region in immovable node

[Chao Fan]

kaslr: calculate the memory region in immovable node

If there is no immovable memory region specified, use region directely.
There are several conditons:
1. CONFIG_MEMORY_HOTPLUG is not specified to y.
2. immovable_mem= is not specified.

Otherwise, calculate the intersecting between memmap entry and
immovable memory.

Rename process_mem_region to slots_count to match
slots_fetch_random, and rename new function sa process_mem_region.

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

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 40f299d8cd34..fe49d6e79c50 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -579,9 +579,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;
@@ -658,6 +658,53 @@ 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)
+{
+#ifdef CONFIG_MEMORY_HOTPLUG
+	/*
+	 * If immovable_mem= specified, walk all immovable regions, and
+	 * filter the intersection to slots_count.
+	 */
+	if (num_immovable_region > 0) {
+		for (int i = 0; i < num_immovable_region; i++) {
+			struct mem_vector entry;
+			unsigned long long start, end, entry_end, region_end;
+
+			start = immovable_mem[i].start;
+			end = start + immovable_mem[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 memmap scan (slot_areas full)!\n");
+				return 1;
+			}
+		}
+		return 0;
+	}
+#endif
+
+	/*
+	 * If no immovable_mem stored, or CONFIG_MEMORY_HOTPLUG not specified,
+	 * use region directly
+	 */
+	slots_count(&region, minimum, image_size);
+	if (slot_area_index == MAX_SLOT_AREA) {
+		debug_putstr("Aborted memmap scan (slot_areas full)!\n");
+		return 1;
+	}
+	return 0;
+}
+
 #ifdef CONFIG_EFI
 /*
  * Returns true if mirror region found (and must have been processed
@@ -723,11 +770,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;
 }
@@ -754,11 +799,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 v4 3/4] kaslr: disable memory mirror feature when movable_node

[Chao Fan]

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

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>
---
 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 fe49d6e79c50..203bec3d400d 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -714,6 +714,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;
@@ -747,6 +748,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 v4 4/4] document: change the document for immovable_mem

[Chao Fan]

Add the document for the change of new parameter
immovable_mem=nn[KMG][@ss[KMG]].

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 b5b527227afe..1d8aea01e2f6 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2352,6 +2352,16 @@
 			allocations which rules out almost all kernel
 			allocations. Use with caution!
 
+	immovable_mem=nn[KMG][@ss[KMG]]
+			[KNL] Force usage of a specific region of memory.
+			Make memory hotplug work well with KASLR.
+			Region of memory in immovable node 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:
+			immovable_mem=1G,500M@2G,1G@4G
+
 	MTD_Partition=	[MTD]
 			Format: <name>,<region-number>,<size>,<offset>
 
-- 
2.14.3

Re: [PATCH v4 1/4] kaslr: add immovable_mem=nn[KMG]@ss[KMG] to specify extracting memory

[Chao Fan]

On Tue, Dec 12, 2017 at 08:07:02PM +0800, Chao Fan wrote:
>In current code, kaslr may choose the memory region in movable
>nodes to extract kernel, which will make the nodes can't be hot-removed.
>To solve it, we can specify the memory region in immovable node.
>Create immovable_mem to store the regions in immovable_mem, where should
>be chosen by kaslr.
>
>Also change the "handle_mem_memmap" to "handle_mem_filter", since
>it will not only handle memmap parameter now.
>Since "immovable_mem=" only works with "movable_node", so "immovable_mem="
>doesn't work alone. If specify "movable_node" without "immovable_mem=",
>disable KASLR.
>
>Multiple regions can be specified, comma delimited.
>Considering the usage of memory, only support for 4 regions.
>4 regions contains 2 nodes at least, enough for kernel to extract.
>
>Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
>---
> arch/x86/boot/compressed/kaslr.c | 112 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 109 insertions(+), 3 deletions(-)
>
>diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
>index 8199a6187251..40f299d8cd34 100644
>--- a/arch/x86/boot/compressed/kaslr.c
>+++ b/arch/x86/boot/compressed/kaslr.c
>@@ -108,6 +108,19 @@ enum mem_avoid_index {
> 
> static struct mem_vector mem_avoid[MEM_AVOID_MAX];
> 
>+#ifdef CONFIG_MEMORY_HOTPLUG
>+/* Only supporting at most 4 immovable memory regions with kaslr */
>+#define MAX_IMMOVABLE_MEM	4
>+
>+static bool lack_immovable_mem;
>+
>+/* Store the memory regions in immovable node */
>+static struct mem_vector immovable_mem[MAX_IMMOVABLE_MEM];
>+
>+/* The immovable regions user specify, not more than 4 */
>+static int num_immovable_region;
>+#endif
>+

Hi Kees,

I put all the functions and variables about immovable_mem to the #ifdefs
and make some adjustments according to this change.

I try to reuse some codes, but found that '!' '@' '#' '$' work in
memmap, but can't work in "immovable_mem=", then the reuse will make
codes look strange.

So how do you think about this version.

Thanks,
Chao Fan

> static bool mem_overlaps(struct mem_vector *one, struct mem_vector *two)
> {
> 	/* Item one is entirely before item two. */
>@@ -206,17 +219,99 @@ static void mem_avoid_memmap(char *str)
> 		memmap_too_large = true;
> }
> 
>-static int handle_mem_memmap(void)
>+#ifdef CONFIG_MEMORY_HOTPLUG
>+static int parse_immovable_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, immovable_mem=nn[KMG]
>+		 * has the same behaviour as immovable_mem=nn[KMG]@0. It means
>+		 * the region starts from 0.
>+		 */
>+		*start = 0;
>+		return 0;
>+	}
>+
>+	return -EINVAL;
>+}
>+
>+static void parse_immovable_mem_regions(char *str)
>+{
>+	static int i;
>+
>+	while (str && (i < MAX_IMMOVABLE_MEM)) {
>+		int rc;
>+		unsigned long long start, size;
>+		char *k = strchr(str, ',');
>+
>+		if (k)
>+			*k++ = 0;
>+
>+		rc = parse_immovable_mem(str, &start, &size);
>+		if (rc < 0)
>+			break;
>+		str = k;
>+
>+		immovable_mem[i].start = start;
>+		immovable_mem[i].size = size;
>+		i++;
>+	}
>+	num_immovable_region = i;
>+}
>+#else
>+static inline void parse_immovable_mem_regions(char *str)
>+{
>+}
>+#endif
>+
>+static int handle_mem_filter(void)
> {
> 	char *args = (char *)get_cmd_line_ptr();
> 	size_t len = strlen((char *)args);
>+	bool exist_movable_node = false;
> 	char *tmp_cmdline;
> 	char *param, *val;
> 	u64 mem_size;
> 
>-	if (!strstr(args, "memmap=") && !strstr(args, "mem="))
>+	if (!strstr(args, "memmap=") && !strstr(args, "mem=") &&
>+	    !strstr(args, "movable_node"))
> 		return 0;
> 
>+#ifdef CONFIG_MEMORY_HOTPLUG
>+	if (strstr(args, "movable_node")) {
>+		/*
>+		 * Confirm "movable_node" specified, otherwise
>+		 * "immovable_mem=" doesn't work.
>+		 */
>+		exist_movable_node = true;
>+
>+		/*
>+		 * If only specify "movable_node" without "immovable_mem=",
>+		 * disable KASLR.
>+		 */
>+		if (!strstr(args, "immovable_mem=")) {
>+			lack_immovable_mem = true;
>+			return 0;
>+		}
>+	}
>+#endif
>+
> 	tmp_cmdline = malloc(len + 1);
> 	if (!tmp_cmdline)
> 		error("Failed to allocate space for tmp_cmdline");
>@@ -239,6 +334,9 @@ static int handle_mem_memmap(void)
> 
> 		if (!strcmp(param, "memmap")) {
> 			mem_avoid_memmap(val);
>+		} else if (!strcmp(param, "immovable_mem=") &&
>+			   exist_movable_node) {
>+			parse_immovable_mem_regions(val);
> 		} else if (!strcmp(param, "mem")) {
> 			char *p = val;
> 
>@@ -378,7 +476,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. */
>@@ -673,6 +771,14 @@ static unsigned long find_random_phys_addr(unsigned long minimum,
> 		return 0;
> 	}
> 
>+#ifdef CONFIG_MEMORY_HOTPLUG
>+	/* Check if specify "movable_node" without "immovable_mem=". */
>+	if (lack_immovable_mem) {
>+		debug_putstr("Fail KASLR when movable_node specified without immovable_mem=.\n");
>+		return 0;
>+	}
>+#endif
>+
> 	/* Make sure minimum is aligned. */
> 	minimum = ALIGN(minimum, CONFIG_PHYSICAL_ALIGN);
> 
>-- 
>2.14.3
>

Re: [PATCH v4 2/4] kaslr: calculate the memory region in immovable node

[Chao Fan]

On Tue, Dec 12, 2017 at 08:07:03PM +0800, Chao Fan wrote:
>kaslr: calculate the memory region in immovable node
>
>If there is no immovable memory region specified, use region directely.
>There are several conditons:
>1. CONFIG_MEMORY_HOTPLUG is not specified to y.
>2. immovable_mem= is not specified.
>
>Otherwise, calculate the intersecting between memmap entry and
>immovable memory.
>
>Rename process_mem_region to slots_count to match
>slots_fetch_random, and rename new function sa process_mem_region.
>
>Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
>---
> arch/x86/boot/compressed/kaslr.c | 65 +++++++++++++++++++++++++++++++++-------
> 1 file changed, 54 insertions(+), 11 deletions(-)
>
>diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
>index 40f299d8cd34..fe49d6e79c50 100644
>--- a/arch/x86/boot/compressed/kaslr.c
>+++ b/arch/x86/boot/compressed/kaslr.c
>@@ -579,9 +579,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;
>@@ -658,6 +658,53 @@ 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)
>+{

Hi Kees and Baoquan,

I follow Kees's suggestion, put num_immovable_region into #ifdef
CONFIG_MEMORY_HOTPLUG, since there won't be num_immovable_region if
not in #ifdef ..., so I changh the order of these two situations
of Baoquan's version.

Thanks,
Chao Fan

>+#ifdef CONFIG_MEMORY_HOTPLUG
>+	/*
>+	 * If immovable_mem= specified, walk all immovable regions, and
>+	 * filter the intersection to slots_count.
>+	 */
>+	if (num_immovable_region > 0) {
>+		for (int i = 0; i < num_immovable_region; i++) {
>+			struct mem_vector entry;
>+			unsigned long long start, end, entry_end, region_end;
>+
>+			start = immovable_mem[i].start;
>+			end = start + immovable_mem[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 memmap scan (slot_areas full)!\n");
>+				return 1;
>+			}
>+		}
>+		return 0;
>+	}
>+#endif
>+
>+	/*
>+	 * If no immovable_mem stored, or CONFIG_MEMORY_HOTPLUG not specified,
>+	 * use region directly
>+	 */
>+	slots_count(&region, minimum, image_size);
>+	if (slot_area_index == MAX_SLOT_AREA) {
>+		debug_putstr("Aborted memmap scan (slot_areas full)!\n");
>+		return 1;
>+	}
>+	return 0;
>+}
>+
> #ifdef CONFIG_EFI
> /*
>  * Returns true if mirror region found (and must have been processed
>@@ -723,11 +770,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;
> }
>@@ -754,11 +799,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 v4 1/4] kaslr: add immovable_mem=nn[KMG]@ss[KMG] to specify extracting memory

[Chao Fan]

On Tue, Dec 12, 2017 at 08:07:02PM +0800, Chao Fan wrote:
>In current code, kaslr may choose the memory region in movable
>nodes to extract kernel, which will make the nodes can't be hot-removed.
>To solve it, we can specify the memory region in immovable node.
>Create immovable_mem to store the regions in immovable_mem, where should
>be chosen by kaslr.
>
>Also change the "handle_mem_memmap" to "handle_mem_filter", since
>it will not only handle memmap parameter now.
>Since "immovable_mem=" only works with "movable_node", so "immovable_mem="
>doesn't work alone. If specify "movable_node" without "immovable_mem=",
>disable KASLR.
>
>Multiple regions can be specified, comma delimited.
>Considering the usage of memory, only support for 4 regions.
>4 regions contains 2 nodes at least, enough for kernel to extract.
>

I tried reuse codes like this:
@@ -129,7 +142,7 @@ char *skip_spaces(const char *str)
 #include "../../../../lib/cmdline.c"

 static int
-parse_memmap(char *p, unsigned long long *start, unsigned long long *size)
+parse_mem_filter(char *p, unsigned long long *start, unsigned long long *size)
 {
        char *oldp;

@@ -149,6 +162,9 @@ parse_memmap(char *p, unsigned long long *start, unsigned long long *size)
        case '#':
        case '$':
        case '!':
+#ifdef CONFIG_MEMORY_HOTPLUG
+       case '%':
+#endif
                *start = memparse(p + 1, &p);
                return 0;
        case '@':
@@ -183,7 +199,7 @@ static void mem_avoid_memmap(char *str)
                if (k)
                        *k++ = 0;

-               rc = parse_memmap(str, &start, &size);
+               rc = parse_mem_filter(str, &start, &size);
                if (rc < 0)
                        break;
                str = k;
@@ -206,17 +222,68 @@ static void mem_avoid_memmap(char *str)
                memmap_too_large = true;
 }

-static int handle_mem_memmap(void)
+#ifdef CONFIG_MEMORY_HOTPLUG
+static void parse_immovable_mem_regions(char *str)
+{
+       static int i;
+
+       while (str && (i < MAX_IMMOVABLE_MEM)) {
+               int rc;
+               unsigned long long start, size;
+               char *k = strchr(str, ',');
+
+               if (k)
+                       *k++ = 0;
+
+               rc = parse_mem_filter(str, &start, &size);
+               if (rc < 0)
+                       break;
+               str = k;
+
+               immovable_mem[i].start = start;
+               immovable_mem[i].size = size;
+               i++;
+       }
+       num_immovable_region = i;
+}
+#else
+static inline void parse_immovable_mem_regions(char *str)
+{
+}
+#endif

So that we can reuse and rename parse_memmap(), then use '%' for
"immovable_mem=", but I found in this version, '!' '@' '#' '$'
will also work well as '%'. I didn't find a good method to avoid
it, so I gave up the reuse.

Thanks,
Chao Fan

>Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
>---
> arch/x86/boot/compressed/kaslr.c | 112 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 109 insertions(+), 3 deletions(-)
>
>diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
>index 8199a6187251..40f299d8cd34 100644
>--- a/arch/x86/boot/compressed/kaslr.c
>+++ b/arch/x86/boot/compressed/kaslr.c
>@@ -108,6 +108,19 @@ enum mem_avoid_index {
> 
> static struct mem_vector mem_avoid[MEM_AVOID_MAX];
> 
>+#ifdef CONFIG_MEMORY_HOTPLUG
>+/* Only supporting at most 4 immovable memory regions with kaslr */
>+#define MAX_IMMOVABLE_MEM	4
>+
>+static bool lack_immovable_mem;
>+
>+/* Store the memory regions in immovable node */
>+static struct mem_vector immovable_mem[MAX_IMMOVABLE_MEM];
>+
>+/* The immovable regions user specify, not more than 4 */
>+static int num_immovable_region;
>+#endif
>+
> static bool mem_overlaps(struct mem_vector *one, struct mem_vector *two)
> {
> 	/* Item one is entirely before item two. */
>@@ -206,17 +219,99 @@ static void mem_avoid_memmap(char *str)
> 		memmap_too_large = true;
> }
> 
>-static int handle_mem_memmap(void)
>+#ifdef CONFIG_MEMORY_HOTPLUG
>+static int parse_immovable_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, immovable_mem=nn[KMG]
>+		 * has the same behaviour as immovable_mem=nn[KMG]@0. It means
>+		 * the region starts from 0.
>+		 */
>+		*start = 0;
>+		return 0;
>+	}
>+
>+	return -EINVAL;
>+}
>+
>+static void parse_immovable_mem_regions(char *str)
>+{
>+	static int i;
>+
>+	while (str && (i < MAX_IMMOVABLE_MEM)) {
>+		int rc;
>+		unsigned long long start, size;
>+		char *k = strchr(str, ',');
>+
>+		if (k)
>+			*k++ = 0;
>+
>+		rc = parse_immovable_mem(str, &start, &size);
>+		if (rc < 0)
>+			break;
>+		str = k;
>+
>+		immovable_mem[i].start = start;
>+		immovable_mem[i].size = size;
>+		i++;
>+	}
>+	num_immovable_region = i;
>+}
>+#else
>+static inline void parse_immovable_mem_regions(char *str)
>+{
>+}
>+#endif
>+
>+static int handle_mem_filter(void)
> {
> 	char *args = (char *)get_cmd_line_ptr();
> 	size_t len = strlen((char *)args);
>+	bool exist_movable_node = false;
> 	char *tmp_cmdline;
> 	char *param, *val;
> 	u64 mem_size;
> 
>-	if (!strstr(args, "memmap=") && !strstr(args, "mem="))
>+	if (!strstr(args, "memmap=") && !strstr(args, "mem=") &&
>+	    !strstr(args, "movable_node"))
> 		return 0;
> 
>+#ifdef CONFIG_MEMORY_HOTPLUG
>+	if (strstr(args, "movable_node")) {
>+		/*
>+		 * Confirm "movable_node" specified, otherwise
>+		 * "immovable_mem=" doesn't work.
>+		 */
>+		exist_movable_node = true;
>+
>+		/*
>+		 * If only specify "movable_node" without "immovable_mem=",
>+		 * disable KASLR.
>+		 */
>+		if (!strstr(args, "immovable_mem=")) {
>+			lack_immovable_mem = true;
>+			return 0;
>+		}
>+	}
>+#endif
>+
> 	tmp_cmdline = malloc(len + 1);
> 	if (!tmp_cmdline)
> 		error("Failed to allocate space for tmp_cmdline");
>@@ -239,6 +334,9 @@ static int handle_mem_memmap(void)
> 
> 		if (!strcmp(param, "memmap")) {
> 			mem_avoid_memmap(val);
>+		} else if (!strcmp(param, "immovable_mem=") &&
>+			   exist_movable_node) {
>+			parse_immovable_mem_regions(val);
> 		} else if (!strcmp(param, "mem")) {
> 			char *p = val;
> 
>@@ -378,7 +476,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. */
>@@ -673,6 +771,14 @@ static unsigned long find_random_phys_addr(unsigned long minimum,
> 		return 0;
> 	}
> 
>+#ifdef CONFIG_MEMORY_HOTPLUG
>+	/* Check if specify "movable_node" without "immovable_mem=". */
>+	if (lack_immovable_mem) {
>+		debug_putstr("Fail KASLR when movable_node specified without immovable_mem=.\n");
>+		return 0;
>+	}
>+#endif
>+
> 	/* Make sure minimum is aligned. */
> 	minimum = ALIGN(minimum, CONFIG_PHYSICAL_ALIGN);
> 
>-- 
>2.14.3
>

Re: [PATCH v4 2/4] kaslr: calculate the memory region in immovable node

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2839 bytes --]

Hi Chao,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.15-rc3 next-20171215]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chao-Fan/aslr-add-parameter-immovable_mem-nn-KMG-ss-KMG-to-make-memory-hotplug-work-well-with-kaslr/20171215-175022
config: x86_64-randconfig-g0-12161605 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   arch/x86/boot/compressed/kaslr.c: In function 'process_mem_region':
>> arch/x86/boot/compressed/kaslr.c:671:3: error: 'for' loop initial declarations are only allowed in C99 or C11 mode
      for (int i = 0; i < num_immovable_region; i++) {
      ^
   arch/x86/boot/compressed/kaslr.c:671:3: note: use option -std=c99, -std=gnu99, -std=c11 or -std=gnu11 to compile your code

vim +/for +671 arch/x86/boot/compressed/kaslr.c

   660	
   661	static bool process_mem_region(struct mem_vector region,
   662				       unsigned long long minimum,
   663				       unsigned long long image_size)
   664	{
   665	#ifdef CONFIG_MEMORY_HOTPLUG
   666		/*
   667		 * If immovable_mem= specified, walk all immovable regions, and
   668		 * filter the intersection to slots_count.
   669		 */
   670		if (num_immovable_region > 0) {
 > 671			for (int i = 0; i < num_immovable_region; i++) {
   672				struct mem_vector entry;
   673				unsigned long long start, end, entry_end, region_end;
   674	
   675				start = immovable_mem[i].start;
   676				end = start + immovable_mem[i].size;
   677				region_end = region.start + region.size;
   678	
   679				entry.start = clamp(region.start, start, end);
   680				entry_end = clamp(region_end, start, end);
   681	
   682				if (entry.start < entry_end) {
   683					entry.size = entry_end - entry.start;
   684					slots_count(&entry, minimum, image_size);
   685				}
   686	
   687				if (slot_area_index == MAX_SLOT_AREA) {
   688					debug_putstr("Aborted memmap scan (slot_areas full)!\n");
   689					return 1;
   690				}
   691			}
   692			return 0;
   693		}
   694	#endif
   695	
   696		/*
   697		 * If no immovable_mem stored, or CONFIG_MEMORY_HOTPLUG not specified,
   698		 * use region directly
   699		 */
   700		slots_count(&region, minimum, image_size);
   701		if (slot_area_index == MAX_SLOT_AREA) {
   702			debug_putstr("Aborted memmap scan (slot_areas full)!\n");
   703			return 1;
   704		}
   705		return 0;
   706	}
   707	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24783 bytes --]

[PATCH v4.1 2/4] kaslr: calculate the memory region in immovable node

[Chao Fan]

If there is no immovable memory region specified, use region directely.
There are several conditons:
1. CONFIG_MEMORY_HOTPLUG is not specified to y.
2. immovable_mem= is not specified.

Otherwise, calculate the intersecting between memmap entry and
immovable memory.

Rename process_mem_region to slots_count to match
slots_fetch_random, and rename new function sa process_mem_region.

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
---
v4->v4.1:
 - Move the define "int i" out of for() loop.
---
 arch/x86/boot/compressed/kaslr.c | 66 +++++++++++++++++++++++++++++++++-------
 1 file changed, 55 insertions(+), 11 deletions(-)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 40f299d8cd34..5b8c9288e9e9 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -579,9 +579,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;
@@ -658,6 +658,54 @@ 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)
+{
+#ifdef CONFIG_MEMORY_HOTPLUG
+	/*
+	 * If immovable_mem= specified, walk all immovable regions, and
+	 * filter the intersection to slots_count.
+	 */
+	int i;
+	if (num_immovable_region > 0) {
+		for (i = 0; i < num_immovable_region; i++) {
+			struct mem_vector entry;
+			unsigned long long start, end, entry_end, region_end;
+
+			start = immovable_mem[i].start;
+			end = start + immovable_mem[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 memmap scan (slot_areas full)!\n");
+				return 1;
+			}
+		}
+		return 0;
+	}
+#endif
+
+	/*
+	 * If no immovable_mem stored, or CONFIG_MEMORY_HOTPLUG not specified,
+	 * use region directly
+	 */
+	slots_count(&region, minimum, image_size);
+	if (slot_area_index == MAX_SLOT_AREA) {
+		debug_putstr("Aborted memmap scan (slot_areas full)!\n");
+		return 1;
+	}
+	return 0;
+}
+
 #ifdef CONFIG_EFI
 /*
  * Returns true if mirror region found (and must have been processed
@@ -723,11 +771,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;
 }
@@ -754,11 +800,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