[PATCH v12 0/5] x86/boot/KASLR: Parse ACPI table and limit KASLR to choosing immovable memory

[PATCH v12 0/5] x86/boot/KASLR: Parse ACPI table and limit KASLR to choosing immovable memory

[Chao Fan]

***Background:
People reported that KASLR may randomly choose some positions
which are located in movable memory regions. This will break memory
hotplug feature and make the movable memory chosen by KASLR can't be
removed.

***Solutions:
Get the information of memory hot-remove, then KASLR will know the
right regions. Information about memory hot-remove is in ACPI
tables, which will be parsed after start_kernel(), so that KASLR
can't get the information.

Somebody suggest to add a kernel parameter to specify the
immovable memory so that limit KASLR in these regions. Then I make
a patchset. After several versions, Ingo gave a suggestion:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1634024.html
Follow Ingo's suggestion, imitate the ACPI code to parse the ACPI
tables, so that the kaslr can get necessary memory information in
ACPI tables.
I think ACPI code is an independent part, so imitate the codes
and functions to 'compressed/' directory, so that kaslr won't
influence the initialization of ACPI.

PATCH 1/5 Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC
PATCH 2/5 Add efi_get_rsdp_addr() to find RSDP from EFI table when
          booting from EFI.
PATCH 3/5 Add bios_get_rsdp_addr() to search RSDP in memory when EFI
          table not found.
PATCH 4/5 Compute SRAT table from RSDP and walk SRAT table to store
          the immovable memory regions.
PATCH 5/5 Calculate the intersection between memory regions from e820/efi
          memory table and immovable memory regions. Limit KASLR to
          choosing these regions for randomization.

v1->v2:
 -  Simplify some code.
Follow Baoquan He's suggestion:
 - Reuse the head file of acpi code.

v2->v3:
 - Test in more conditions, so remove the 'RFC' tag.
 - Change some comments.

v3->v4:
Follow Thomas Gleixner's suggetsion:
 - Put the whole efi related function into #define CONFIG_EFI and return
   false in the other stub.

v4->v5:
Follow Dou Liyang's suggestion:
 - Add more comments about some functions based on kernel code.
 - Change some typo in comments.
 - Clean useless variable.
 - Add check for the boundary of array.
 - Add check for 'movable_node' parameter

v5->v6:
Follow Baoquan He's suggestion:
 - Change some log.
 - Add the check for acpi_rsdp
 - Change some code logical to make code clear

v6->v7:
Follow Rafael's suggestion:
 - Add more comments and patch log.
Follow test robot's suggestion:
 - Add "static" tag for function

v7-v8:
Follow Kees Cook's suggestion:
 - Use mem_overlaps() to check memory region.
 - Use #ifdef in the definition of function.

v8-v9:
Follow Boris' suggestion:
 - Change code style.
 - Splite PATCH 1/3 to more path.
 - Introduce some new function
 - Use existing function to rework some code
Follow Masayoshi's suggetion:
 - Make code more readable

v9->v10:
Follow Baoquan's suggestion:
 - Change some log
 - Merge last two patch together.

v10->v11:
Follow Boris' suggestion:
 - Link kstrtoull() instead of copying it.
 - Drop the useless wrapped function.

v11->v12:
Follow Boris' suggestion:
 - Change patch log and code comments.
 - Add 'CONFIG_EARLY_PARSE_RSDP' to make code easy to read
 - Put strtoull() to misc.c
Follow Masa's suggestion:
 - Remove the detection for 'movable_node'
 - Change the code logical about cmdline_find_option()

Any comments will be welcome.


Chao Fan (5):
  x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC
  x86/boot: Add efi_get_rsdp_addr() to find RSDP from EFI table
  x86/boot: Add bios_get_rsdp_addr() to search RSDP in memory
  x86/boot: Parse SRAT table from RSDP and store immovable memory
  x86/boot/KASLR: Limit KASLR to extracting kernel in immovable memory

 arch/x86/Kconfig                  |  10 +
 arch/x86/boot/compressed/Makefile |   2 +
 arch/x86/boot/compressed/acpitb.c | 322 ++++++++++++++++++++++++++++++
 arch/x86/boot/compressed/kaslr.c  |  79 ++++++--
 arch/x86/boot/compressed/misc.c   |   5 +
 arch/x86/boot/compressed/misc.h   |  24 +++
 lib/kstrtox.c                     |   5 +
 7 files changed, 432 insertions(+), 15 deletions(-)
 create mode 100644 arch/x86/boot/compressed/acpitb.c

-- 
2.19.1

[PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Chao Fan]

To fix the conflict between KASLR and memory-hotremove, memory
information in SRAT table is necessary.

ACPI SRAT (System/Static Resource Affinity Table) can show the details
about memory ranges, including ranges of memory provided by hot-added
memory devices. SRAT table must be introduced by RSDP pointer (Root
System Description Pointer). So RSDP should be found firstly.

When booting form KEXEC/EFI/BIOS, the methods to find RSDP pointer
are different. When booting from KEXEC, 'acpi_rsdp' may have been
added to cmdline, so parse the cmdline and find the RSDP pointer.

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
---
 arch/x86/boot/compressed/acpitb.c | 33 +++++++++++++++++++++++++++++++
 arch/x86/boot/compressed/misc.c   |  5 +++++
 arch/x86/boot/compressed/misc.h   |  4 ++++
 lib/kstrtox.c                     |  5 +++++
 4 files changed, 47 insertions(+)
 create mode 100644 arch/x86/boot/compressed/acpitb.c

diff --git a/arch/x86/boot/compressed/acpitb.c b/arch/x86/boot/compressed/acpitb.c
new file mode 100644
index 000000000000..614c45655cff
--- /dev/null
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+#define BOOT_CTYPE_H
+#include "misc.h"
+#include "error.h"
+
+#include <linux/efi.h>
+#include <asm/efi.h>
+#include <linux/numa.h>
+#include <linux/acpi.h>
+
+#define STATIC
+#include <linux/decompress/mm.h>
+
+/* Store the immovable memory regions. */
+struct mem_vector immovable_mem[MAX_NUMNODES*2];
+#endif
+
+static acpi_physical_address get_acpi_rsdp(void)
+{
+#ifdef CONFIG_KEXEC
+	unsigned long long res;
+	int len = 0;
+	char *val;
+
+	val = malloc(19);
+	len = cmdline_find_option("acpi_rsdp", val, 19);
+	if (len > 0) {
+		val[len] = 0;
+		return (acpi_physical_address)kstrtoull(val, 16, &res);
+	}
+	return 0;
+#endif
+}
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 8dd1d5ccae58..e51713fe3add 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -12,6 +12,7 @@
  * High loaded stuff by Hans Lermen & Werner Almesberger, Feb. 1996
  */
 
+#define BOOT_CTYPE_H
 #include "misc.h"
 #include "error.h"
 #include "pgtable.h"
@@ -426,3 +427,7 @@ void fortify_panic(const char *name)
 {
 	error("detected buffer overflow");
 }
+
+#ifdef BOOT_STRING
+#include "../../../../lib/kstrtox.c"
+#endif
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index a1d5918765f3..809c31effa4b 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -116,3 +116,7 @@ static inline void console_init(void)
 void set_sev_encryption_mask(void);
 
 #endif
+
+/* acpitb.c */
+#define BOOT_STRING
+extern int kstrtoull(const char *s, unsigned int base, unsigned long long *res);
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index 1006bf70bf74..a0ac1b2257b8 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -126,6 +126,9 @@ int kstrtoull(const char *s, unsigned int base, unsigned long long *res)
 }
 EXPORT_SYMBOL(kstrtoull);
 
+/* Make compressed period code be able to use kstrtoull(). */
+#ifndef BOOT_STRING
+
 /**
  * kstrtoll - convert a string to a long long
  * @s: The start of the string. The string must be null-terminated, and may also
@@ -408,3 +411,5 @@ kstrto_from_user(kstrtou16_from_user,	kstrtou16,	u16);
 kstrto_from_user(kstrtos16_from_user,	kstrtos16,	s16);
 kstrto_from_user(kstrtou8_from_user,	kstrtou8,	u8);
 kstrto_from_user(kstrtos8_from_user,	kstrtos8,	s8);
+
+#endif /* BOOT_STRING */
-- 
2.19.1

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Masayoshi Mizuma]

On Thu, Nov 29, 2018 at 04:16:27PM +0800, Chao Fan wrote:
> To fix the conflict between KASLR and memory-hotremove, memory
> information in SRAT table is necessary.
> 
> ACPI SRAT (System/Static Resource Affinity Table) can show the details
> about memory ranges, including ranges of memory provided by hot-added
> memory devices. SRAT table must be introduced by RSDP pointer (Root
> System Description Pointer). So RSDP should be found firstly.
> 
> When booting form KEXEC/EFI/BIOS, the methods to find RSDP pointer
> are different. When booting from KEXEC, 'acpi_rsdp' may have been
> added to cmdline, so parse the cmdline and find the RSDP pointer.
> 
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  arch/x86/boot/compressed/acpitb.c | 33 +++++++++++++++++++++++++++++++
>  arch/x86/boot/compressed/misc.c   |  5 +++++
>  arch/x86/boot/compressed/misc.h   |  4 ++++
>  lib/kstrtox.c                     |  5 +++++
>  4 files changed, 47 insertions(+)
>  create mode 100644 arch/x86/boot/compressed/acpitb.c
> 
> diff --git a/arch/x86/boot/compressed/acpitb.c b/arch/x86/boot/compressed/acpitb.c
> new file mode 100644
> index 000000000000..614c45655cff
> --- /dev/null
> +++ b/arch/x86/boot/compressed/acpitb.c
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define BOOT_CTYPE_H
> +#include "misc.h"
> +#include "error.h"
> +
> +#include <linux/efi.h>
> +#include <asm/efi.h>
> +#include <linux/numa.h>
> +#include <linux/acpi.h>
> +
> +#define STATIC
> +#include <linux/decompress/mm.h>
> +
> +/* Store the immovable memory regions. */
> +struct mem_vector immovable_mem[MAX_NUMNODES*2];
> +#endif
> +
> +static acpi_physical_address get_acpi_rsdp(void)
> +{
> +#ifdef CONFIG_KEXEC
> +	unsigned long long res;
> +	int len = 0;

> +	char *val;
> +
> +	val = malloc(19);
> +	len = cmdline_find_option("acpi_rsdp", val, 19);
> +	if (len > 0) {
> +		val[len] = 0;

		val[len] = '\0';

> +		return (acpi_physical_address)kstrtoull(val, 16, &res);
> +	}

I think free() is needed. Or why don't you use stack?

	char val[19];

	len = cmdline_find_option("acpi_rsdp", val, sizeof(val));
	if (len > 0) {
		val[len] = '\0';
		return (acpi_physical_address)kstrtoull(val, 16, &res);
	}

Thanks,
Masa

> +	return 0;
> +#endif
> +}
> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> index 8dd1d5ccae58..e51713fe3add 100644
> --- a/arch/x86/boot/compressed/misc.c
> +++ b/arch/x86/boot/compressed/misc.c
> @@ -12,6 +12,7 @@
>   * High loaded stuff by Hans Lermen & Werner Almesberger, Feb. 1996
>   */
>  
> +#define BOOT_CTYPE_H
>  #include "misc.h"
>  #include "error.h"
>  #include "pgtable.h"
> @@ -426,3 +427,7 @@ void fortify_panic(const char *name)
>  {
>  	error("detected buffer overflow");
>  }
> +
> +#ifdef BOOT_STRING
> +#include "../../../../lib/kstrtox.c"
> +#endif
> diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> index a1d5918765f3..809c31effa4b 100644
> --- a/arch/x86/boot/compressed/misc.h
> +++ b/arch/x86/boot/compressed/misc.h
> @@ -116,3 +116,7 @@ static inline void console_init(void)
>  void set_sev_encryption_mask(void);
>  
>  #endif
> +
> +/* acpitb.c */
> +#define BOOT_STRING
> +extern int kstrtoull(const char *s, unsigned int base, unsigned long long *res);
> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
> index 1006bf70bf74..a0ac1b2257b8 100644
> --- a/lib/kstrtox.c
> +++ b/lib/kstrtox.c
> @@ -126,6 +126,9 @@ int kstrtoull(const char *s, unsigned int base, unsigned long long *res)
>  }
>  EXPORT_SYMBOL(kstrtoull);
>  
> +/* Make compressed period code be able to use kstrtoull(). */
> +#ifndef BOOT_STRING
> +
>  /**
>   * kstrtoll - convert a string to a long long
>   * @s: The start of the string. The string must be null-terminated, and may also
> @@ -408,3 +411,5 @@ kstrto_from_user(kstrtou16_from_user,	kstrtou16,	u16);
>  kstrto_from_user(kstrtos16_from_user,	kstrtos16,	s16);
>  kstrto_from_user(kstrtou8_from_user,	kstrtou8,	u8);
>  kstrto_from_user(kstrtos8_from_user,	kstrtos8,	s8);
> +
> +#endif /* BOOT_STRING */
> -- 
> 2.19.1
> 
> 
> 

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Chao Fan]

On Thu, Nov 29, 2018 at 11:20:13AM -0500, Masayoshi Mizuma wrote:
>On Thu, Nov 29, 2018 at 04:16:27PM +0800, Chao Fan wrote:
>> To fix the conflict between KASLR and memory-hotremove, memory
>> information in SRAT table is necessary.
>> 
>> ACPI SRAT (System/Static Resource Affinity Table) can show the details
>> about memory ranges, including ranges of memory provided by hot-added
>> memory devices. SRAT table must be introduced by RSDP pointer (Root
>> System Description Pointer). So RSDP should be found firstly.
>> 
>> When booting form KEXEC/EFI/BIOS, the methods to find RSDP pointer
>> are different. When booting from KEXEC, 'acpi_rsdp' may have been
>> added to cmdline, so parse the cmdline and find the RSDP pointer.
>> 
>> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
>> ---
>>  arch/x86/boot/compressed/acpitb.c | 33 +++++++++++++++++++++++++++++++
>>  arch/x86/boot/compressed/misc.c   |  5 +++++
>>  arch/x86/boot/compressed/misc.h   |  4 ++++
>>  lib/kstrtox.c                     |  5 +++++
>>  4 files changed, 47 insertions(+)
>>  create mode 100644 arch/x86/boot/compressed/acpitb.c
>> 
>> diff --git a/arch/x86/boot/compressed/acpitb.c b/arch/x86/boot/compressed/acpitb.c
>> new file mode 100644
>> index 000000000000..614c45655cff
>> --- /dev/null
>> +++ b/arch/x86/boot/compressed/acpitb.c
>> @@ -0,0 +1,33 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#define BOOT_CTYPE_H
>> +#include "misc.h"
>> +#include "error.h"
>> +
>> +#include <linux/efi.h>
>> +#include <asm/efi.h>
>> +#include <linux/numa.h>
>> +#include <linux/acpi.h>
>> +
>> +#define STATIC
>> +#include <linux/decompress/mm.h>
>> +
>> +/* Store the immovable memory regions. */
>> +struct mem_vector immovable_mem[MAX_NUMNODES*2];
>> +#endif
>> +
>> +static acpi_physical_address get_acpi_rsdp(void)
>> +{
>> +#ifdef CONFIG_KEXEC
>> +	unsigned long long res;
>> +	int len = 0;
>
>> +	char *val;
>> +
>> +	val = malloc(19);
>> +	len = cmdline_find_option("acpi_rsdp", val, 19);
>> +	if (len > 0) {
>> +		val[len] = 0;
>
>		val[len] = '\0';
>
>> +		return (acpi_physical_address)kstrtoull(val, 16, &res);
>> +	}
>
>I think free() is needed. Or why don't you use stack?

Oh, thanks, will change it
char val[19];

Thanks,
Chao Fan
>
>	char val[19];
>
>	len = cmdline_find_option("acpi_rsdp", val, sizeof(val));
>	if (len > 0) {
>		val[len] = '\0';
>		return (acpi_physical_address)kstrtoull(val, 16, &res);
>	}
>
>Thanks,
>Masa
>
>> +	return 0;
>> +#endif
>> +}
>> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
>> index 8dd1d5ccae58..e51713fe3add 100644
>> --- a/arch/x86/boot/compressed/misc.c
>> +++ b/arch/x86/boot/compressed/misc.c
>> @@ -12,6 +12,7 @@
>>   * High loaded stuff by Hans Lermen & Werner Almesberger, Feb. 1996
>>   */
>>  
>> +#define BOOT_CTYPE_H
>>  #include "misc.h"
>>  #include "error.h"
>>  #include "pgtable.h"
>> @@ -426,3 +427,7 @@ void fortify_panic(const char *name)
>>  {
>>  	error("detected buffer overflow");
>>  }
>> +
>> +#ifdef BOOT_STRING
>> +#include "../../../../lib/kstrtox.c"
>> +#endif
>> diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
>> index a1d5918765f3..809c31effa4b 100644
>> --- a/arch/x86/boot/compressed/misc.h
>> +++ b/arch/x86/boot/compressed/misc.h
>> @@ -116,3 +116,7 @@ static inline void console_init(void)
>>  void set_sev_encryption_mask(void);
>>  
>>  #endif
>> +
>> +/* acpitb.c */
>> +#define BOOT_STRING
>> +extern int kstrtoull(const char *s, unsigned int base, unsigned long long *res);
>> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
>> index 1006bf70bf74..a0ac1b2257b8 100644
>> --- a/lib/kstrtox.c
>> +++ b/lib/kstrtox.c
>> @@ -126,6 +126,9 @@ int kstrtoull(const char *s, unsigned int base, unsigned long long *res)
>>  }
>>  EXPORT_SYMBOL(kstrtoull);
>>  
>> +/* Make compressed period code be able to use kstrtoull(). */
>> +#ifndef BOOT_STRING
>> +
>>  /**
>>   * kstrtoll - convert a string to a long long
>>   * @s: The start of the string. The string must be null-terminated, and may also
>> @@ -408,3 +411,5 @@ kstrto_from_user(kstrtou16_from_user,	kstrtou16,	u16);
>>  kstrto_from_user(kstrtos16_from_user,	kstrtos16,	s16);
>>  kstrto_from_user(kstrtou8_from_user,	kstrtou8,	u8);
>>  kstrto_from_user(kstrtos8_from_user,	kstrtos8,	s8);
>> +
>> +#endif /* BOOT_STRING */
>> -- 
>> 2.19.1
>> 
>> 
>> 
>
>

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Borislav Petkov]

On Fri, Nov 30, 2018 at 10:29:14AM +0800, Chao Fan wrote:
> Oh, thanks, will change it
> char val[19];

And make that 19 a define.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Chao Fan]

On Tue, Dec 04, 2018 at 07:34:00PM +0100, Borislav Petkov wrote:
>On Fri, Nov 30, 2018 at 10:29:14AM +0800, Chao Fan wrote:
>> Oh, thanks, will change it
>> char val[19];
>
>And make that 19 a define.

I will do that.

Thanks,
Chao Fan

>
>-- 
>Regards/Gruss,
>    Boris.
>
>Good mailing practices for 400: avoid top-posting and trim the reply.
>
>

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Masayoshi Mizuma]

On Thu, Nov 29, 2018 at 04:16:27PM +0800, Chao Fan wrote:
> To fix the conflict between KASLR and memory-hotremove, memory
> information in SRAT table is necessary.
> 
> ACPI SRAT (System/Static Resource Affinity Table) can show the details
> about memory ranges, including ranges of memory provided by hot-added
> memory devices. SRAT table must be introduced by RSDP pointer (Root
> System Description Pointer). So RSDP should be found firstly.
> 
> When booting form KEXEC/EFI/BIOS, the methods to find RSDP pointer
> are different. When booting from KEXEC, 'acpi_rsdp' may have been
> added to cmdline, so parse the cmdline and find the RSDP pointer.
> 
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  arch/x86/boot/compressed/acpitb.c | 33 +++++++++++++++++++++++++++++++
>  arch/x86/boot/compressed/misc.c   |  5 +++++
>  arch/x86/boot/compressed/misc.h   |  4 ++++
>  lib/kstrtox.c                     |  5 +++++
>  4 files changed, 47 insertions(+)
>  create mode 100644 arch/x86/boot/compressed/acpitb.c
> 
> diff --git a/arch/x86/boot/compressed/acpitb.c b/arch/x86/boot/compressed/acpitb.c
> new file mode 100644
> index 000000000000..614c45655cff
> --- /dev/null
> +++ b/arch/x86/boot/compressed/acpitb.c
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define BOOT_CTYPE_H
> +#include "misc.h"
> +#include "error.h"
> +
> +#include <linux/efi.h>
> +#include <asm/efi.h>
> +#include <linux/numa.h>
> +#include <linux/acpi.h>
> +
> +#define STATIC
> +#include <linux/decompress/mm.h>
> +
> +/* Store the immovable memory regions. */
> +struct mem_vector immovable_mem[MAX_NUMNODES*2];

> +#endif

Remove this #endif...

Thanks,
Masa

> +
> +static acpi_physical_address get_acpi_rsdp(void)
> +{
> +#ifdef CONFIG_KEXEC
> +	unsigned long long res;
> +	int len = 0;
> +	char *val;
> +
> +	val = malloc(19);
> +	len = cmdline_find_option("acpi_rsdp", val, 19);
> +	if (len > 0) {
> +		val[len] = 0;
> +		return (acpi_physical_address)kstrtoull(val, 16, &res);
> +	}
> +	return 0;
> +#endif
> +}
> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> index 8dd1d5ccae58..e51713fe3add 100644
> --- a/arch/x86/boot/compressed/misc.c
> +++ b/arch/x86/boot/compressed/misc.c
> @@ -12,6 +12,7 @@
>   * High loaded stuff by Hans Lermen & Werner Almesberger, Feb. 1996
>   */
>  
> +#define BOOT_CTYPE_H
>  #include "misc.h"
>  #include "error.h"
>  #include "pgtable.h"
> @@ -426,3 +427,7 @@ void fortify_panic(const char *name)
>  {
>  	error("detected buffer overflow");
>  }
> +
> +#ifdef BOOT_STRING
> +#include "../../../../lib/kstrtox.c"
> +#endif
> diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> index a1d5918765f3..809c31effa4b 100644
> --- a/arch/x86/boot/compressed/misc.h
> +++ b/arch/x86/boot/compressed/misc.h
> @@ -116,3 +116,7 @@ static inline void console_init(void)
>  void set_sev_encryption_mask(void);
>  
>  #endif
> +
> +/* acpitb.c */
> +#define BOOT_STRING
> +extern int kstrtoull(const char *s, unsigned int base, unsigned long long *res);
> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
> index 1006bf70bf74..a0ac1b2257b8 100644
> --- a/lib/kstrtox.c
> +++ b/lib/kstrtox.c
> @@ -126,6 +126,9 @@ int kstrtoull(const char *s, unsigned int base, unsigned long long *res)
>  }
>  EXPORT_SYMBOL(kstrtoull);
>  
> +/* Make compressed period code be able to use kstrtoull(). */
> +#ifndef BOOT_STRING
> +
>  /**
>   * kstrtoll - convert a string to a long long
>   * @s: The start of the string. The string must be null-terminated, and may also
> @@ -408,3 +411,5 @@ kstrto_from_user(kstrtou16_from_user,	kstrtou16,	u16);
>  kstrto_from_user(kstrtos16_from_user,	kstrtos16,	s16);
>  kstrto_from_user(kstrtou8_from_user,	kstrtou8,	u8);
>  kstrto_from_user(kstrtos8_from_user,	kstrtos8,	s8);
> +
> +#endif /* BOOT_STRING */
> -- 
> 2.19.1
> 
> 
> 

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Masayoshi Mizuma]

On Thu, Nov 29, 2018 at 04:16:27PM +0800, Chao Fan wrote:
> To fix the conflict between KASLR and memory-hotremove, memory
> information in SRAT table is necessary.
> 
> ACPI SRAT (System/Static Resource Affinity Table) can show the details
> about memory ranges, including ranges of memory provided by hot-added
> memory devices. SRAT table must be introduced by RSDP pointer (Root
> System Description Pointer). So RSDP should be found firstly.
> 
> When booting form KEXEC/EFI/BIOS, the methods to find RSDP pointer
> are different. When booting from KEXEC, 'acpi_rsdp' may have been
> added to cmdline, so parse the cmdline and find the RSDP pointer.
> 
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  arch/x86/boot/compressed/acpitb.c | 33 +++++++++++++++++++++++++++++++
>  arch/x86/boot/compressed/misc.c   |  5 +++++
>  arch/x86/boot/compressed/misc.h   |  4 ++++
>  lib/kstrtox.c                     |  5 +++++
>  4 files changed, 47 insertions(+)
>  create mode 100644 arch/x86/boot/compressed/acpitb.c
> 
> diff --git a/arch/x86/boot/compressed/acpitb.c b/arch/x86/boot/compressed/acpitb.c
> new file mode 100644
> index 000000000000..614c45655cff
> --- /dev/null
> +++ b/arch/x86/boot/compressed/acpitb.c
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define BOOT_CTYPE_H
> +#include "misc.h"
> +#include "error.h"
> +
> +#include <linux/efi.h>
> +#include <asm/efi.h>
> +#include <linux/numa.h>
> +#include <linux/acpi.h>
> +
> +#define STATIC
> +#include <linux/decompress/mm.h>
> +
> +/* Store the immovable memory regions. */
> +struct mem_vector immovable_mem[MAX_NUMNODES*2];
> +#endif
> +
> +static acpi_physical_address get_acpi_rsdp(void)
> +{
> +#ifdef CONFIG_KEXEC
> +	unsigned long long res;
> +	int len = 0;
> +	char *val;
> +
> +	val = malloc(19);
> +	len = cmdline_find_option("acpi_rsdp", val, 19);
> +	if (len > 0) {
> +		val[len] = 0;
> +		return (acpi_physical_address)kstrtoull(val, 16, &res);
> +	}
> +	return 0;
> +#endif
> +}
> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> index 8dd1d5ccae58..e51713fe3add 100644
> --- a/arch/x86/boot/compressed/misc.c
> +++ b/arch/x86/boot/compressed/misc.c
> @@ -12,6 +12,7 @@
>   * High loaded stuff by Hans Lermen & Werner Almesberger, Feb. 1996
>   */
>  
> +#define BOOT_CTYPE_H
>  #include "misc.h"
>  #include "error.h"
>  #include "pgtable.h"
> @@ -426,3 +427,7 @@ void fortify_panic(const char *name)
>  {
>  	error("detected buffer overflow");
>  }
> +
> +#ifdef BOOT_STRING
> +#include "../../../../lib/kstrtox.c"
> +#endif
> diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> index a1d5918765f3..809c31effa4b 100644
> --- a/arch/x86/boot/compressed/misc.h
> +++ b/arch/x86/boot/compressed/misc.h
> @@ -116,3 +116,7 @@ static inline void console_init(void)
>  void set_sev_encryption_mask(void);
>  
>  #endif
> +
> +/* acpitb.c */
> +#define BOOT_STRING
> +extern int kstrtoull(const char *s, unsigned int base, unsigned long long *res);
> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
> index 1006bf70bf74..a0ac1b2257b8 100644
> --- a/lib/kstrtox.c
> +++ b/lib/kstrtox.c
> @@ -126,6 +126,9 @@ int kstrtoull(const char *s, unsigned int base, unsigned long long *res)
>  }
>  EXPORT_SYMBOL(kstrtoull);
>  
> +/* Make compressed period code be able to use kstrtoull(). */
> +#ifndef BOOT_STRING

I got the following build error.

]$ make arch/x86/boot/compressed/misc.o
  CALL    scripts/checksyscalls.sh
  DESCEND  objtool
  CC      arch/x86/boot/compressed/misc.o
ld: -r and -pie may not be used together
make[1]: *** [scripts/Makefile.build:294: arch/x86/boot/compressed/misc.o] Error 1
make: *** [Makefile:1715: arch/x86/boot/compressed/misc.o] Error 2
]$

I think this error gets fixed by changing the BOOT_STRING ifndef
order before the EXPORT_SYMBOL, like this:

#ifndef BOOT_STRING
EXPORT_SYMBOL(kstrtoull);

I'm not sure this change is a good way...

Thanks,
Masa

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Chao Fan]

>> +
>> +/* acpitb.c */
>> +#define BOOT_STRING
>> +extern int kstrtoull(const char *s, unsigned int base, unsigned long long *res);
>> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
>> index 1006bf70bf74..a0ac1b2257b8 100644
>> --- a/lib/kstrtox.c
>> +++ b/lib/kstrtox.c
>> @@ -126,6 +126,9 @@ int kstrtoull(const char *s, unsigned int base, unsigned long long *res)
>>  }
>>  EXPORT_SYMBOL(kstrtoull);
>>  
>> +/* Make compressed period code be able to use kstrtoull(). */
>> +#ifndef BOOT_STRING
>
>I got the following build error.
>
>]$ make arch/x86/boot/compressed/misc.o
>  CALL    scripts/checksyscalls.sh
>  DESCEND  objtool
>  CC      arch/x86/boot/compressed/misc.o
>ld: -r and -pie may not be used together
>make[1]: *** [scripts/Makefile.build:294: arch/x86/boot/compressed/misc.o] Error 1
>make: *** [Makefile:1715: arch/x86/boot/compressed/misc.o] Error 2
>]$

Hi Masa,

So many thanks for your test.

Could you give me more details about this error? More error message.
Just on the first commit or the whole PATCHSET?
Cause I didn't get error both on this commit and on the whole PATCHSET.

Thanks,
Chao Fan

>
>I think this error gets fixed by changing the BOOT_STRING ifndef
>order before the EXPORT_SYMBOL, like this:
>
>#ifndef BOOT_STRING
>EXPORT_SYMBOL(kstrtoull);
>
>I'm not sure this change is a good way...
>
>Thanks,
>Masa
>
>

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Masayoshi Mizuma]

On Fri, Nov 30, 2018 at 10:43:47AM +0800, Chao Fan wrote:
...
> >]$ make arch/x86/boot/compressed/misc.o
> >  CALL    scripts/checksyscalls.sh
> >  DESCEND  objtool
> >  CC      arch/x86/boot/compressed/misc.o
> >ld: -r and -pie may not be used together
> >make[1]: *** [scripts/Makefile.build:294: arch/x86/boot/compressed/misc.o] Error 1
> >make: *** [Makefile:1715: arch/x86/boot/compressed/misc.o] Error 2
> >]$
> 
> Hi Masa,
> 
> So many thanks for your test.
> 
> Could you give me more details about this error? More error message.
> Just on the first commit or the whole PATCHSET?
> Cause I didn't get error both on this commit and on the whole PATCHSET.

I built your whole patchset and got the error.
The error depends on CONFIG_MODVERSIONS.
If CONFIG_MODVERSIONS=y, you will get the build error.

Thanks,
Masa

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Chao Fan]

On Fri, Nov 30, 2018 at 12:35:16PM -0500, Masayoshi Mizuma wrote:
>On Fri, Nov 30, 2018 at 10:43:47AM +0800, Chao Fan wrote:
>...
>> >]$ make arch/x86/boot/compressed/misc.o
>> >  CALL    scripts/checksyscalls.sh
>> >  DESCEND  objtool
>> >  CC      arch/x86/boot/compressed/misc.o
>> >ld: -r and -pie may not be used together
>> >make[1]: *** [scripts/Makefile.build:294: arch/x86/boot/compressed/misc.o] Error 1
>> >make: *** [Makefile:1715: arch/x86/boot/compressed/misc.o] Error 2
>> >]$
>> 
>> Hi Masa,
>> 
>> So many thanks for your test.
>> 
>> Could you give me more details about this error? More error message.
>> Just on the first commit or the whole PATCHSET?
>> Cause I didn't get error both on this commit and on the whole PATCHSET.
>
>I built your whole patchset and got the error.
>The error depends on CONFIG_MODVERSIONS.
>If CONFIG_MODVERSIONS=y, you will get the build error.

Hi Masa,

Thanks, after that, I got the error.
About your solution, it can fix the error. But I am not sure whether
it's good.

Boris and Baoquan,
How do you think this issue.

Thanks,
Chao Fan

>
>Thanks,
>Masa
>
>

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Borislav Petkov]

On Sat, Dec 01, 2018 at 02:05:39PM +0800, Chao Fan wrote:
> >I built your whole patchset and got the error.
> >The error depends on CONFIG_MODVERSIONS.
> >If CONFIG_MODVERSIONS=y, you will get the build error.
> 
> Hi Masa,
> 
> Thanks, after that, I got the error.
> About your solution, it can fix the error. But I am not sure whether
> it's good.

What does CONFIG_MODVERSIONS=y have to do with this? I don't see the
connection.

Also, your patch triggers another build error:

ld -m elf_x86_64 -z noreloc-overflow -pie --no-dynamic-linker   -T arch/x86/boot/compressed/vmlinux.lds arch/x86/boot/compressed/head_64.o arch/x86/boot/compressed/misc.o arch/x86/boot/compressed/string.o arch/x86/boot/compressed/cmdline.o arch/x86/boot/compressed/error.o arch/x86/boot/compressed/piggy.o arch/x86/boot/compressed/cpuflags.o arch/x86/boot/compressed/early_serial_console.o arch/x86/boot/compressed/mem_encrypt.o arch/x86/boot/compressed/pgtable_64.o arch/x86/boot/compressed/eboot.o arch/x86/boot/compressed/efi_stub_64.o drivers/firmware/efi/libstub/lib.a arch/x86/boot/compressed/efi_thunk_64.o -o arch/x86/boot/compressed/vmlinux
ld: arch/x86/boot/compressed/misc.o: in function `_parse_integer_fixup_radix':
misc.c:(.text+0x3057): undefined reference to `_ctype'
make[2]: *** [arch/x86/boot/compressed/Makefile:116: arch/x86/boot/compressed/vmlinux] Error 1
make[1]: *** [arch/x86/boot/Makefile:112: arch/x86/boot/compressed/vmlinux] Error 2
make: *** [arch/x86/Makefile:289: bzImage] Error 2
make: *** Waiting for unfinished jobs....

due to misc.o not seeing _ctype.

Looks like this is going to become too hairy and perhaps it would
be easier and cleaner if you copy all the functions needed into
arch/x86/boot/cmdline.c and this way get rid of the crazy ifdeffery and
the subtle build issues.

I mean, we do this already in other places like perf tool is copying
stuff selectively from kernel proper and since compressed/ is a
completely separate stage, we probably should do that for the sake of
keeping our sanity. :)

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Chao Fan]

On Tue, Dec 04, 2018 at 07:42:20PM +0100, Borislav Petkov wrote:
>On Sat, Dec 01, 2018 at 02:05:39PM +0800, Chao Fan wrote:
>> >I built your whole patchset and got the error.
>> >The error depends on CONFIG_MODVERSIONS.
>> >If CONFIG_MODVERSIONS=y, you will get the build error.
>> 
>> Hi Masa,
>> 
>> Thanks, after that, I got the error.
>> About your solution, it can fix the error. But I am not sure whether
>> it's good.
>
>What does CONFIG_MODVERSIONS=y have to do with this? I don't see the
>connection.
>
>Also, your patch triggers another build error:
>
>ld -m elf_x86_64 -z noreloc-overflow -pie --no-dynamic-linker   -T arch/x86/boot/compressed/vmlinux.lds arch/x86/boot/compressed/head_64.o arch/x86/boot/compressed/misc.o arch/x86/boot/compressed/string.o arch/x86/boot/compressed/cmdline.o arch/x86/boot/compressed/error.o arch/x86/boot/compressed/piggy.o arch/x86/boot/compressed/cpuflags.o arch/x86/boot/compressed/early_serial_console.o arch/x86/boot/compressed/mem_encrypt.o arch/x86/boot/compressed/pgtable_64.o arch/x86/boot/compressed/eboot.o arch/x86/boot/compressed/efi_stub_64.o drivers/firmware/efi/libstub/lib.a arch/x86/boot/compressed/efi_thunk_64.o -o arch/x86/boot/compressed/vmlinux
>ld: arch/x86/boot/compressed/misc.o: in function `_parse_integer_fixup_radix':
>misc.c:(.text+0x3057): undefined reference to `_ctype'
>make[2]: *** [arch/x86/boot/compressed/Makefile:116: arch/x86/boot/compressed/vmlinux] Error 1
>make[1]: *** [arch/x86/boot/Makefile:112: arch/x86/boot/compressed/vmlinux] Error 2
>make: *** [arch/x86/Makefile:289: bzImage] Error 2
>make: *** Waiting for unfinished jobs....
>
>due to misc.o not seeing _ctype.
>
>Looks like this is going to become too hairy and perhaps it would
>be easier and cleaner if you copy all the functions needed into
>arch/x86/boot/cmdline.c and this way get rid of the crazy ifdeffery and
>the subtle build issues.
>
>I mean, we do this already in other places like perf tool is copying
>stuff selectively from kernel proper and since compressed/ is a
>completely separate stage, we probably should do that for the sake of
>keeping our sanity. :)

Thank you, I will do the copying job and consider more about putting
functions to arch/x86/boot/cmdline.c or arch/x86/boot/string.c.
Anyway, this method is simple.

Thanks,
Chao Fan

>
>Thx.
>
>-- 
>Regards/Gruss,
>    Boris.
>
>Good mailing practices for 400: avoid top-posting and trim the reply.
>
>

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Chao Fan]

Hi Boris, Baoquan and Masa,

When copying kstrtoull() and functions needed to arch/x86/boot/string.c,
I got an error in LD period:

  LD      arch/x86/boot/setup.elf
ld: arch/x86/boot/string.o: in function `div_u64_rem':
/home/cfan/code/tip/./include/linux/math64.h:28: undefined reference to `__udivdi3'
make[1]: *** [arch/x86/boot/Makefile:105: arch/x86/boot/setup.elf] Error 1
make: *** [arch/x86/Makefile:289: bzImage] Error 2

I google it and the key problem is div_u64(ULLONG_MAX - val, base))
when dividend is u64 and divisor is u32, the dividend must be a variable,
since the result will be stored in the first variable.
So here when I changed it to ((ULLONG_MAX - val) / base), I got the
same error.
But what puzzled me is, why the cdoe in lib/kstrtox.c works well.
And in old version PATCHSET, I ever copy the code to
arch/x86/boot/compressed/misc.c, it also worked well.

So I wonder if there are some options I miss or some problems.
The patch is below.

Thanks,
Chao Fan


diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c
index c4428a176973..62ffe0437c8e 100644
--- a/arch/x86/boot/string.c
+++ b/arch/x86/boot/string.c
@@ -13,6 +13,9 @@
  */

 #include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/math64.h>
 #include <asm/asm.h>
 #include "ctype.h"
 #include "string.h"
@@ -187,3 +190,112 @@ char *strchr(const char *s, int c)
                        return NULL;
        return (char *)s;
 }
+
+static inline char _tolower(const char c)
+{
+       return c | 0x20;
+}
+
+const char *_parse_integer_fixup_radix(const char *s, unsigned int *base)
+{
+       if (*base == 0) {
+               if (s[0] == '0') {
+                       if (_tolower(s[1]) == 'x' && isxdigit(s[2]))
+                               *base = 16;
+                       else
+                               *base = 8;
+               } else
+                       *base = 10;
+       }
+       if (*base == 16 && s[0] == '0' && _tolower(s[1]) == 'x')
+               s += 2;
+       return s;
+}
+
+/*
+ * Convert non-negative integer string representation in explicitly given radix
+ * to an integer.
+ * Return number of characters consumed maybe or-ed with overflow bit.
+ * If overflow occurs, result integer (incorrect) is still returned.
+ *
+ * Don't you dare use this function.
+ */
+unsigned int _parse_integer(const char *s, unsigned int base, unsigned long long *p)
+{
+       unsigned long long res;
+       unsigned int rv;
+
+       res = 0;
+       rv = 0;
+       while (1) {
+               unsigned int c = *s;
+               unsigned int lc = c | 0x20; /* don't tolower() this line */
+               unsigned int val;
+
+               if ('0' <= c && c <= '9')
+                       val = c - '0';
+               else if ('a' <= lc && lc <= 'f')
+                       val = lc - 'a' + 10;
+               else
+                       break;
+
+               if (val >= base)
+                       break;
+               /*
+                * Check for overflow only if we are within range of
+                * it in the max base we support (16)
+                */
+               if (unlikely(res & (~0ull << 60))) {
+                       if (res > div_u64(ULLONG_MAX - val, base))
+                               rv |= KSTRTOX_OVERFLOW;
+               }
+               res = res * base + val;
+               rv++;
+               s++;
+       }
+       *p = res;
+       return rv;
+}
+
+static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
+{
+       unsigned long long _res;
+       unsigned int rv;
+
+       s = _parse_integer_fixup_radix(s, &base);
+       rv = _parse_integer(s, base, &_res);
+       if (rv & KSTRTOX_OVERFLOW)
+               return -ERANGE;
+       if (rv == 0)
+               return -EINVAL;
+       s += rv;
+       if (*s == '\n')
+               s++;
+       if (*s)
+               return -EINVAL;
+       *res = _res;
+       return 0;
+}
+
+/**
+ * kstrtoull - convert a string to an unsigned long long
+ * @s: The start of the string. The string must be null-terminated, and may also
+ *  include a single newline before its terminating null. The first character
+ *  may also be a plus sign, but not a minus sign.
+ * @base: The number base to use. The maximum supported base is 16. If base is
+ *  given as 0, then the base of the string is automatically detected with the
+ *  conventional semantics - If it begins with 0x the number will be parsed as a
+ *  hexadecimal (case insensitive), if it otherwise begins with 0, it will be
+ *  parsed as an octal number. Otherwise it will be parsed as a decimal.
+ * @res: Where to write the result of the conversion on success.
+ *
+ * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
+ * Used as a replacement for the obsolete simple_strtoull. Return code must
+ * be checked.
+ */
+int kstrtoull(const char *s, unsigned int base, unsigned long long *res)
+{
+       if (s[0] == '+')
+               s++;
+       return _kstrtoull(s, base, res);
+}
diff --git a/arch/x86/boot/string.h b/arch/x86/boot/string.h
index 3d78e27077f4..9619c8990d95 100644
--- a/arch/x86/boot/string.h
+++ b/arch/x86/boot/string.h
@@ -29,4 +29,6 @@ extern unsigned int atou(const char *s);
 extern unsigned long long simple_strtoull(const char *cp, char **endp,
                                          unsigned int base);

+#define KSTRTOX_OVERFLOW       (1U << 31)
+
 #endif /* BOOT_STRING_H */

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Baoquan He]

Hi,

On 11/29/18 at 04:10pm, Masayoshi Mizuma wrote:
> > diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> > index 8dd1d5ccae58..e51713fe3add 100644
> > --- a/arch/x86/boot/compressed/misc.c
> > +++ b/arch/x86/boot/compressed/misc.c
> > @@ -12,6 +12,7 @@
> >   * High loaded stuff by Hans Lermen & Werner Almesberger, Feb. 1996
> >   */
> >  
> > +#define BOOT_CTYPE_H
> >  #include "misc.h"
> >  #include "error.h"
> >  #include "pgtable.h"
> > @@ -426,3 +427,7 @@ void fortify_panic(const char *name)
> >  {
> >  	error("detected buffer overflow");
> >  }
> > +
> > +#ifdef BOOT_STRING
> > +#include "../../../../lib/kstrtox.c"
> > +#endif
> > diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> > index a1d5918765f3..809c31effa4b 100644
> > --- a/arch/x86/boot/compressed/misc.h
> > +++ b/arch/x86/boot/compressed/misc.h
> > @@ -116,3 +116,7 @@ static inline void console_init(void)
> >  void set_sev_encryption_mask(void);
> >  
> >  #endif
> > +
> > +/* acpitb.c */
> > +#define BOOT_STRING
> > +extern int kstrtoull(const char *s, unsigned int base, unsigned long long *res);
> > diff --git a/lib/kstrtox.c b/lib/kstrtox.c
> > index 1006bf70bf74..a0ac1b2257b8 100644
> > --- a/lib/kstrtox.c
> > +++ b/lib/kstrtox.c
> > @@ -126,6 +126,9 @@ int kstrtoull(const char *s, unsigned int base, unsigned long long *res)
> >  }
> >  EXPORT_SYMBOL(kstrtoull);
> >  
> > +/* Make compressed period code be able to use kstrtoull(). */
> > +#ifndef BOOT_STRING
> 
> I got the following build error.
> 
> ]$ make arch/x86/boot/compressed/misc.o
>   CALL    scripts/checksyscalls.sh
>   DESCEND  objtool
>   CC      arch/x86/boot/compressed/misc.o
> ld: -r and -pie may not be used together
> make[1]: *** [scripts/Makefile.build:294: arch/x86/boot/compressed/misc.o] Error 1
> make: *** [Makefile:1715: arch/x86/boot/compressed/misc.o] Error 2

I have met this issue when change code in boot/compressed/kaslr.c.

I remember Ingo merged my patch and found this build error. Since if 
CONFIG_MODULES=n EXPORT_SYMBOL is defined as empty in
include/linux/export.h, that's why you can only meet it only if
CONFIG_MODULES=y.

I remember I just muted it with below code, please check commit
d52e7d5a95.

+#define _LINUX_EXPORT_H
+#define EXPORT_SYMBOL(sym)

commit d52e7d5a952c5e35783f96e8c5b7fcffbb0d7c60
Author: Baoquan He <bhe@redhat.com>
Date:   Sat May 13 13:46:28 2017 +0800

    x86/KASLR: Parse all 'memmap=' boot option entries

Later in commit f922c4abdf76, Ard added a new switch __DISABLE_EXPORTS.
So you can just add #define __DISABLE_EXPORTS into misc.c. Then you
don't need to copy those lib functions to boot. Maintaining two copies
might have trouble if it has updates later.

commit f922c4abdf7648523589abee9460c87f51630d2f
Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date:   Tue Aug 21 21:56:04 2018 -0700

    module: allow symbol exports to be disabled

Thanks
Baoquan

> ]$
> 
> I think this error gets fixed by changing the BOOT_STRING ifndef
> order before the EXPORT_SYMBOL, like this:
> 
> #ifndef BOOT_STRING
> EXPORT_SYMBOL(kstrtoull);
> 
> I'm not sure this change is a good way...
> 
> Thanks,
> Masa

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Baoquan He]

On 12/07/18 at 10:10am, Baoquan He wrote:
> Hi,
> 
> On 11/29/18 at 04:10pm, Masayoshi Mizuma wrote:
> > > diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> > > index 8dd1d5ccae58..e51713fe3add 100644
> > > --- a/arch/x86/boot/compressed/misc.c
> > > +++ b/arch/x86/boot/compressed/misc.c
> > > @@ -12,6 +12,7 @@
> > >   * High loaded stuff by Hans Lermen & Werner Almesberger, Feb. 1996
> > >   */
> > >  
> > > +#define BOOT_CTYPE_H
> > >  #include "misc.h"
> > >  #include "error.h"
> > >  #include "pgtable.h"
> > > @@ -426,3 +427,7 @@ void fortify_panic(const char *name)
> > >  {
> > >  	error("detected buffer overflow");
> > >  }
> > > +
> > > +#ifdef BOOT_STRING
> > > +#include "../../../../lib/kstrtox.c"
> > > +#endif
> > > diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> > > index a1d5918765f3..809c31effa4b 100644
> > > --- a/arch/x86/boot/compressed/misc.h
> > > +++ b/arch/x86/boot/compressed/misc.h
> > > @@ -116,3 +116,7 @@ static inline void console_init(void)
> > >  void set_sev_encryption_mask(void);
> > >  
> > >  #endif
> > > +
> > > +/* acpitb.c */
> > > +#define BOOT_STRING
> > > +extern int kstrtoull(const char *s, unsigned int base, unsigned long long *res);
> > > diff --git a/lib/kstrtox.c b/lib/kstrtox.c
> > > index 1006bf70bf74..a0ac1b2257b8 100644
> > > --- a/lib/kstrtox.c
> > > +++ b/lib/kstrtox.c
> > > @@ -126,6 +126,9 @@ int kstrtoull(const char *s, unsigned int base, unsigned long long *res)
> > >  }
> > >  EXPORT_SYMBOL(kstrtoull);
> > >  
> > > +/* Make compressed period code be able to use kstrtoull(). */
> > > +#ifndef BOOT_STRING
> > 
> > I got the following build error.
> > 
> > ]$ make arch/x86/boot/compressed/misc.o
> >   CALL    scripts/checksyscalls.sh
> >   DESCEND  objtool
> >   CC      arch/x86/boot/compressed/misc.o
> > ld: -r and -pie may not be used together
> > make[1]: *** [scripts/Makefile.build:294: arch/x86/boot/compressed/misc.o] Error 1
> > make: *** [Makefile:1715: arch/x86/boot/compressed/misc.o] Error 2
> 
> I have met this issue when change code in boot/compressed/kaslr.c.
> 
> I remember Ingo merged my patch and found this build error. Since if 
> CONFIG_MODULES=n EXPORT_SYMBOL is defined as empty in
> include/linux/export.h, that's why you can only meet it only if
> CONFIG_MODULES=y.
> 
> I remember I just muted it with below code, please check commit
> d52e7d5a95.
> 
> +#define _LINUX_EXPORT_H
> +#define EXPORT_SYMBOL(sym)

I made below change, it passed. My another question is why you need
include "lib/kstrtox.c" into misc.c. Does it make sense if you plan to
use it to replace simple_strtoull() in arch/x86/boot/string.c ?

diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index e51713fe3add..777e807756e8 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -13,6 +13,8 @@
  */
 
 #define BOOT_CTYPE_H
+#define __DISABLE_EXPORTS
+
 #include "misc.h"
 #include "error.h"
 #include "pgtable.h"

I just tried to include it into boot/string.c, there's no any problem.

Thanks
Baoquan

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Chao Fan]

On Fri, Dec 07, 2018 at 10:50:21AM +0800, Baoquan He wrote:
>On 12/07/18 at 10:10am, Baoquan He wrote:
>> Hi,
>> 
>> On 11/29/18 at 04:10pm, Masayoshi Mizuma wrote:
>> > > diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
>> > > index 8dd1d5ccae58..e51713fe3add 100644
>> > > --- a/arch/x86/boot/compressed/misc.c
>> > > +++ b/arch/x86/boot/compressed/misc.c
>> > > @@ -12,6 +12,7 @@
>> > >   * High loaded stuff by Hans Lermen & Werner Almesberger, Feb. 1996
>> > >   */
>> > >  
>> > > +#define BOOT_CTYPE_H
>> > >  #include "misc.h"
>> > >  #include "error.h"
>> > >  #include "pgtable.h"
>> > > @@ -426,3 +427,7 @@ void fortify_panic(const char *name)
>> > >  {
>> > >  	error("detected buffer overflow");
>> > >  }
>> > > +
>> > > +#ifdef BOOT_STRING
>> > > +#include "../../../../lib/kstrtox.c"
>> > > +#endif
>> > > diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
>> > > index a1d5918765f3..809c31effa4b 100644
>> > > --- a/arch/x86/boot/compressed/misc.h
>> > > +++ b/arch/x86/boot/compressed/misc.h
>> > > @@ -116,3 +116,7 @@ static inline void console_init(void)
>> > >  void set_sev_encryption_mask(void);
>> > >  
>> > >  #endif
>> > > +
>> > > +/* acpitb.c */
>> > > +#define BOOT_STRING
>> > > +extern int kstrtoull(const char *s, unsigned int base, unsigned long long *res);
>> > > diff --git a/lib/kstrtox.c b/lib/kstrtox.c
>> > > index 1006bf70bf74..a0ac1b2257b8 100644
>> > > --- a/lib/kstrtox.c
>> > > +++ b/lib/kstrtox.c
>> > > @@ -126,6 +126,9 @@ int kstrtoull(const char *s, unsigned int base, unsigned long long *res)
>> > >  }
>> > >  EXPORT_SYMBOL(kstrtoull);
>> > >  
>> > > +/* Make compressed period code be able to use kstrtoull(). */
>> > > +#ifndef BOOT_STRING
>> > 
>> > I got the following build error.
>> > 
>> > ]$ make arch/x86/boot/compressed/misc.o
>> >   CALL    scripts/checksyscalls.sh
>> >   DESCEND  objtool
>> >   CC      arch/x86/boot/compressed/misc.o
>> > ld: -r and -pie may not be used together
>> > make[1]: *** [scripts/Makefile.build:294: arch/x86/boot/compressed/misc.o] Error 1
>> > make: *** [Makefile:1715: arch/x86/boot/compressed/misc.o] Error 2
>> 
>> I have met this issue when change code in boot/compressed/kaslr.c.
>> 
>> I remember Ingo merged my patch and found this build error. Since if 
>> CONFIG_MODULES=n EXPORT_SYMBOL is defined as empty in
>> include/linux/export.h, that's why you can only meet it only if
>> CONFIG_MODULES=y.
>> 
>> I remember I just muted it with below code, please check commit
>> d52e7d5a95.
>> 
>> +#define _LINUX_EXPORT_H
>> +#define EXPORT_SYMBOL(sym)
>
>I made below change, it passed. My another question is why you need
>include "lib/kstrtox.c" into misc.c. Does it make sense if you plan to
>use it to replace simple_strtoull() in arch/x86/boot/string.c ?
>
>diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
>index e51713fe3add..777e807756e8 100644
>--- a/arch/x86/boot/compressed/misc.c
>+++ b/arch/x86/boot/compressed/misc.c
>@@ -13,6 +13,8 @@
>  */
> 
> #define BOOT_CTYPE_H
>+#define __DISABLE_EXPORTS
>+
> #include "misc.h"
> #include "error.h"
> #include "pgtable.h"
>
>I just tried to include it into boot/string.c, there's no any problem.

Thanks for your help, I think it's better to include it into
boot/string.c.

Thanks,
Chao Fan
>
>Thanks
>Baoquan
>
>

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Borislav Petkov]

On Thu, Nov 29, 2018 at 04:16:27PM +0800, Chao Fan wrote:
> To fix the conflict between KASLR and memory-hotremove, memory
> information in SRAT table is necessary.
> 
> ACPI SRAT (System/Static Resource Affinity Table) can show the details
> about memory ranges, including ranges of memory provided by hot-added
> memory devices. SRAT table must be introduced by RSDP pointer (Root
> System Description Pointer). So RSDP should be found firstly.
> 
> When booting form KEXEC/EFI/BIOS, the methods to find RSDP pointer
> are different. When booting from KEXEC, 'acpi_rsdp' may have been
> added to cmdline, so parse the cmdline and find the RSDP pointer.
> 
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  arch/x86/boot/compressed/acpitb.c | 33 +++++++++++++++++++++++++++++++
>  arch/x86/boot/compressed/misc.c   |  5 +++++
>  arch/x86/boot/compressed/misc.h   |  4 ++++
>  lib/kstrtox.c                     |  5 +++++
>  4 files changed, 47 insertions(+)
>  create mode 100644 arch/x86/boot/compressed/acpitb.c
> 
> diff --git a/arch/x86/boot/compressed/acpitb.c b/arch/x86/boot/compressed/acpitb.c
> new file mode 100644
> index 000000000000..614c45655cff
> --- /dev/null
> +++ b/arch/x86/boot/compressed/acpitb.c

Also, I guess calling that file simply "acpi.c" is good enough.

> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define BOOT_CTYPE_H
> +#include "misc.h"
> +#include "error.h"
> +
> +#include <linux/efi.h>
> +#include <asm/efi.h>

asm/ headers come after linux/ header.

...

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v12 1/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC

[Chao Fan]

On Wed, Dec 05, 2018 at 03:58:14PM +0100, Borislav Petkov wrote:
>On Thu, Nov 29, 2018 at 04:16:27PM +0800, Chao Fan wrote:
>> To fix the conflict between KASLR and memory-hotremove, memory
>> information in SRAT table is necessary.
>> 
>> ACPI SRAT (System/Static Resource Affinity Table) can show the details
>> about memory ranges, including ranges of memory provided by hot-added
>> memory devices. SRAT table must be introduced by RSDP pointer (Root
>> System Description Pointer). So RSDP should be found firstly.
>> 
>> When booting form KEXEC/EFI/BIOS, the methods to find RSDP pointer
>> are different. When booting from KEXEC, 'acpi_rsdp' may have been
>> added to cmdline, so parse the cmdline and find the RSDP pointer.
>> 
>> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
>> ---
>>  arch/x86/boot/compressed/acpitb.c | 33 +++++++++++++++++++++++++++++++
>>  arch/x86/boot/compressed/misc.c   |  5 +++++
>>  arch/x86/boot/compressed/misc.h   |  4 ++++
>>  lib/kstrtox.c                     |  5 +++++
>>  4 files changed, 47 insertions(+)
>>  create mode 100644 arch/x86/boot/compressed/acpitb.c
>> 
>> diff --git a/arch/x86/boot/compressed/acpitb.c b/arch/x86/boot/compressed/acpitb.c
>> new file mode 100644
>> index 000000000000..614c45655cff
>> --- /dev/null
>> +++ b/arch/x86/boot/compressed/acpitb.c
>
>Also, I guess calling that file simply "acpi.c" is good enough.

Fine, so change it next version.

>
>> @@ -0,0 +1,33 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#define BOOT_CTYPE_H
>> +#include "misc.h"
>> +#include "error.h"
>> +
>> +#include <linux/efi.h>
>> +#include <asm/efi.h>
>
>asm/ headers come after linux/ header.

Will change the place of header.

Thanks,
Chao Fan

>
>...
>
>Thx.
>
>-- 
>Regards/Gruss,
>    Boris.
>
>Good mailing practices for 400: avoid top-posting and trim the reply.
>
>

[PATCH v12 2/5] x86/boot: Add efi_get_rsdp_addr() to find RSDP from EFI table

[Chao Fan]

To fix the conflict between KASLR and memory-hotremove, memory
information in SRAT table is necessary. So RSDP and SRAT table
should be parsed.

When booting form KEXEC/EFI/BIOS, the methods to compute RSDP pointer
are different. When booting from EFI, EFI table points to RSDP.
So parse the EFI table and find the RSDP pointer.

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

diff --git a/arch/x86/boot/compressed/acpitb.c b/arch/x86/boot/compressed/acpitb.c
index 614c45655cff..c43546648638 100644
--- a/arch/x86/boot/compressed/acpitb.c
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -31,3 +31,82 @@ static acpi_physical_address get_acpi_rsdp(void)
 	return 0;
 #endif
 }
+
+/* Search EFI table for RSDP. */
+static acpi_physical_address efi_get_rsdp_addr(void)
+{
+#ifdef CONFIG_EFI
+	acpi_physical_address rsdp_addr = 0;
+	efi_system_table_t *systab;
+	struct efi_info *e;
+	bool efi_64;
+	char *sig;
+	int size;
+	int i;
+
+	e = &boot_params->efi_info;
+	sig = (char *)&e->efi_loader_signature;
+
+	if (!strncmp(sig, EFI64_LOADER_SIGNATURE, 4))
+		efi_64 = true;
+	else if (!strncmp(sig, EFI32_LOADER_SIGNATURE, 4))
+		efi_64 = false;
+	else {
+		debug_putstr("Wrong EFI loader signature.\n");
+		return 0;
+	}
+
+	/* Get systab from boot params. Based on efi_init(). */
+#ifdef CONFIG_X86_64
+	systab = (efi_system_table_t *)(e->efi_systab | ((__u64)e->efi_systab_hi<<32));
+#else
+	if (e->efi_systab_hi || e->efi_memmap_hi) {
+		debug_putstr("Error getting RSDP address: EFI system table located above 4GB.\n");
+		return 0;
+	}
+	systab = (efi_system_table_t *)e->efi_systab;
+#endif
+
+	if (!systab)
+		return 0;
+
+	/*
+	 * Get EFI tables from systab. Based on efi_config_init() and
+	 * efi_config_parse_tables().
+	 */
+	size = efi_64 ? sizeof(efi_config_table_64_t) :
+			sizeof(efi_config_table_32_t);
+
+	for (i = 0; i < systab->nr_tables; i++) {
+		void *config_tables;
+		unsigned long table;
+		efi_guid_t guid;
+
+		config_tables = (void *)(systab->tables + size * i);
+		if (efi_64) {
+			efi_config_table_64_t *tmp_table;
+
+			tmp_table = (efi_config_table_64_t *)config_tables;
+			guid = tmp_table->guid;
+			table = tmp_table->table;
+
+			if (!IS_ENABLED(CONFIG_X86_64) && table >> 32) {
+				debug_putstr("Error getting RSDP address: EFI system table located above 4GB.\n");
+				return 0;
+			}
+		} else {
+			efi_config_table_32_t *tmp_table;
+
+			tmp_table = (efi_config_table_32_t *)config_tables;
+			guid = tmp_table->guid;
+			table = tmp_table->table;
+		}
+
+		if (!(efi_guidcmp(guid, ACPI_TABLE_GUID)))
+			rsdp_addr = (acpi_physical_address)table;
+		else if (!(efi_guidcmp(guid, ACPI_20_TABLE_GUID)))
+			return (acpi_physical_address)table;
+	}
+	return rsdp_addr;
+#endif
+}
-- 
2.19.1

[PATCH v12 3/5] x86/boot: Add bios_get_rsdp_addr() to search RSDP in memory

[Chao Fan]

To fix the conflict between KASLR and memory-hotremove, memory
information in SRAT table is necessary. So RSDP and SRAT table
should be parsed.

When booting form KEXEC/EFI/BIOS, the methods to compute RSDP pointer
are different. When booting from BIOS, there is no variable who can
point to RSDP directly, so scan memory for the RSDP and verify RSDP
by signature and checksum.

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

diff --git a/arch/x86/boot/compressed/acpitb.c b/arch/x86/boot/compressed/acpitb.c
index c43546648638..82d27c4b8978 100644
--- a/arch/x86/boot/compressed/acpitb.c
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -110,3 +110,88 @@ static acpi_physical_address efi_get_rsdp_addr(void)
 	return rsdp_addr;
 #endif
 }
+
+static u8 compute_checksum(u8 *buffer, u32 length)
+{
+	u8 *end = buffer + length;
+	u8 sum = 0;
+
+	while (buffer < end)
+		sum += *(buffer++);
+
+	return sum;
+}
+
+/* Search a block of memory for the RSDP signature. */
+static u8 *scan_mem_for_rsdp(u8 *start, u32 length)
+{
+	struct acpi_table_rsdp *rsdp;
+	u8 *address;
+	u8 *end;
+
+	end = start + length;
+
+	/* Search from given start address for the requested length */
+	for (address = start; address < end; address += ACPI_RSDP_SCAN_STEP) {
+		/*
+		 * Both RSDP signature and checksum must be correct.
+		 * Note: Sometimes there exists more than one RSDP in memory;
+		 * the valid RSDP has a valid checksum, all others have an
+		 * invalid checksum.
+		 */
+		rsdp = (struct acpi_table_rsdp *)address;
+
+		/* BAD Signature */
+		if (!ACPI_VALIDATE_RSDP_SIG(rsdp->signature))
+			continue;
+
+		/* Check the standard checksum */
+		if (compute_checksum((u8 *) rsdp, ACPI_RSDP_CHECKSUM_LENGTH))
+			continue;
+
+		/* Check extended checksum if table version >= 2 */
+		if ((rsdp->revision >= 2) &&
+		    (compute_checksum((u8 *) rsdp, ACPI_RSDP_XCHECKSUM_LENGTH)))
+			continue;
+
+		/* Signature and checksum valid, we have found a real RSDP */
+		return address;
+	}
+	return NULL;
+}
+
+/* Used to search RSDP physical address, based on acpi_find_root_pointer(). */
+static acpi_physical_address bios_get_rsdp_addr(void)
+{
+	u8 *table_ptr;
+	u32 address;
+	u8 *rsdp;
+
+	/* Get the location of the Extended BIOS Data Area (EBDA) */
+	table_ptr = (u8 *)ACPI_EBDA_PTR_LOCATION;
+	*(u32 *)(void *)&address = *(u16 *)(void *)table_ptr;
+	address <<= 4;
+	table_ptr = (u8 *)(long)address;
+
+	/*
+	 * Search EBDA paragraphs (EBDA is required to be a minimum of
+	 * 1K length)
+	 */
+	if (address > 0x400) {
+		rsdp = scan_mem_for_rsdp(table_ptr, ACPI_EBDA_WINDOW_SIZE);
+		if (rsdp) {
+			address += (u32)ACPI_PTR_DIFF(rsdp, table_ptr);
+			return (acpi_physical_address)address;
+		}
+	}
+
+	table_ptr = (u8 *)ACPI_HI_RSDP_WINDOW_BASE;
+	rsdp = scan_mem_for_rsdp(table_ptr, ACPI_HI_RSDP_WINDOW_SIZE);
+
+	/* Search upper memory: 16-byte boundaries in E0000h-FFFFFh */
+	if (rsdp) {
+		address = (u32)(ACPI_HI_RSDP_WINDOW_BASE +
+				ACPI_PTR_DIFF(rsdp, table_ptr));
+		return (acpi_physical_address)address;
+	}
+}
-- 
2.19.1

[PATCH v12 4/5] x86/boot: Parse SRAT table from RSDP and store immovable memory

[Chao Fan]

To fix the conflict between KASLR and memory-hotremove, SRAT table
should be parsed by RSDP pointer, then find the immovable
memory regions and store them in an array called immovable_mem[].
The array called immovable_mem[] will extern to KASLR, then
KASLR will avoid to extract kernel to these regions.

Add 'CONFIG_EARLY_PARSE_RSDP' which depends on RANDOMIZE_BASE &&
MEMORY_HOTREMOVE, cause only when both KASLR and memory-hotremove
are enabled, RSDP needs to be parsed in compressed period.

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
---
 arch/x86/Kconfig                  |  10 +++
 arch/x86/boot/compressed/Makefile |   2 +
 arch/x86/boot/compressed/acpitb.c | 125 ++++++++++++++++++++++++++++++
 arch/x86/boot/compressed/kaslr.c  |   4 -
 arch/x86/boot/compressed/misc.h   |  20 +++++
 5 files changed, 157 insertions(+), 4 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index a29d49ef4d56..bc775968557b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2146,6 +2146,16 @@ config X86_NEED_RELOCS
 	def_bool y
 	depends on RANDOMIZE_BASE || (X86_32 && RELOCATABLE)
 
+config CONFIG_EARLY_PARSE_RSDP
+	bool "Parse RSDP pointer on compressed period for KASLR"
+	def_bool n
+	depends on RANDOMIZE_BASE && MEMORY_HOTREMOVE
+	help
+	  This option parses RSDP pointer in compressed period. Works
+	  for KASLR to get memory information by SRAT table and choose
+	  immovable memory to extract kernel.
+	  Say Y if you want to use both KASLR and memory-hotremove.
+
 config PHYSICAL_ALIGN
 	hex "Alignment value to which kernel should be aligned"
 	default "0x200000"
diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 466f66c8a7f8..4cbfb58bf083 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -84,6 +84,8 @@ ifdef CONFIG_X86_64
 	vmlinux-objs-y += $(obj)/pgtable_64.o
 endif
 
+vmlinux-objs-$(CONFIG_EARLY_PARSE_RSDP) += $(obj)/acpitb.o
+
 $(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone
 
 vmlinux-objs-$(CONFIG_EFI_STUB) += $(obj)/eboot.o $(obj)/efi_stub_$(BITS).o \
diff --git a/arch/x86/boot/compressed/acpitb.c b/arch/x86/boot/compressed/acpitb.c
index 82d27c4b8978..023b33d0cd3b 100644
--- a/arch/x86/boot/compressed/acpitb.c
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -195,3 +195,128 @@ static acpi_physical_address bios_get_rsdp_addr(void)
 		return (acpi_physical_address)address;
 	}
 }
+
+/* Used to determine RSDP table, based on acpi_os_get_root_pointer(). */
+static acpi_physical_address get_rsdp_addr(void)
+{
+	acpi_physical_address pa = 0;
+
+	pa = get_acpi_rsdp();
+
+	if (!pa)
+		pa = efi_get_rsdp_addr();
+
+	if (!pa)
+		pa = bios_get_rsdp_addr();
+
+	return pa;
+}
+
+/* Compute SRAT table from RSDP. */
+static struct acpi_table_header *get_acpi_srat_table(void)
+{
+	acpi_physical_address acpi_table;
+	acpi_physical_address root_table;
+	struct acpi_table_header *header;
+	struct acpi_table_rsdp *rsdp;
+	int num_entries;
+	char arg[10];
+	u8 *entry;
+	u32 size;
+	u32 len;
+
+	rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
+	if (!rsdp)
+		return NULL;
+
+	/* Get RSDT or XSDT from RSDP. */
+	if (!(cmdline_find_option("acpi", arg, sizeof(arg)) == 4 &&
+	    !strncmp(arg, "rsdt", 4)) &&
+	    rsdp->xsdt_physical_address &&
+	    rsdp->revision > 1) {
+		root_table = rsdp->xsdt_physical_address;
+		size = ACPI_XSDT_ENTRY_SIZE;
+	} else {
+		root_table = rsdp->rsdt_physical_address;
+		size = ACPI_RSDT_ENTRY_SIZE;
+	}
+
+	/* Get ACPI root table from RSDT or XSDT.*/
+	header = (struct acpi_table_header *)root_table;
+	if (!header)
+		return NULL;
+
+	len = header->length;
+	num_entries = (u32)((len - sizeof(struct acpi_table_header)) / size);
+	if (num_entries > MAX_ACPI_SIG)
+		return NULL;
+
+	entry = ACPI_ADD_PTR(u8, header, sizeof(struct acpi_table_header));
+
+	while (num_entries--) {
+		u64 address64;
+
+		if (size == ACPI_RSDT_ENTRY_SIZE)
+			acpi_table = ((acpi_physical_address)
+				      (*ACPI_CAST_PTR(u32, entry)));
+		else {
+			*(u64 *)(void *)&address64 = *(u64 *)(void *)entry;
+			acpi_table = (acpi_physical_address) address64;
+		}
+
+		if (acpi_table) {
+			header = (struct acpi_table_header *)acpi_table;
+
+			if (ACPI_COMPARE_NAME(header->signature, ACPI_SIG_SRAT))
+				return header;
+		}
+		entry += size;
+	}
+	return NULL;
+}
+
+/*
+ * According to ACPI table, filter the immovable memory regions
+ * and store them in immovable_mem[].
+ */
+void get_immovable_mem(void)
+{
+	struct acpi_table_header *table_header;
+	struct acpi_subtable_header *table;
+	struct acpi_srat_mem_affinity *ma;
+	unsigned long table_end;
+	char arg[10];
+	int i = 0;
+
+	if (cmdline_find_option("acpi", arg, sizeof(arg)) == 3 &&
+	    !strncmp(arg, "off", 3))
+		return;
+
+	table_header = get_acpi_srat_table();
+	if (!table_header)
+		return;
+
+	table_end = (unsigned long)table_header + table_header->length;
+	table = (struct acpi_subtable_header *)
+		((unsigned long)table_header + sizeof(struct acpi_table_srat));
+
+	while (((unsigned long)table) +
+		       sizeof(struct acpi_subtable_header) < table_end) {
+		if (table->type == ACPI_SRAT_TYPE_MEMORY_AFFINITY) {
+			ma = (struct acpi_srat_mem_affinity *)table;
+			if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)) {
+				immovable_mem[i].start = ma->base_address;
+				immovable_mem[i].size = ma->length;
+				i++;
+			}
+
+			if (i >= MAX_NUMNODES*2) {
+				debug_putstr("Too many immovable memory regions, aborting.\n");
+				return;
+			}
+		}
+		table = (struct acpi_subtable_header *)
+			((unsigned long)table + table->length);
+	}
+	num_immovable_mem = i;
+}
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 9ed9709d9947..b251572e77af 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -87,10 +87,6 @@ static unsigned long get_boot_seed(void)
 #define KASLR_COMPRESSED_BOOT
 #include "../../lib/kaslr.c"
 
-struct mem_vector {
-	unsigned long long start;
-	unsigned long long size;
-};
 
 /* Only supporting at most 4 unusable memmap regions with kaslr */
 #define MAX_MEMMAP_REGIONS	4
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 809c31effa4b..d94e2a419c13 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -77,6 +77,11 @@ void choose_random_location(unsigned long input,
 			    unsigned long *output,
 			    unsigned long output_size,
 			    unsigned long *virt_addr);
+struct mem_vector {
+	unsigned long long start;
+	unsigned long long size;
+};
+
 /* cpuflags.c */
 bool has_cpuflag(int flag);
 #else
@@ -118,5 +123,20 @@ void set_sev_encryption_mask(void);
 #endif
 
 /* acpitb.c */
+#ifdef CONFIG_RANDOMIZE_BASE
+/* Store the amount of immovable memory regions */
+int num_immovable_mem;
+#endif
+
+#ifdef CONFIG_EARLY_PARSE_RSDP
+void get_immovable_mem(void);
+/* There are 72 kinds of ACPI_SIG in head file of ACPI. */
+#define MAX_ACPI_SIG	72
+#else
+static void get_immovable_mem(void)
+{
+}
+#endif
+
 #define BOOT_STRING
 extern int kstrtoull(const char *s, unsigned int base, unsigned long long *res);
-- 
2.19.1

Re: [PATCH v12 4/5] x86/boot: Parse SRAT table from RSDP and store immovable memory

[Masayoshi Mizuma]

On Thu, Nov 29, 2018 at 04:16:30PM +0800, Chao Fan wrote:
> To fix the conflict between KASLR and memory-hotremove, SRAT table
> should be parsed by RSDP pointer, then find the immovable
> memory regions and store them in an array called immovable_mem[].
> The array called immovable_mem[] will extern to KASLR, then
> KASLR will avoid to extract kernel to these regions.
> 
> Add 'CONFIG_EARLY_PARSE_RSDP' which depends on RANDOMIZE_BASE &&
> MEMORY_HOTREMOVE, cause only when both KASLR and memory-hotremove
> are enabled, RSDP needs to be parsed in compressed period.
> 
> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> ---
>  arch/x86/Kconfig                  |  10 +++
>  arch/x86/boot/compressed/Makefile |   2 +
>  arch/x86/boot/compressed/acpitb.c | 125 ++++++++++++++++++++++++++++++
>  arch/x86/boot/compressed/kaslr.c  |   4 -
>  arch/x86/boot/compressed/misc.h   |  20 +++++
>  5 files changed, 157 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index a29d49ef4d56..bc775968557b 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2146,6 +2146,16 @@ config X86_NEED_RELOCS
>  	def_bool y
>  	depends on RANDOMIZE_BASE || (X86_32 && RELOCATABLE)
>  

> +config CONFIG_EARLY_PARSE_RSDP

config EARLY_PARSE_RSDP

> +	bool "Parse RSDP pointer on compressed period for KASLR"
> +	def_bool n

Should be def_bool y?
It is better to enable EARLY_PARSE_RSDP by default if 
RANDOMIZE_BASE and MEMORY_HOTREMOVE are enabled.

> +	depends on RANDOMIZE_BASE && MEMORY_HOTREMOVE
> +	help
> +	  This option parses RSDP pointer in compressed period. Works
> +	  for KASLR to get memory information by SRAT table and choose
> +	  immovable memory to extract kernel.
> +	  Say Y if you want to use both KASLR and memory-hotremove.
> +
>  config PHYSICAL_ALIGN
>  	hex "Alignment value to which kernel should be aligned"
>  	default "0x200000"
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index 466f66c8a7f8..4cbfb58bf083 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -84,6 +84,8 @@ ifdef CONFIG_X86_64
>  	vmlinux-objs-y += $(obj)/pgtable_64.o
>  endif
>  
> +vmlinux-objs-$(CONFIG_EARLY_PARSE_RSDP) += $(obj)/acpitb.o
> +
>  $(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone
>  
>  vmlinux-objs-$(CONFIG_EFI_STUB) += $(obj)/eboot.o $(obj)/efi_stub_$(BITS).o \
> diff --git a/arch/x86/boot/compressed/acpitb.c b/arch/x86/boot/compressed/acpitb.c
> index 82d27c4b8978..023b33d0cd3b 100644
> --- a/arch/x86/boot/compressed/acpitb.c
> +++ b/arch/x86/boot/compressed/acpitb.c
> @@ -195,3 +195,128 @@ static acpi_physical_address bios_get_rsdp_addr(void)
>  		return (acpi_physical_address)address;
>  	}
>  }
> +
> +/* Used to determine RSDP table, based on acpi_os_get_root_pointer(). */
> +static acpi_physical_address get_rsdp_addr(void)
> +{
> +	acpi_physical_address pa = 0;
> +
> +	pa = get_acpi_rsdp();
> +
> +	if (!pa)
> +		pa = efi_get_rsdp_addr();
> +
> +	if (!pa)
> +		pa = bios_get_rsdp_addr();
> +
> +	return pa;
> +}
> +
> +/* Compute SRAT table from RSDP. */
> +static struct acpi_table_header *get_acpi_srat_table(void)
> +{
> +	acpi_physical_address acpi_table;
> +	acpi_physical_address root_table;
> +	struct acpi_table_header *header;
> +	struct acpi_table_rsdp *rsdp;
> +	int num_entries;
> +	char arg[10];
> +	u8 *entry;
> +	u32 size;
> +	u32 len;
> +
> +	rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
> +	if (!rsdp)
> +		return NULL;
> +
> +	/* Get RSDT or XSDT from RSDP. */

> +	if (!(cmdline_find_option("acpi", arg, sizeof(arg)) == 4 &&
> +	    !strncmp(arg, "rsdt", 4)) &&
> +	    rsdp->xsdt_physical_address &&
> +	    rsdp->revision > 1) {
> +		root_table = rsdp->xsdt_physical_address;
> +		size = ACPI_XSDT_ENTRY_SIZE;
> +	} else {
> +		root_table = rsdp->rsdt_physical_address;
> +		size = ACPI_RSDT_ENTRY_SIZE;
> +	}
> +
> +	/* Get ACPI root table from RSDT or XSDT.*/
> +	header = (struct acpi_table_header *)root_table;
> +	if (!header)
> +		return NULL;
> +
> +	len = header->length;
> +	num_entries = (u32)((len - sizeof(struct acpi_table_header)) / size);

> +	if (num_entries > MAX_ACPI_SIG)
> +		return NULL;

I think this check isn't needed...

> +
> +	entry = ACPI_ADD_PTR(u8, header, sizeof(struct acpi_table_header));
> +
> +	while (num_entries--) {
> +		u64 address64;
> +
> +		if (size == ACPI_RSDT_ENTRY_SIZE)
> +			acpi_table = ((acpi_physical_address)
> +				      (*ACPI_CAST_PTR(u32, entry)));
> +		else {
> +			*(u64 *)(void *)&address64 = *(u64 *)(void *)entry;
> +			acpi_table = (acpi_physical_address) address64;
> +		}
> +
> +		if (acpi_table) {
> +			header = (struct acpi_table_header *)acpi_table;
> +
> +			if (ACPI_COMPARE_NAME(header->signature, ACPI_SIG_SRAT))
> +				return header;
> +		}
> +		entry += size;
> +	}
> +	return NULL;
> +}
> +
> +/*
> + * According to ACPI table, filter the immovable memory regions
> + * and store them in immovable_mem[].
> + */
> +void get_immovable_mem(void)
> +{
> +	struct acpi_table_header *table_header;
> +	struct acpi_subtable_header *table;
> +	struct acpi_srat_mem_affinity *ma;
> +	unsigned long table_end;
> +	char arg[10];
> +	int i = 0;
> +
> +	if (cmdline_find_option("acpi", arg, sizeof(arg)) == 3 &&
> +	    !strncmp(arg, "off", 3))
> +		return;
> +
> +	table_header = get_acpi_srat_table();
> +	if (!table_header)
> +		return;
> +
> +	table_end = (unsigned long)table_header + table_header->length;
> +	table = (struct acpi_subtable_header *)
> +		((unsigned long)table_header + sizeof(struct acpi_table_srat));
> +
> +	while (((unsigned long)table) +
> +		       sizeof(struct acpi_subtable_header) < table_end) {
> +		if (table->type == ACPI_SRAT_TYPE_MEMORY_AFFINITY) {
> +			ma = (struct acpi_srat_mem_affinity *)table;
> +			if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)) {
> +				immovable_mem[i].start = ma->base_address;
> +				immovable_mem[i].size = ma->length;
> +				i++;
> +			}
> +
> +			if (i >= MAX_NUMNODES*2) {
> +				debug_putstr("Too many immovable memory regions, aborting.\n");
> +				return;
> +			}
> +		}
> +		table = (struct acpi_subtable_header *)
> +			((unsigned long)table + table->length);
> +	}
> +	num_immovable_mem = i;
> +}
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 9ed9709d9947..b251572e77af 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -87,10 +87,6 @@ static unsigned long get_boot_seed(void)
>  #define KASLR_COMPRESSED_BOOT
>  #include "../../lib/kaslr.c"
>  
> -struct mem_vector {
> -	unsigned long long start;
> -	unsigned long long size;
> -};
>  
>  /* Only supporting at most 4 unusable memmap regions with kaslr */
>  #define MAX_MEMMAP_REGIONS	4
> diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> index 809c31effa4b..d94e2a419c13 100644
> --- a/arch/x86/boot/compressed/misc.h
> +++ b/arch/x86/boot/compressed/misc.h
> @@ -77,6 +77,11 @@ void choose_random_location(unsigned long input,
>  			    unsigned long *output,
>  			    unsigned long output_size,
>  			    unsigned long *virt_addr);
> +struct mem_vector {
> +	unsigned long long start;
> +	unsigned long long size;
> +};
> +
>  /* cpuflags.c */
>  bool has_cpuflag(int flag);
>  #else
> @@ -118,5 +123,20 @@ void set_sev_encryption_mask(void);
>  #endif
>  
>  /* acpitb.c */
> +#ifdef CONFIG_RANDOMIZE_BASE
> +/* Store the amount of immovable memory regions */
> +int num_immovable_mem;
> +#endif
> +
> +#ifdef CONFIG_EARLY_PARSE_RSDP
> +void get_immovable_mem(void);

> +/* There are 72 kinds of ACPI_SIG in head file of ACPI. */
> +#define MAX_ACPI_SIG	72

The 72 isn't the specification of ACPI, right? So the number
of SIG may increase and decrease in the future.
This macro and the check I commented above isn't needed...

Thanks,
Masa

> +#else
> +static void get_immovable_mem(void)
> +{
> +}
> +#endif
> +
>  #define BOOT_STRING
>  extern int kstrtoull(const char *s, unsigned int base, unsigned long long *res);
> -- 
> 2.19.1
> 
> 
> 

Re: [PATCH v12 4/5] x86/boot: Parse SRAT table from RSDP and store immovable memory

[Chao Fan]

On Thu, Nov 29, 2018 at 12:55:21PM -0500, Masayoshi Mizuma wrote:
>On Thu, Nov 29, 2018 at 04:16:30PM +0800, Chao Fan wrote:
>> To fix the conflict between KASLR and memory-hotremove, SRAT table
>> should be parsed by RSDP pointer, then find the immovable
>> memory regions and store them in an array called immovable_mem[].
>> The array called immovable_mem[] will extern to KASLR, then
>> KASLR will avoid to extract kernel to these regions.
>> 
>> Add 'CONFIG_EARLY_PARSE_RSDP' which depends on RANDOMIZE_BASE &&
>> MEMORY_HOTREMOVE, cause only when both KASLR and memory-hotremove
>> are enabled, RSDP needs to be parsed in compressed period.
>> 
>> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
>> ---
>>  arch/x86/Kconfig                  |  10 +++
>>  arch/x86/boot/compressed/Makefile |   2 +
>>  arch/x86/boot/compressed/acpitb.c | 125 ++++++++++++++++++++++++++++++
>>  arch/x86/boot/compressed/kaslr.c  |   4 -
>>  arch/x86/boot/compressed/misc.h   |  20 +++++
>>  5 files changed, 157 insertions(+), 4 deletions(-)
>> 
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index a29d49ef4d56..bc775968557b 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -2146,6 +2146,16 @@ config X86_NEED_RELOCS
>>  	def_bool y
>>  	depends on RANDOMIZE_BASE || (X86_32 && RELOCATABLE)
>>  
>
>> +config CONFIG_EARLY_PARSE_RSDP
>
>config EARLY_PARSE_RSDP
>
>> +	bool "Parse RSDP pointer on compressed period for KASLR"
>> +	def_bool n
>
>Should be def_bool y?

I will change it to y.

>It is better to enable EARLY_PARSE_RSDP by default if 
>RANDOMIZE_BASE and MEMORY_HOTREMOVE are enabled.
>
>> +	depends on RANDOMIZE_BASE && MEMORY_HOTREMOVE
>> +	help
>> +	  This option parses RSDP pointer in compressed period. Works
>> +	  for KASLR to get memory information by SRAT table and choose
>> +	  immovable memory to extract kernel.
>> +	  Say Y if you want to use both KASLR and memory-hotremove.
>> +
>>  config PHYSICAL_ALIGN
>>  	hex "Alignment value to which kernel should be aligned"
>>  	default "0x200000"
>> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
>> index 466f66c8a7f8..4cbfb58bf083 100644
>> --- a/arch/x86/boot/compressed/Makefile
>> +++ b/arch/x86/boot/compressed/Makefile
>> @@ -84,6 +84,8 @@ ifdef CONFIG_X86_64
>>  	vmlinux-objs-y += $(obj)/pgtable_64.o
>>  endif
>>  
>> +vmlinux-objs-$(CONFIG_EARLY_PARSE_RSDP) += $(obj)/acpitb.o
>> +
>>  $(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone
>>  
>>  vmlinux-objs-$(CONFIG_EFI_STUB) += $(obj)/eboot.o $(obj)/efi_stub_$(BITS).o \
>> diff --git a/arch/x86/boot/compressed/acpitb.c b/arch/x86/boot/compressed/acpitb.c
>> index 82d27c4b8978..023b33d0cd3b 100644
>> --- a/arch/x86/boot/compressed/acpitb.c
>> +++ b/arch/x86/boot/compressed/acpitb.c
>> @@ -195,3 +195,128 @@ static acpi_physical_address bios_get_rsdp_addr(void)
>>  		return (acpi_physical_address)address;
>>  	}
>>  }
>> +
>> +/* Used to determine RSDP table, based on acpi_os_get_root_pointer(). */
>> +static acpi_physical_address get_rsdp_addr(void)
>> +{
>> +	acpi_physical_address pa = 0;
>> +
>> +	pa = get_acpi_rsdp();
>> +
>> +	if (!pa)
>> +		pa = efi_get_rsdp_addr();
>> +
>> +	if (!pa)
>> +		pa = bios_get_rsdp_addr();
>> +
>> +	return pa;
>> +}
>> +
>> +/* Compute SRAT table from RSDP. */
>> +static struct acpi_table_header *get_acpi_srat_table(void)
>> +{
>> +	acpi_physical_address acpi_table;
>> +	acpi_physical_address root_table;
>> +	struct acpi_table_header *header;
>> +	struct acpi_table_rsdp *rsdp;
>> +	int num_entries;
>> +	char arg[10];
>> +	u8 *entry;
>> +	u32 size;
>> +	u32 len;
>> +
>> +	rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
>> +	if (!rsdp)
>> +		return NULL;
>> +
>> +	/* Get RSDT or XSDT from RSDP. */
>
>> +	if (!(cmdline_find_option("acpi", arg, sizeof(arg)) == 4 &&
>> +	    !strncmp(arg, "rsdt", 4)) &&
>> +	    rsdp->xsdt_physical_address &&
>> +	    rsdp->revision > 1) {
>> +		root_table = rsdp->xsdt_physical_address;
>> +		size = ACPI_XSDT_ENTRY_SIZE;
>> +	} else {
>> +		root_table = rsdp->rsdt_physical_address;
>> +		size = ACPI_RSDT_ENTRY_SIZE;
>> +	}
>> +
>> +	/* Get ACPI root table from RSDT or XSDT.*/
>> +	header = (struct acpi_table_header *)root_table;
>> +	if (!header)
>> +		return NULL;
>> +
>> +	len = header->length;
>> +	num_entries = (u32)((len - sizeof(struct acpi_table_header)) / size);
>
>> +	if (num_entries > MAX_ACPI_SIG)
>> +		return NULL;
>
>I think this check isn't needed...
>
>> +
>> +	entry = ACPI_ADD_PTR(u8, header, sizeof(struct acpi_table_header));
>> +
>> +	while (num_entries--) {
>> +		u64 address64;
>> +
>> +		if (size == ACPI_RSDT_ENTRY_SIZE)
>> +			acpi_table = ((acpi_physical_address)
>> +				      (*ACPI_CAST_PTR(u32, entry)));
>> +		else {
>> +			*(u64 *)(void *)&address64 = *(u64 *)(void *)entry;
>> +			acpi_table = (acpi_physical_address) address64;
>> +		}
>> +
>> +		if (acpi_table) {
>> +			header = (struct acpi_table_header *)acpi_table;
>> +
>> +			if (ACPI_COMPARE_NAME(header->signature, ACPI_SIG_SRAT))
>> +				return header;
>> +		}
>> +		entry += size;
>> +	}
>> +	return NULL;
>> +}
>> +
>> +/*
>> + * According to ACPI table, filter the immovable memory regions
>> + * and store them in immovable_mem[].
>> + */
>> +void get_immovable_mem(void)
>> +{
>> +	struct acpi_table_header *table_header;
>> +	struct acpi_subtable_header *table;
>> +	struct acpi_srat_mem_affinity *ma;
>> +	unsigned long table_end;
>> +	char arg[10];
>> +	int i = 0;
>> +
>> +	if (cmdline_find_option("acpi", arg, sizeof(arg)) == 3 &&
>> +	    !strncmp(arg, "off", 3))
>> +		return;
>> +
>> +	table_header = get_acpi_srat_table();
>> +	if (!table_header)
>> +		return;
>> +
>> +	table_end = (unsigned long)table_header + table_header->length;
>> +	table = (struct acpi_subtable_header *)
>> +		((unsigned long)table_header + sizeof(struct acpi_table_srat));
>> +
>> +	while (((unsigned long)table) +
>> +		       sizeof(struct acpi_subtable_header) < table_end) {
>> +		if (table->type == ACPI_SRAT_TYPE_MEMORY_AFFINITY) {
>> +			ma = (struct acpi_srat_mem_affinity *)table;
>> +			if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)) {
>> +				immovable_mem[i].start = ma->base_address;
>> +				immovable_mem[i].size = ma->length;
>> +				i++;
>> +			}
>> +
>> +			if (i >= MAX_NUMNODES*2) {
>> +				debug_putstr("Too many immovable memory regions, aborting.\n");
>> +				return;
>> +			}
>> +		}
>> +		table = (struct acpi_subtable_header *)
>> +			((unsigned long)table + table->length);
>> +	}
>> +	num_immovable_mem = i;
>> +}
>> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
>> index 9ed9709d9947..b251572e77af 100644
>> --- a/arch/x86/boot/compressed/kaslr.c
>> +++ b/arch/x86/boot/compressed/kaslr.c
>> @@ -87,10 +87,6 @@ static unsigned long get_boot_seed(void)
>>  #define KASLR_COMPRESSED_BOOT
>>  #include "../../lib/kaslr.c"
>>  
>> -struct mem_vector {
>> -	unsigned long long start;
>> -	unsigned long long size;
>> -};
>>  
>>  /* Only supporting at most 4 unusable memmap regions with kaslr */
>>  #define MAX_MEMMAP_REGIONS	4
>> diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
>> index 809c31effa4b..d94e2a419c13 100644
>> --- a/arch/x86/boot/compressed/misc.h
>> +++ b/arch/x86/boot/compressed/misc.h
>> @@ -77,6 +77,11 @@ void choose_random_location(unsigned long input,
>>  			    unsigned long *output,
>>  			    unsigned long output_size,
>>  			    unsigned long *virt_addr);
>> +struct mem_vector {
>> +	unsigned long long start;
>> +	unsigned long long size;
>> +};
>> +
>>  /* cpuflags.c */
>>  bool has_cpuflag(int flag);
>>  #else
>> @@ -118,5 +123,20 @@ void set_sev_encryption_mask(void);
>>  #endif
>>  
>>  /* acpitb.c */
>> +#ifdef CONFIG_RANDOMIZE_BASE
>> +/* Store the amount of immovable memory regions */
>> +int num_immovable_mem;
>> +#endif
>> +
>> +#ifdef CONFIG_EARLY_PARSE_RSDP
>> +void get_immovable_mem(void);
>
>> +/* There are 72 kinds of ACPI_SIG in head file of ACPI. */
>> +#define MAX_ACPI_SIG	72
>
>The 72 isn't the specification of ACPI, right? So the number

Yes, it's from  ACPI code, include/acpi/actbl*h.
Boris said there should be a check for the num_entries,
I didn't get a good idea, so I use the max number to check it.
So do you have some advice?

Thanks,
Chao Fan

>of SIG may increase and decrease in the future.
>This macro and the check I commented above isn't needed...
>
>Thanks,
>Masa
>
>> +#else
>> +static void get_immovable_mem(void)
>> +{
>> +}
>> +#endif
>> +
>>  #define BOOT_STRING
>>  extern int kstrtoull(const char *s, unsigned int base, unsigned long long *res);
>> -- 
>> 2.19.1
>> 
>> 
>> 
>
>

Re: [PATCH v12 4/5] x86/boot: Parse SRAT table from RSDP and store immovable memory

[Masayoshi Mizuma]

On Fri, Nov 30, 2018 at 09:24:54AM +0800, Chao Fan wrote:
> On Thu, Nov 29, 2018 at 12:55:21PM -0500, Masayoshi Mizuma wrote:
> >On Thu, Nov 29, 2018 at 04:16:30PM +0800, Chao Fan wrote:
> >> To fix the conflict between KASLR and memory-hotremove, SRAT table
> >> should be parsed by RSDP pointer, then find the immovable
> >> memory regions and store them in an array called immovable_mem[].
> >> The array called immovable_mem[] will extern to KASLR, then
> >> KASLR will avoid to extract kernel to these regions.
> >> 
> >> Add 'CONFIG_EARLY_PARSE_RSDP' which depends on RANDOMIZE_BASE &&
> >> MEMORY_HOTREMOVE, cause only when both KASLR and memory-hotremove
> >> are enabled, RSDP needs to be parsed in compressed period.
> >> 
> >> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>
> >> ---
> >>  arch/x86/Kconfig                  |  10 +++
> >>  arch/x86/boot/compressed/Makefile |   2 +
> >>  arch/x86/boot/compressed/acpitb.c | 125 ++++++++++++++++++++++++++++++
> >>  arch/x86/boot/compressed/kaslr.c  |   4 -
> >>  arch/x86/boot/compressed/misc.h   |  20 +++++
> >>  5 files changed, 157 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> >> index a29d49ef4d56..bc775968557b 100644
> >> --- a/arch/x86/Kconfig
> >> +++ b/arch/x86/Kconfig
> >> @@ -2146,6 +2146,16 @@ config X86_NEED_RELOCS
> >>  	def_bool y
> >>  	depends on RANDOMIZE_BASE || (X86_32 && RELOCATABLE)
> >>  
> >
> >> +config CONFIG_EARLY_PARSE_RSDP
> >
> >config EARLY_PARSE_RSDP
> >
> >> +	bool "Parse RSDP pointer on compressed period for KASLR"
> >> +	def_bool n
> >
> >Should be def_bool y?
> 
> I will change it to y.
> 
> >It is better to enable EARLY_PARSE_RSDP by default if 
> >RANDOMIZE_BASE and MEMORY_HOTREMOVE are enabled.
> >
> >> +	depends on RANDOMIZE_BASE && MEMORY_HOTREMOVE
> >> +	help
> >> +	  This option parses RSDP pointer in compressed period. Works
> >> +	  for KASLR to get memory information by SRAT table and choose
> >> +	  immovable memory to extract kernel.
> >> +	  Say Y if you want to use both KASLR and memory-hotremove.
> >> +
> >>  config PHYSICAL_ALIGN
> >>  	hex "Alignment value to which kernel should be aligned"
> >>  	default "0x200000"
> >> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> >> index 466f66c8a7f8..4cbfb58bf083 100644
> >> --- a/arch/x86/boot/compressed/Makefile
> >> +++ b/arch/x86/boot/compressed/Makefile
> >> @@ -84,6 +84,8 @@ ifdef CONFIG_X86_64
> >>  	vmlinux-objs-y += $(obj)/pgtable_64.o
> >>  endif
> >>  
> >> +vmlinux-objs-$(CONFIG_EARLY_PARSE_RSDP) += $(obj)/acpitb.o
> >> +
> >>  $(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone
> >>  
> >>  vmlinux-objs-$(CONFIG_EFI_STUB) += $(obj)/eboot.o $(obj)/efi_stub_$(BITS).o \
> >> diff --git a/arch/x86/boot/compressed/acpitb.c b/arch/x86/boot/compressed/acpitb.c
> >> index 82d27c4b8978..023b33d0cd3b 100644
> >> --- a/arch/x86/boot/compressed/acpitb.c
> >> +++ b/arch/x86/boot/compressed/acpitb.c
> >> @@ -195,3 +195,128 @@ static acpi_physical_address bios_get_rsdp_addr(void)
> >>  		return (acpi_physical_address)address;
> >>  	}
> >>  }
> >> +
> >> +/* Used to determine RSDP table, based on acpi_os_get_root_pointer(). */
> >> +static acpi_physical_address get_rsdp_addr(void)
> >> +{
> >> +	acpi_physical_address pa = 0;
> >> +
> >> +	pa = get_acpi_rsdp();
> >> +
> >> +	if (!pa)
> >> +		pa = efi_get_rsdp_addr();
> >> +
> >> +	if (!pa)
> >> +		pa = bios_get_rsdp_addr();
> >> +
> >> +	return pa;
> >> +}
> >> +
> >> +/* Compute SRAT table from RSDP. */
> >> +static struct acpi_table_header *get_acpi_srat_table(void)
> >> +{
> >> +	acpi_physical_address acpi_table;
> >> +	acpi_physical_address root_table;
> >> +	struct acpi_table_header *header;
> >> +	struct acpi_table_rsdp *rsdp;
> >> +	int num_entries;
> >> +	char arg[10];
> >> +	u8 *entry;
> >> +	u32 size;
> >> +	u32 len;
> >> +
> >> +	rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
> >> +	if (!rsdp)
> >> +		return NULL;
> >> +
> >> +	/* Get RSDT or XSDT from RSDP. */
> >
> >> +	if (!(cmdline_find_option("acpi", arg, sizeof(arg)) == 4 &&
> >> +	    !strncmp(arg, "rsdt", 4)) &&
> >> +	    rsdp->xsdt_physical_address &&
> >> +	    rsdp->revision > 1) {
> >> +		root_table = rsdp->xsdt_physical_address;
> >> +		size = ACPI_XSDT_ENTRY_SIZE;
> >> +	} else {
> >> +		root_table = rsdp->rsdt_physical_address;
> >> +		size = ACPI_RSDT_ENTRY_SIZE;
> >> +	}
> >> +
> >> +	/* Get ACPI root table from RSDT or XSDT.*/
> >> +	header = (struct acpi_table_header *)root_table;
> >> +	if (!header)
> >> +		return NULL;
> >> +
> >> +	len = header->length;
> >> +	num_entries = (u32)((len - sizeof(struct acpi_table_header)) / size);
> >
> >> +	if (num_entries > MAX_ACPI_SIG)
> >> +		return NULL;
> >
> >I think this check isn't needed...
> >
> >> +
> >> +	entry = ACPI_ADD_PTR(u8, header, sizeof(struct acpi_table_header));
> >> +
> >> +	while (num_entries--) {
> >> +		u64 address64;
> >> +
> >> +		if (size == ACPI_RSDT_ENTRY_SIZE)
> >> +			acpi_table = ((acpi_physical_address)
> >> +				      (*ACPI_CAST_PTR(u32, entry)));
> >> +		else {
> >> +			*(u64 *)(void *)&address64 = *(u64 *)(void *)entry;
> >> +			acpi_table = (acpi_physical_address) address64;
> >> +		}
> >> +
> >> +		if (acpi_table) {
> >> +			header = (struct acpi_table_header *)acpi_table;
> >> +
> >> +			if (ACPI_COMPARE_NAME(header->signature, ACPI_SIG_SRAT))
> >> +				return header;
> >> +		}
> >> +		entry += size;
> >> +	}
> >> +	return NULL;
> >> +}
> >> +
> >> +/*
> >> + * According to ACPI table, filter the immovable memory regions
> >> + * and store them in immovable_mem[].
> >> + */
> >> +void get_immovable_mem(void)
> >> +{
> >> +	struct acpi_table_header *table_header;
> >> +	struct acpi_subtable_header *table;
> >> +	struct acpi_srat_mem_affinity *ma;
> >> +	unsigned long table_end;
> >> +	char arg[10];
> >> +	int i = 0;
> >> +
> >> +	if (cmdline_find_option("acpi", arg, sizeof(arg)) == 3 &&
> >> +	    !strncmp(arg, "off", 3))
> >> +		return;
> >> +
> >> +	table_header = get_acpi_srat_table();
> >> +	if (!table_header)
> >> +		return;
> >> +
> >> +	table_end = (unsigned long)table_header + table_header->length;
> >> +	table = (struct acpi_subtable_header *)
> >> +		((unsigned long)table_header + sizeof(struct acpi_table_srat));
> >> +
> >> +	while (((unsigned long)table) +
> >> +		       sizeof(struct acpi_subtable_header) < table_end) {
> >> +		if (table->type == ACPI_SRAT_TYPE_MEMORY_AFFINITY) {
> >> +			ma = (struct acpi_srat_mem_affinity *)table;
> >> +			if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)) {
> >> +				immovable_mem[i].start = ma->base_address;
> >> +				immovable_mem[i].size = ma->length;
> >> +				i++;
> >> +			}
> >> +
> >> +			if (i >= MAX_NUMNODES*2) {
> >> +				debug_putstr("Too many immovable memory regions, aborting.\n");
> >> +				return;
> >> +			}
> >> +		}
> >> +		table = (struct acpi_subtable_header *)
> >> +			((unsigned long)table + table->length);
> >> +	}
> >> +	num_immovable_mem = i;
> >> +}
> >> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> >> index 9ed9709d9947..b251572e77af 100644
> >> --- a/arch/x86/boot/compressed/kaslr.c
> >> +++ b/arch/x86/boot/compressed/kaslr.c
> >> @@ -87,10 +87,6 @@ static unsigned long get_boot_seed(void)
> >>  #define KASLR_COMPRESSED_BOOT
> >>  #include "../../lib/kaslr.c"
> >>  
> >> -struct mem_vector {
> >> -	unsigned long long start;
> >> -	unsigned long long size;
> >> -};
> >>  
> >>  /* Only supporting at most 4 unusable memmap regions with kaslr */
> >>  #define MAX_MEMMAP_REGIONS	4
> >> diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> >> index 809c31effa4b..d94e2a419c13 100644
> >> --- a/arch/x86/boot/compressed/misc.h
> >> +++ b/arch/x86/boot/compressed/misc.h
> >> @@ -77,6 +77,11 @@ void choose_random_location(unsigned long input,
> >>  			    unsigned long *output,
> >>  			    unsigned long output_size,
> >>  			    unsigned long *virt_addr);
> >> +struct mem_vector {
> >> +	unsigned long long start;
> >> +	unsigned long long size;
> >> +};
> >> +
> >>  /* cpuflags.c */
> >>  bool has_cpuflag(int flag);
> >>  #else
> >> @@ -118,5 +123,20 @@ void set_sev_encryption_mask(void);
> >>  #endif
> >>  
> >>  /* acpitb.c */
> >> +#ifdef CONFIG_RANDOMIZE_BASE
> >> +/* Store the amount of immovable memory regions */
> >> +int num_immovable_mem;
> >> +#endif
> >> +
> >> +#ifdef CONFIG_EARLY_PARSE_RSDP
> >> +void get_immovable_mem(void);
> >
> >> +/* There are 72 kinds of ACPI_SIG in head file of ACPI. */
> >> +#define MAX_ACPI_SIG	72
> >
> >The 72 isn't the specification of ACPI, right? So the number
> 
> Yes, it's from  ACPI code, include/acpi/actbl*h.
> Boris said there should be a check for the num_entries,
> I didn't get a good idea, so I use the max number to check it.
> So do you have some advice?

Ah, got it. How about adding the check for len to prevent the wrap?
Like as:

	len = header->length;
	if (len <= sizeof(struct acpi_table_header))
		return NULL;
 	num_entries = (u32)((len - sizeof(struct acpi_table_header)) / size);

Thanks,
Masa

Re: [PATCH v12 4/5] x86/boot: Parse SRAT table from RSDP and store immovable memory

[Chao Fan]

On Fri, Nov 30, 2018 at 09:54:33AM -0500, Masayoshi Mizuma wrote:
>On Fri, Nov 30, 2018 at 09:24:54AM +0800, Chao Fan wrote:
>> >>  /* acpitb.c */
>> >> +#ifdef CONFIG_RANDOMIZE_BASE
>> >> +/* Store the amount of immovable memory regions */
>> >> +int num_immovable_mem;
>> >> +#endif
>> >> +
>> >> +#ifdef CONFIG_EARLY_PARSE_RSDP
>> >> +void get_immovable_mem(void);
>> >
>> >> +/* There are 72 kinds of ACPI_SIG in head file of ACPI. */
>> >> +#define MAX_ACPI_SIG	72
>> >
>> >The 72 isn't the specification of ACPI, right? So the number
>> 
>> Yes, it's from  ACPI code, include/acpi/actbl*h.
>> Boris said there should be a check for the num_entries,
>> I didn't get a good idea, so I use the max number to check it.
>> So do you have some advice?
>
>Ah, got it. How about adding the check for len to prevent the wrap?
>Like as:
>
>	len = header->length;
>	if (len <= sizeof(struct acpi_table_header))
>		return NULL;
> 	num_entries = (u32)((len - sizeof(struct acpi_table_header)) / size);

Hi Masa,

Your check is right, but not exactly the same.
I think what Boris said is to prevent num_entries from getting too large
and the loop getting too much.
So the 'num_entries' or 'len' can't be more than a fixed value.

Thanks,
Chao Fan

>
>Thanks,
>Masa
>
>

[PATCH v12 5/5] x86/boot/KASLR: Limit KASLR to extracting kernel in immovable memory

[Chao Fan]

KASLR may randomly choose some positions which are located in movable
memory regions. This will break memory hotplug feature and make the
movable memory chosen by KASLR practically immovable.

The solution is to limit KASLR to choose memory regions in immovable
node according to SRAT tables.
If CONFIG_EARLY_PARSE_RSDP is enabled, walk through the SRAT memory
tables and store those immovable memory regions so that KASLR can get
where to choose for randomization.

Also, rename process_mem_region() as __process_mem_region() and name new
function as process_mem_region().

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

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index b251572e77af..f0c30e3ddcb4 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -97,6 +97,11 @@ static bool memmap_too_large;
 /* Store memory limit specified by "mem=nn[KMG]" or "memmap=nn[KMG]" */
 static unsigned long long mem_limit = ULLONG_MAX;
 
+#ifdef CONFIG_MEMORY_HOTREMOVE
+/* Store the immovable memory regions */
+extern struct mem_vector immovable_mem[MAX_NUMNODES*2];
+#endif
+
 
 enum mem_avoid_index {
 	MEM_AVOID_ZO_RANGE = 0,
@@ -413,6 +418,9 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
 	/* Mark the memmap regions we need to avoid */
 	handle_mem_options();
 
+	/* Mark the immovable regions we need to choose */
+	get_immovable_mem();
+
 #ifdef CONFIG_X86_VERBOSE_BOOTUP
 	/* Make sure video RAM can be used. */
 	add_identity_map(0, PMD_SIZE);
@@ -568,9 +576,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 __process_mem_region(struct mem_vector *entry,
+				 unsigned long minimum,
+				 unsigned long image_size)
 {
 	struct mem_vector region, overlap;
 	unsigned long start_orig, end;
@@ -646,6 +654,57 @@ 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)
+{
+	int i;
+	/*
+	 * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
+	 * walk all the regions, so use region directly.
+	 */
+	if (!num_immovable_mem) {
+		__process_mem_region(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_EARLY_PARSE_RSDP
+	/*
+	 * If immovable memory found, filter the intersection between
+	 * immovable memory and region to __process_mem_region().
+	 * Otherwise, go on old code.
+	 */
+	for (i = 0; i < num_immovable_mem; i++) {
+		struct mem_vector entry;
+		unsigned long long start, end, entry_end, region_end;
+
+		if (!mem_overlaps(region, &immovable_mem[i]))
+			continue;
+
+		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);
+		entry.size = entry_end - entry.start;
+
+		__process_mem_region(&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;
+#endif
+}
+
 #ifdef CONFIG_EFI
 /*
  * Returns true if mirror region found (and must have been processed
@@ -711,11 +770,8 @@ 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;
 }
@@ -742,11 +798,8 @@ 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.19.1

Re: [PATCH v12 0/5] x86/boot/KASLR: Parse ACPI table and limit KASLR to choosing immovable memory

[Masayoshi Mizuma]

Hi Chao,

Thank you for your continued working.

Could you please build your patches before sending?
Your patches depend on the following kconfig,
so please build them under the config combination.

RANDOMIZE_BASE
MEMORY_HOTREMOVE
EARLY_PARSE_RSDP
KEXEC
EFI

Thanks,
Masa

On Thu, Nov 29, 2018 at 04:16:26PM +0800, Chao Fan wrote:
> ***Background:
> People reported that KASLR may randomly choose some positions
> which are located in movable memory regions. This will break memory
> hotplug feature and make the movable memory chosen by KASLR can't be
> removed.
> 
> ***Solutions:
> Get the information of memory hot-remove, then KASLR will know the
> right regions. Information about memory hot-remove is in ACPI
> tables, which will be parsed after start_kernel(), so that KASLR
> can't get the information.
> 
> Somebody suggest to add a kernel parameter to specify the
> immovable memory so that limit KASLR in these regions. Then I make
> a patchset. After several versions, Ingo gave a suggestion:
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1634024.html
> Follow Ingo's suggestion, imitate the ACPI code to parse the ACPI
> tables, so that the kaslr can get necessary memory information in
> ACPI tables.
> I think ACPI code is an independent part, so imitate the codes
> and functions to 'compressed/' directory, so that kaslr won't
> influence the initialization of ACPI.
> 
> PATCH 1/5 Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC
> PATCH 2/5 Add efi_get_rsdp_addr() to find RSDP from EFI table when
>           booting from EFI.
> PATCH 3/5 Add bios_get_rsdp_addr() to search RSDP in memory when EFI
>           table not found.
> PATCH 4/5 Compute SRAT table from RSDP and walk SRAT table to store
>           the immovable memory regions.
> PATCH 5/5 Calculate the intersection between memory regions from e820/efi
>           memory table and immovable memory regions. Limit KASLR to
>           choosing these regions for randomization.
> 
> v1->v2:
>  -  Simplify some code.
> Follow Baoquan He's suggestion:
>  - Reuse the head file of acpi code.
> 
> v2->v3:
>  - Test in more conditions, so remove the 'RFC' tag.
>  - Change some comments.
> 
> v3->v4:
> Follow Thomas Gleixner's suggetsion:
>  - Put the whole efi related function into #define CONFIG_EFI and return
>    false in the other stub.
> 
> v4->v5:
> Follow Dou Liyang's suggestion:
>  - Add more comments about some functions based on kernel code.
>  - Change some typo in comments.
>  - Clean useless variable.
>  - Add check for the boundary of array.
>  - Add check for 'movable_node' parameter
> 
> v5->v6:
> Follow Baoquan He's suggestion:
>  - Change some log.
>  - Add the check for acpi_rsdp
>  - Change some code logical to make code clear
> 
> v6->v7:
> Follow Rafael's suggestion:
>  - Add more comments and patch log.
> Follow test robot's suggestion:
>  - Add "static" tag for function
> 
> v7-v8:
> Follow Kees Cook's suggestion:
>  - Use mem_overlaps() to check memory region.
>  - Use #ifdef in the definition of function.
> 
> v8-v9:
> Follow Boris' suggestion:
>  - Change code style.
>  - Splite PATCH 1/3 to more path.
>  - Introduce some new function
>  - Use existing function to rework some code
> Follow Masayoshi's suggetion:
>  - Make code more readable
> 
> v9->v10:
> Follow Baoquan's suggestion:
>  - Change some log
>  - Merge last two patch together.
> 
> v10->v11:
> Follow Boris' suggestion:
>  - Link kstrtoull() instead of copying it.
>  - Drop the useless wrapped function.
> 
> v11->v12:
> Follow Boris' suggestion:
>  - Change patch log and code comments.
>  - Add 'CONFIG_EARLY_PARSE_RSDP' to make code easy to read
>  - Put strtoull() to misc.c
> Follow Masa's suggestion:
>  - Remove the detection for 'movable_node'
>  - Change the code logical about cmdline_find_option()
> 
> Any comments will be welcome.
> 
> 
> Chao Fan (5):
>   x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC
>   x86/boot: Add efi_get_rsdp_addr() to find RSDP from EFI table
>   x86/boot: Add bios_get_rsdp_addr() to search RSDP in memory
>   x86/boot: Parse SRAT table from RSDP and store immovable memory
>   x86/boot/KASLR: Limit KASLR to extracting kernel in immovable memory
> 
>  arch/x86/Kconfig                  |  10 +
>  arch/x86/boot/compressed/Makefile |   2 +
>  arch/x86/boot/compressed/acpitb.c | 322 ++++++++++++++++++++++++++++++
>  arch/x86/boot/compressed/kaslr.c  |  79 ++++++--
>  arch/x86/boot/compressed/misc.c   |   5 +
>  arch/x86/boot/compressed/misc.h   |  24 +++
>  lib/kstrtox.c                     |   5 +
>  7 files changed, 432 insertions(+), 15 deletions(-)
>  create mode 100644 arch/x86/boot/compressed/acpitb.c
> 
> -- 
> 2.19.1
> 
> 
> 

Re: [PATCH v12 0/5] x86/boot/KASLR: Parse ACPI table and limit KASLR to choosing immovable memory

[Chao Fan]

On Thu, Nov 29, 2018 at 12:32:46PM -0500, Masayoshi Mizuma wrote:
>Hi Chao,
>
>Thank you for your continued working.

Thanks for your test.

>
>Could you please build your patches before sending?

Sorry for the mistake, I build it with the whole patches.
I found there are some problems with the method to splite patch.
I will rework on it and build every commit.

Thanks,
Chao Fan

>Your patches depend on the following kconfig,
>so please build them under the config combination.
>
>RANDOMIZE_BASE
>MEMORY_HOTREMOVE
>EARLY_PARSE_RSDP
>KEXEC
>EFI
>
>Thanks,
>Masa
>
>On Thu, Nov 29, 2018 at 04:16:26PM +0800, Chao Fan wrote:
>> ***Background:
>> People reported that KASLR may randomly choose some positions
>> which are located in movable memory regions. This will break memory
>> hotplug feature and make the movable memory chosen by KASLR can't be
>> removed.
>> 
>> ***Solutions:
>> Get the information of memory hot-remove, then KASLR will know the
>> right regions. Information about memory hot-remove is in ACPI
>> tables, which will be parsed after start_kernel(), so that KASLR
>> can't get the information.
>> 
>> Somebody suggest to add a kernel parameter to specify the
>> immovable memory so that limit KASLR in these regions. Then I make
>> a patchset. After several versions, Ingo gave a suggestion:
>> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1634024.html
>> Follow Ingo's suggestion, imitate the ACPI code to parse the ACPI
>> tables, so that the kaslr can get necessary memory information in
>> ACPI tables.
>> I think ACPI code is an independent part, so imitate the codes
>> and functions to 'compressed/' directory, so that kaslr won't
>> influence the initialization of ACPI.
>> 
>> PATCH 1/5 Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC
>> PATCH 2/5 Add efi_get_rsdp_addr() to find RSDP from EFI table when
>>           booting from EFI.
>> PATCH 3/5 Add bios_get_rsdp_addr() to search RSDP in memory when EFI
>>           table not found.
>> PATCH 4/5 Compute SRAT table from RSDP and walk SRAT table to store
>>           the immovable memory regions.
>> PATCH 5/5 Calculate the intersection between memory regions from e820/efi
>>           memory table and immovable memory regions. Limit KASLR to
>>           choosing these regions for randomization.
>> 
>> v1->v2:
>>  -  Simplify some code.
>> Follow Baoquan He's suggestion:
>>  - Reuse the head file of acpi code.
>> 
>> v2->v3:
>>  - Test in more conditions, so remove the 'RFC' tag.
>>  - Change some comments.
>> 
>> v3->v4:
>> Follow Thomas Gleixner's suggetsion:
>>  - Put the whole efi related function into #define CONFIG_EFI and return
>>    false in the other stub.
>> 
>> v4->v5:
>> Follow Dou Liyang's suggestion:
>>  - Add more comments about some functions based on kernel code.
>>  - Change some typo in comments.
>>  - Clean useless variable.
>>  - Add check for the boundary of array.
>>  - Add check for 'movable_node' parameter
>> 
>> v5->v6:
>> Follow Baoquan He's suggestion:
>>  - Change some log.
>>  - Add the check for acpi_rsdp
>>  - Change some code logical to make code clear
>> 
>> v6->v7:
>> Follow Rafael's suggestion:
>>  - Add more comments and patch log.
>> Follow test robot's suggestion:
>>  - Add "static" tag for function
>> 
>> v7-v8:
>> Follow Kees Cook's suggestion:
>>  - Use mem_overlaps() to check memory region.
>>  - Use #ifdef in the definition of function.
>> 
>> v8-v9:
>> Follow Boris' suggestion:
>>  - Change code style.
>>  - Splite PATCH 1/3 to more path.
>>  - Introduce some new function
>>  - Use existing function to rework some code
>> Follow Masayoshi's suggetion:
>>  - Make code more readable
>> 
>> v9->v10:
>> Follow Baoquan's suggestion:
>>  - Change some log
>>  - Merge last two patch together.
>> 
>> v10->v11:
>> Follow Boris' suggestion:
>>  - Link kstrtoull() instead of copying it.
>>  - Drop the useless wrapped function.
>> 
>> v11->v12:
>> Follow Boris' suggestion:
>>  - Change patch log and code comments.
>>  - Add 'CONFIG_EARLY_PARSE_RSDP' to make code easy to read
>>  - Put strtoull() to misc.c
>> Follow Masa's suggestion:
>>  - Remove the detection for 'movable_node'
>>  - Change the code logical about cmdline_find_option()
>> 
>> Any comments will be welcome.
>> 
>> 
>> Chao Fan (5):
>>   x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC
>>   x86/boot: Add efi_get_rsdp_addr() to find RSDP from EFI table
>>   x86/boot: Add bios_get_rsdp_addr() to search RSDP in memory
>>   x86/boot: Parse SRAT table from RSDP and store immovable memory
>>   x86/boot/KASLR: Limit KASLR to extracting kernel in immovable memory
>> 
>>  arch/x86/Kconfig                  |  10 +
>>  arch/x86/boot/compressed/Makefile |   2 +
>>  arch/x86/boot/compressed/acpitb.c | 322 ++++++++++++++++++++++++++++++
>>  arch/x86/boot/compressed/kaslr.c  |  79 ++++++--
>>  arch/x86/boot/compressed/misc.c   |   5 +
>>  arch/x86/boot/compressed/misc.h   |  24 +++
>>  lib/kstrtox.c                     |   5 +
>>  7 files changed, 432 insertions(+), 15 deletions(-)
>>  create mode 100644 arch/x86/boot/compressed/acpitb.c
>> 
>> -- 
>> 2.19.1
>> 
>> 
>> 
>
>

Re: [PATCH v12 0/5] x86/boot/KASLR: Parse ACPI table and limit KASLR to choosing immovable memory

[Chao Fan]

On Fri, Nov 30, 2018 at 09:15:13AM +0800, Chao Fan wrote:
>On Thu, Nov 29, 2018 at 12:32:46PM -0500, Masayoshi Mizuma wrote:
>>Hi Chao,
>>
>>Thank you for your continued working.
>
>Thanks for your test.
>
>>
>>Could you please build your patches before sending?
>
>Sorry for the mistake, I build it with the whole patches.
>I found there are some problems with the method to splite patch.
>I will rework on it and build every commit.
>
>Thanks,
>Chao Fan
>
>>Your patches depend on the following kconfig,
>>so please build them under the config combination.
>>
>>RANDOMIZE_BASE
>>MEMORY_HOTREMOVE
>>EARLY_PARSE_RSDP
>>KEXEC
>>EFI

Hi Masa,

After retest, I make clean and make again.
I met the problem is the useless #endif issue you mentioned.
But I found both:

#ifndef BOOT_STRING
EXPORT_SYMBOL(kstrtoull);

and

EXPORT_SYMBOL(kstrtoull);
#ifndef BOOT_STRING

work for me. So I don't know the key problem, cause the config you
mentioned are y.

Thanks,
Chao Fan

>>
>>Thanks,
>>Masa
>>
>>On Thu, Nov 29, 2018 at 04:16:26PM +0800, Chao Fan wrote:
>>> ***Background:
>>> People reported that KASLR may randomly choose some positions
>>> which are located in movable memory regions. This will break memory
>>> hotplug feature and make the movable memory chosen by KASLR can't be
>>> removed.
>>> 
>>> ***Solutions:
>>> Get the information of memory hot-remove, then KASLR will know the
>>> right regions. Information about memory hot-remove is in ACPI
>>> tables, which will be parsed after start_kernel(), so that KASLR
>>> can't get the information.
>>> 
>>> Somebody suggest to add a kernel parameter to specify the
>>> immovable memory so that limit KASLR in these regions. Then I make
>>> a patchset. After several versions, Ingo gave a suggestion:
>>> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1634024.html
>>> Follow Ingo's suggestion, imitate the ACPI code to parse the ACPI
>>> tables, so that the kaslr can get necessary memory information in
>>> ACPI tables.
>>> I think ACPI code is an independent part, so imitate the codes
>>> and functions to 'compressed/' directory, so that kaslr won't
>>> influence the initialization of ACPI.
>>> 
>>> PATCH 1/5 Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC
>>> PATCH 2/5 Add efi_get_rsdp_addr() to find RSDP from EFI table when
>>>           booting from EFI.
>>> PATCH 3/5 Add bios_get_rsdp_addr() to search RSDP in memory when EFI
>>>           table not found.
>>> PATCH 4/5 Compute SRAT table from RSDP and walk SRAT table to store
>>>           the immovable memory regions.
>>> PATCH 5/5 Calculate the intersection between memory regions from e820/efi
>>>           memory table and immovable memory regions. Limit KASLR to
>>>           choosing these regions for randomization.
>>> 
>>> v1->v2:
>>>  -  Simplify some code.
>>> Follow Baoquan He's suggestion:
>>>  - Reuse the head file of acpi code.
>>> 
>>> v2->v3:
>>>  - Test in more conditions, so remove the 'RFC' tag.
>>>  - Change some comments.
>>> 
>>> v3->v4:
>>> Follow Thomas Gleixner's suggetsion:
>>>  - Put the whole efi related function into #define CONFIG_EFI and return
>>>    false in the other stub.
>>> 
>>> v4->v5:
>>> Follow Dou Liyang's suggestion:
>>>  - Add more comments about some functions based on kernel code.
>>>  - Change some typo in comments.
>>>  - Clean useless variable.
>>>  - Add check for the boundary of array.
>>>  - Add check for 'movable_node' parameter
>>> 
>>> v5->v6:
>>> Follow Baoquan He's suggestion:
>>>  - Change some log.
>>>  - Add the check for acpi_rsdp
>>>  - Change some code logical to make code clear
>>> 
>>> v6->v7:
>>> Follow Rafael's suggestion:
>>>  - Add more comments and patch log.
>>> Follow test robot's suggestion:
>>>  - Add "static" tag for function
>>> 
>>> v7-v8:
>>> Follow Kees Cook's suggestion:
>>>  - Use mem_overlaps() to check memory region.
>>>  - Use #ifdef in the definition of function.
>>> 
>>> v8-v9:
>>> Follow Boris' suggestion:
>>>  - Change code style.
>>>  - Splite PATCH 1/3 to more path.
>>>  - Introduce some new function
>>>  - Use existing function to rework some code
>>> Follow Masayoshi's suggetion:
>>>  - Make code more readable
>>> 
>>> v9->v10:
>>> Follow Baoquan's suggestion:
>>>  - Change some log
>>>  - Merge last two patch together.
>>> 
>>> v10->v11:
>>> Follow Boris' suggestion:
>>>  - Link kstrtoull() instead of copying it.
>>>  - Drop the useless wrapped function.
>>> 
>>> v11->v12:
>>> Follow Boris' suggestion:
>>>  - Change patch log and code comments.
>>>  - Add 'CONFIG_EARLY_PARSE_RSDP' to make code easy to read
>>>  - Put strtoull() to misc.c
>>> Follow Masa's suggestion:
>>>  - Remove the detection for 'movable_node'
>>>  - Change the code logical about cmdline_find_option()
>>> 
>>> Any comments will be welcome.
>>> 
>>> 
>>> Chao Fan (5):
>>>   x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC
>>>   x86/boot: Add efi_get_rsdp_addr() to find RSDP from EFI table
>>>   x86/boot: Add bios_get_rsdp_addr() to search RSDP in memory
>>>   x86/boot: Parse SRAT table from RSDP and store immovable memory
>>>   x86/boot/KASLR: Limit KASLR to extracting kernel in immovable memory
>>> 
>>>  arch/x86/Kconfig                  |  10 +
>>>  arch/x86/boot/compressed/Makefile |   2 +
>>>  arch/x86/boot/compressed/acpitb.c | 322 ++++++++++++++++++++++++++++++
>>>  arch/x86/boot/compressed/kaslr.c  |  79 ++++++--
>>>  arch/x86/boot/compressed/misc.c   |   5 +
>>>  arch/x86/boot/compressed/misc.h   |  24 +++
>>>  lib/kstrtox.c                     |   5 +
>>>  7 files changed, 432 insertions(+), 15 deletions(-)
>>>  create mode 100644 arch/x86/boot/compressed/acpitb.c
>>> 
>>> -- 
>>> 2.19.1
>>> 
>>> 
>>> 
>>
>>