Re: [PATCH v8 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory

2019-02-10 Thread Chao Fan
On Tue, Feb 05, 2019 at 10:05:16AM -0500, Masayoshi Mizuma wrote:
[...]

Hi Masa,

Sorry for delay, since last days were Chinese holiday.

>diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
>index c5a949335..7dd61b943 100644
>--- a/arch/x86/boot/compressed/acpi.c
>+++ b/arch/x86/boot/compressed/acpi.c
>@@ -288,6 +288,7 @@ int count_immovable_mem_regions(void)
>   struct acpi_subtable_header *sub_table;
>   struct acpi_table_header *table_header;
>   char arg[MAX_ACPI_ARG_LENGTH];
>+  unsigned long long possible_addr, max_possible_addr = 0;

This line is so long that it should be added in first line.

>   int num = 0;
> 
>   if (cmdline_find_option("acpi", arg, sizeof(arg)) == 3 &&
>@@ -308,10 +309,19 @@ int count_immovable_mem_regions(void)
>   struct acpi_srat_mem_affinity *ma;
> 
>   ma = (struct acpi_srat_mem_affinity *)sub_table;
>-  if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) && 
>ma->length) {
>-  immovable_mem[num].start = ma->base_address;
>-  immovable_mem[num].size = ma->length;
>-  num++;
>+  if (ma->length) {
>+  if (ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) {
>+  possible_addr =
>+  ma->base_address + ma->length;
>+  if (possible_addr > max_possible_addr)
>+  max_possible_addr =
>+  possible_addr;
>+  } else {
>+  immovable_mem[num].start =
>+  ma->base_address;
>+  immovable_mem[num].size = ma->length;
>+          num++;
>+  }
>   }

It looks better in another mail where you add a new function.

Thanks,
Chao Fan

> 
>   if (num >= MAX_NUMNODES*2) {
>@@ -320,6 +330,7 @@ int count_immovable_mem_regions(void)
>   }
>   }
>   table += sub_table->length;
>+  boot_params->possible_mem_addr = max_possible_addr;
>   }
>   return num;
> }
>diff --git a/arch/x86/include/uapi/asm/bootparam.h 
>b/arch/x86/include/uapi/asm/bootparam.h
>index 60733f137..5b64b606e 100644
>--- a/arch/x86/include/uapi/asm/bootparam.h
>+++ b/arch/x86/include/uapi/asm/bootparam.h
>@@ -156,7 +156,7 @@ struct boot_params {
>   __u64  tboot_addr;  /* 0x058 */
>   struct ist_info ist_info;   /* 0x060 */
>   __u64 acpi_rsdp_addr;   /* 0x070 */
>-  __u8  _pad3[8]; /* 0x078 */
>+  __u64  possible_mem_addr;   /* 0x078 */
>   __u8  hd0_info[16]; /* obsolete! */ /* 0x080 */
>   __u8  hd1_info[16]; /* obsolete! */ /* 0x090 */
>   struct sys_desc_table sys_desc_table; /* obsolete! */   /* 0x0a0 */
>diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
>index 3f452ffed..71fc28570 100644
>--- a/arch/x86/mm/kaslr.c
>+++ b/arch/x86/mm/kaslr.c
>@@ -70,6 +70,30 @@ static inline bool kaslr_memory_enabled(void)
>   return kaslr_enabled() && !IS_ENABLED(CONFIG_KASAN);
> }
> 
>+static unsigned int __init kaslr_padding(void)
>+{
>+  unsigned int rand_mem_physical_padding =
>+  CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING;
>+#ifdef CONFIG_MEMORY_HOTPLUG
>+  unsigned long long max_possible_phys, max_actual_phys, threshold;
>+
>+  if (!boot_params.possible_mem_addr)
>+  goto out;
>+
>+  max_actual_phys = roundup(PFN_PHYS(max_pfn), 1ULL << TB_SHIFT);
>+  max_possible_phys = roundup(boot_params.possible_mem_addr,
>+  1ULL << TB_SHIFT);
>+  threshold = max_actual_phys +
>+  ((unsigned long long)rand_mem_physical_padding << TB_SHIFT);
>+
>+  if (max_possible_phys > threshold)
>+  rand_mem_physical_padding =
>+  (max_possible_phys - max_actual_phys) >> TB_SHIFT;
>+out:
>+#endif
>+  return rand_mem_physical_padding;
>+}
>+
> /* Initialize base and padding for each memory region randomized with KASLR */
> void __init kernel_randomize_memory(void)
> {
>@@ -103,7 +127,7 @@ void __init 

Re: [PATCH v11 4/5] x86/boot: Dig out SRAT table from RSDP and find immovable memory

2018-11-19 Thread Chao Fan
Hi Boris,

On Fri, Nov 16, 2018 at 12:16:54PM +0100, Borislav Petkov wrote:
>On Mon, Nov 12, 2018 at 05:46:44PM +0800, Chao Fan wrote:
>> To avoid KASLR extracting kernel on movable memory, slove the
> ^
>
>Please introduce a spellchecker into your patch creation workflow.
>
>> conflict between KASLR and movable_node feature, dig the SRAT tables
>
>s/dig/determine/ or "compute SRAT table's address" or so.
>
>Also, replace "dig" with a more suitable verb in all your patches.
>
>> from RSDP pointer. Walk the SRAT tables and store the immovable
>> memory regions in immovable_mem[].
>
>   "... in an array called immovable_mem[]."
>
>> There are three methods to get RSDP pointer: KEXEC condition,
>> EFI confition, BIOS condition.
>
>"condition" is not the right word here.
>
>> If KEXEC add 'acpi_rsdp' to cmdline, use it.
>> Otherwise, parse EFI table for RSDP.
>> Then, search memory for RSDP.
>> 
>> Imitate from ACPI code, based on acpi_os_get_root_pointer().
>> Process: RSDP->RSDT/XSDT->ACPI root table->SRAT.
>
>What?!
>
>This looks like a comment you've added as a note for yourself but not
>part of the final commit message. If you wanna explain the process, then
>write it out in plain english as if you're explaining it to someone who
>doesn't know what you're doing.
>
>> 
>> Signed-off-by: Chao Fan 
>> ---
>>  arch/x86/boot/compressed/Makefile |   4 +
>>  arch/x86/boot/compressed/acpitb.c | 139 ++
>>  arch/x86/boot/compressed/kaslr.c  |   4 -
>>  arch/x86/boot/compressed/misc.h   |  15 
>>  4 files changed, 158 insertions(+), 4 deletions(-)
>> 
>> diff --git a/arch/x86/boot/compressed/Makefile 
>> b/arch/x86/boot/compressed/Makefile
>> index 466f66c8a7f8..b51f7629b8ef 100644
>> --- a/arch/x86/boot/compressed/Makefile
>> +++ b/arch/x86/boot/compressed/Makefile
>> @@ -84,6 +84,10 @@ ifdef CONFIG_X86_64
>>  vmlinux-objs-y += $(obj)/pgtable_64.o
>>  endif
>>  
>> +#if (defined CONFIG_MEMORY_HOTREMOVE) && (defined CONFIG_RANDOMIZE_BASE)
>> +vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/acpitb.o
>> +#endif
>
>Right, as previously pointed out, this needs that CONFIG_ symbol and
>then you can save yourself most (if not all) of the ifdeffery in the
>rest of the patchset.
>
>> +
>>  $(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 5cfb4efa5a19..161f21a7fb3b 100644
>> --- a/arch/x86/boot/compressed/acpitb.c
>> +++ b/arch/x86/boot/compressed/acpitb.c
>> @@ -14,6 +14,11 @@
>>  #define BOOT_STRING
>>  #include "../string.h"
>>  
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +/* Store the immovable memory regions */
>> +struct mem_vector immovable_mem[MAX_NUMNODES*2];
>> +#endif
>> +
>>  /* Search EFI table for RSDP table. */
>>  static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
>>  {
>> @@ -226,3 +231,137 @@ static void get_acpi_rsdp(acpi_physical_address 
>> *rsdp_addr)
>>  }
>>  #endif
>>  }
>> +
>> +/*
>> + * Used to dig RSDP table from EFI table or BIOS.
>> + * If RSDP table found in EFI table, use it. Or search BIOS.
>> + * Based on acpi_os_get_root_pointer().
>> + */
>> +static acpi_physical_address get_rsdp_addr(void)
>> +{
>> +acpi_physical_address pa = 0;
>> +
>> +get_acpi_rsdp();
>> +
>> +if (!pa)
>> +efi_get_rsdp_addr();
>> +
>> +if (!pa)
>> +bios_get_rsdp_addr();
>> +
>> +return pa;
>> +}
>> +
>> +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;
>> +bool acpi_use_rsdt = false;
>> +char *signature;
>> +char arg[10];
>> +u8 *entry;
>> +u32 count;
>> +u32 size;
>> +int i, j;
>> +int ret;
>> +u32 len;
>> +
>> +rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
>> +if (!rsdp)
>> +return NULL;
>> +
>> +ret = cmdline_find_option("acpi", arg, sizeof(arg));
>&

Re: [PATCH v11 4/5] x86/boot: Dig out SRAT table from RSDP and find immovable memory

2018-11-18 Thread Chao Fan
On Fri, Nov 16, 2018 at 12:16:54PM +0100, Borislav Petkov wrote:
>On Mon, Nov 12, 2018 at 05:46:44PM +0800, Chao Fan wrote:
>> To avoid KASLR extracting kernel on movable memory, slove the
> ^
>
>Please introduce a spellchecker into your patch creation workflow.

OK.

>
>> conflict between KASLR and movable_node feature, dig the SRAT tables
>
>s/dig/determine/ or "compute SRAT table's address" or so.
>
>Also, replace "dig" with a more suitable verb in all your patches.

How about "search RSDP pointer"
>
>> from RSDP pointer. Walk the SRAT tables and store the immovable
>> memory regions in immovable_mem[].
>
>   "... in an array called immovable_mem[]."

Looks good.

>
>> There are three methods to get RSDP pointer: KEXEC condition,
>> EFI confition, BIOS condition.
>
>"condition" is not the right word here.
>
>> If KEXEC add 'acpi_rsdp' to cmdline, use it.
>> Otherwise, parse EFI table for RSDP.
>> Then, search memory for RSDP.
>> 
>> Imitate from ACPI code, based on acpi_os_get_root_pointer().
>> Process: RSDP->RSDT/XSDT->ACPI root table->SRAT.
>
>What?!
>
>This looks like a comment you've added as a note for yourself but not
>part of the final commit message. If you wanna explain the process, then
>write it out in plain english as if you're explaining it to someone who
>doesn't know what you're doing.

OK.

>
>> 
>> Signed-off-by: Chao Fan 
>> ---
>>  arch/x86/boot/compressed/Makefile |   4 +
>>  arch/x86/boot/compressed/acpitb.c | 139 ++
>>  arch/x86/boot/compressed/kaslr.c  |   4 -
>>  arch/x86/boot/compressed/misc.h   |  15 
>>  4 files changed, 158 insertions(+), 4 deletions(-)
>> 
>> diff --git a/arch/x86/boot/compressed/Makefile 
>> b/arch/x86/boot/compressed/Makefile
>> index 466f66c8a7f8..b51f7629b8ef 100644
>> --- a/arch/x86/boot/compressed/Makefile
>> +++ b/arch/x86/boot/compressed/Makefile
>> @@ -84,6 +84,10 @@ ifdef CONFIG_X86_64
>>  vmlinux-objs-y += $(obj)/pgtable_64.o
>>  endif
>>  
>> +#if (defined CONFIG_MEMORY_HOTREMOVE) && (defined CONFIG_RANDOMIZE_BASE)
>> +vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/acpitb.o
>> +#endif
>
>Right, as previously pointed out, this needs that CONFIG_ symbol and
>then you can save yourself most (if not all) of the ifdeffery in the
>rest of the patchset.

That makes sense, I will do that.

>
>> +
>>  $(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 5cfb4efa5a19..161f21a7fb3b 100644
>> --- a/arch/x86/boot/compressed/acpitb.c
>> +++ b/arch/x86/boot/compressed/acpitb.c
>> @@ -14,6 +14,11 @@
>>  #define BOOT_STRING
>>  #include "../string.h"
>>  
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +/* Store the immovable memory regions */
>> +struct mem_vector immovable_mem[MAX_NUMNODES*2];
>> +#endif
>> +
>>  /* Search EFI table for RSDP table. */
>>  static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
>>  {
>> @@ -226,3 +231,137 @@ static void get_acpi_rsdp(acpi_physical_address 
>> *rsdp_addr)
>>  }
>>  #endif
>>  }
>> +
>> +/*
>> + * Used to dig RSDP table from EFI table or BIOS.
>> + * If RSDP table found in EFI table, use it. Or search BIOS.
>> + * Based on acpi_os_get_root_pointer().
>> + */
>> +static acpi_physical_address get_rsdp_addr(void)
>> +{
>> +acpi_physical_address pa = 0;
>> +
>> +get_acpi_rsdp();
>> +
>> +if (!pa)
>> +efi_get_rsdp_addr();
>> +
>> +if (!pa)
>> +bios_get_rsdp_addr();
>> +
>> +return pa;
>> +}
>> +
>> +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;
>> +bool acpi_use_rsdt = false;
>> +char *signature;
>> +char arg[10];
>> +u8 *entry;
>> +u32 count;
>> +u32 size;
>> +int i, j;
>> +int ret;
>> +u32 len;
>> +
>> +rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
>> +if (!rsdp)
>> +return NULL;
>

Re: [PATCH v11 5/5] x86/boot/KASLR: Walk srat tables to filter immovable memory

2018-11-18 Thread Chao Fan
On Fri, Nov 16, 2018 at 02:50:39PM +0100, Borislav Petkov wrote:
> Subject: Re: [PATCH v11 5/5] x86/boot/KASLR: Walk srat tables to filter 
> immovable memory
>
>s/srat/SRAT/g
>
>On Mon, Nov 12, 2018 at 05:46:45PM +0800, Chao Fan wrote:
>> KASLR may randomly chooses some positions which are located in movable
>
>   choose
>
>> memory regions. This will break memory hotplug feature and make the
>> movable memory chosen by KASLR can't be removed.
>
>   by KASLR practically immovable.

Thanks,

>
>:)
>
>> The solution is limite KASLR to choose memory regions in immovable
>
>limite?
>
>"to limit"
>
>> node according to SRAT tables.
>> 
>> If CONFIG_MEMORY_HOTREMOVE enabled, walk through the SRAT memory
>
>  *is* enabled,
>
>> tables and store those immovable memory regions so that KASLR can get
>> where to choose for randomization.
>> 
>> If the amount of immovable memory regions is not zero, which
>> means the immovable memory regions existing. Calculate the intersection
>> between memory regions from e820/efi memory table and immovable memory
>> regions.
>
>This is explaining *what* the patch does and generally doesn't need to
>be in the commit messge as people can read it in the patch itself.

OK,

>
>> Signed-off-by: Chao Fan 
>> ---
>>  arch/x86/boot/compressed/kaslr.c | 77 +++-
>>  1 file changed, 66 insertions(+), 11 deletions(-)
>> 
>> diff --git a/arch/x86/boot/compressed/kaslr.c 
>> b/arch/x86/boot/compressed/kaslr.c
>> index b251572e77af..174d2114045e 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
>
>For this and the other occurrences of ifdef CONFIG_MEMORY_HOTREMOVE,
>define empty stubs for those functions in a header and remove the
>ifdeffery at the call sites.

OK,

>
>> +
>>  
>>  enum mem_avoid_index {
>>  MEM_AVOID_ZO_RANGE = 0,
>> @@ -413,6 +418,11 @@ static void mem_avoid_init(unsigned long input, 
>> unsigned long input_size,
>>  /* Mark the memmap regions we need to avoid */
>>  handle_mem_options();
>>  
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +/* Mark the immovable regions we need to choose */
>> +get_immovable_mem();
>> +#endif
>> +
>>  #ifdef CONFIG_X86_VERBOSE_BOOTUP
>>  /* Make sure video RAM can be used. */
>>  add_identity_map(0, PMD_SIZE);
>> @@ -568,9 +578,9 @@ static unsigned long slots_fetch_random(void)
>>  return 0;
>>  }
>>  
>> -static void process_mem_region(struct mem_vector *entry,
>> -   unsigned long minimum,
>> -   unsigned long image_size)
>> +static void slots_count(struct mem_vector *entry,
>
>That's a strange rename.
>
I will change it.

Thanks,
Chao Fan

>__process_mem_region() makes more sense to me.
>
>> +unsigned long minimum,
>> +unsigned long image_size)
>>  {
>>  struct mem_vector region, overlap;
>>  unsigned long start_orig, end;
>> @@ -646,6 +656,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 directely.
>
>"directly"
>
>> + */
>> +if (num_immovable_mem == 0) {
>
>   if (!...
>
>> +slots_count(region, minimum, image_size);
>> +
>> +if (slot_area_index == MAX_SLOT_AREA) {
>> +debug_putstr("Aborted e820/efi memmap scan (slot_areas 
>> full)!\n");
>> +return 1;
>> +}
>> +return 0;
>> +}
>> +
>
>-- 
>Regards/Gruss,
>Boris.
>
>Good mailing practices for 400: avoid top-posting and trim the reply.
>
>




Re: [PATCH v11 3/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdlien from kexec

2018-11-18 Thread Chao Fan
On Wed, Nov 14, 2018 at 07:30:17PM +0100, Borislav Petkov wrote:
>On Wed, Nov 14, 2018 at 02:12:16PM +0800, Chao Fan wrote:
>> But isdigit() would be redefine, so:
>> 
>> diff --git a/include/linux/ctype.h b/include/linux/ctype.h
>> index 363b004426db..aba01c385232 100644
>> --- a/include/linux/ctype.h
>> +++ b/include/linux/ctype.h
>> @@ -23,10 +23,12 @@ extern const unsigned char _ctype[];
>>  #define isalnum(c) ((__ismask(c)&(_U|_L|_D)) != 0)
>>  #define isalpha(c) ((__ismask(c)&(_U|_L)) != 0)
>>  #define iscntrl(c) ((__ismask(c)&(_C)) != 0)
>> +#ifndef BOOT_STRING
>>  static inline int isdigit(int c)
>>  {
>> return '0' <= c && c <= '9';
>>  }
>> +#endif
>>  #define isgraph(c) ((__ismask(c)&(_P|_U|_L|_D)) != 0)
>>  #define islower(c) ((__ismask(c)&(_L)) != 0)
>>  #define isprint(c) ((__ismask(c)&(_P|_U|_L|_D|_SP)) != 0)
>> 
>> Now I can make it.
>> I wonder whether this is OK to cover isdigit() with 'BOOT_STRING' tag.
>
>See the beginning of arch/x86/boot/compressed/kaslr.c for a possible way
>to disable boot/ctype.h

I have done this with BOOT_CTYPE_H.
So misc.c can only use isdigit() and isxdigit() in
include/linux/ctype.h.

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

This looks better than before.

Thanks,
Chao Fan

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




Re: [PATCH v11 3/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdlien from kexec

2018-11-13 Thread Chao Fan
Hi Boris, Masa, and Baoquan,

On Tue, Nov 13, 2018 at 10:51:56PM +0100, Borislav Petkov wrote:
>On Tue, Nov 13, 2018 at 03:06:16PM -0500, Masayoshi Mizuma wrote:
>> I just felt the BOOT_STRING thing in lib/kstrtox.c confuses...
>> I'm OK for now if it's applied your below comment.
>
>Well, actually, upon a second look, I don't think that including a .c
>file into a header is ok:
>
>diff --git a/arch/x86/boot/string.h b/arch/x86/boot/string.h
>index 3d78e27077f4..0ff3edb888e4 100644
>--- a/arch/x86/boot/string.h
>+++ b/arch/x86/boot/string.h
>@@ -30,3 +30,7 @@ extern unsigned long long simple_strtoull(const char *cp, 
>char **endp,
>  unsigned int base);
>
> #endif /* BOOT_STRING_H */
>+
>+#ifdef BOOT_STRING
>+#include "../../../lib/kstrtox.c"
>+#endif
>
>Chao, why isn't this part of arch/x86/boot/compressed/misc.c ?
>

Fine, I have put it to misc.c:

diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 8dd1d5ccae58..714b05b65a33 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -426,3 +426,7 @@ void fortify_panic(const char *name)
 {
error("detected buffer overflow");
 }
+
+#ifdef BOOT_STRING
+#include "../../../../lib/kstrtox.c"
+#endif

And define it in misc.h:

diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 4a3645fda0ed..98e28c4281ee 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -131,3 +131,5 @@ int num_immovable_mem;
 void get_immovable_mem(void);
 #endif
 #endif
+#define BOOT_STRING
+extern int kstrtoull(const char *s, unsigned int base, unsigned long long 
*res);

But isdigit() would be redefine, so:

diff --git a/include/linux/ctype.h b/include/linux/ctype.h
index 363b004426db..aba01c385232 100644
--- a/include/linux/ctype.h
+++ b/include/linux/ctype.h
@@ -23,10 +23,12 @@ extern const unsigned char _ctype[];
 #define isalnum(c) ((__ismask(c)&(_U|_L|_D)) != 0)
 #define isalpha(c) ((__ismask(c)&(_U|_L)) != 0)
 #define iscntrl(c) ((__ismask(c)&(_C)) != 0)
+#ifndef BOOT_STRING
 static inline int isdigit(int c)
 {
return '0' <= c && c <= '9';
 }
+#endif
 #define isgraph(c) ((__ismask(c)&(_P|_U|_L|_D)) != 0)
 #define islower(c) ((__ismask(c)&(_L)) != 0)
 #define isprint(c) ((__ismask(c)&(_P|_U|_L|_D|_SP)) != 0)

Now I can make it.
I wonder whether this is OK to cover isdigit() with 'BOOT_STRING' tag.

Thanks,
Chao Fan

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




Re: [PATCH v11 3/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdlien from kexec

2018-11-13 Thread Chao Fan
On Wed, Nov 14, 2018 at 09:54:50AM +0800, Chao Fan wrote:
>On Tue, Nov 13, 2018 at 06:51:50PM +0100, Borislav Petkov wrote:
>>On Mon, Nov 12, 2018 at 05:46:43PM +0800, Chao Fan wrote:
>>> Imitate setup_acpi_rsdp() for the early_param of 'acpi_rsdp'.
>>> KEXEC writes the RSDP pointer to cmdline for EFI booting.
>>> So if 'acpi_rsdp' found in cmdline, use it directely.
>>> 
>>> Since function kstrtoull() is needed, include it in
>>> arch/x86/boot/string.h. To solve the definition conflict
>>> problem, set BOOT_STRING tag to expose only kstrtoull() and
>>> functions used by it. Other functions in lib/kstrtox.c will
>>> be covered.
>>> 
>>> Signed-off-by: Chao Fan 
>>> ---
>>>  arch/x86/boot/compressed/acpitb.c | 26 ++
>>>  arch/x86/boot/string.h|  4 
>>>  lib/kstrtox.c |  4 
>>>  3 files changed, 34 insertions(+)
>>> 
>>> diff --git a/arch/x86/boot/compressed/acpitb.c 
>>> b/arch/x86/boot/compressed/acpitb.c
>>> index 50fa65cf824d..5cfb4efa5a19 100644
>>> --- a/arch/x86/boot/compressed/acpitb.c
>>> +++ b/arch/x86/boot/compressed/acpitb.c
>>> @@ -8,6 +8,12 @@
>>>  #include 
>>>  #include 
>>>  
>>> +#define STATIC
>>> +#include 
>>> +
>>> +#define BOOT_STRING
>>> +#include "../string.h"
>>> +
>>>  /* Search EFI table for RSDP table. */
>>>  static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
>>>  {
>>> @@ -200,3 +206,23 @@ static void bios_get_rsdp_addr(acpi_physical_address 
>>> *rsdp_addr)
>>> *rsdp_addr = (acpi_physical_address)address;
>>> }
>>>  }
>>> +
>>> +static void get_acpi_rsdp(acpi_physical_address *rsdp_addr)
>>> +{
>>> +#ifdef CONFIG_KEXEC
>>
>>Ok, why is that CONFIG_KEXEC dependency needed now too?
>>
>
>CONFIG_KEXEC is only needed in this function.
>
>When searching RSDP, there are three methods in order:
>1. When booting from KEXEC, 'acpi_rsdp' is added to cmdline by KEXEC,
>   so it can be parsed and used. CONFIG_KEXEC is needed here.
>2. When booting from EFI, parse EFI table and find RSDP.
>3. When booting from BIOS, search memory for RSDP just like
>   acpi_find_root_pointer() in drivers/acpi/acpica/tbxfroot.c did.
>
>So, CONFIG_KEXEC is only needed in 1, exactly in this function
>get_acpi_rsdp() of my PATCH.
>
>Thanks,
>Chao Fan
>

That means, CONFIG_KEXEC is needed by a little part of the whole PATCHSET.
Without CONFIG_KEXEC, RSDP can only be found in other methods.

Thanks,
Chao Fan

>>Ok, let's recap: so far, for your use case you need:
>>
>>CONFIG_MEMORY_HOTREMOVE
>>CONFIG_RANDOMIZE_BASE
>>and now
>>CONFIG_KEXEC
>>
>>So, you can clean up all that ifdeffery by defining a new config item
>>CONFIG_EARLY_PARSE_RSDP or so which depends on all those three items and
>>then you can do
>>
>>vmlinux-objs-$(CONFIG_EARLY_PARSE_RSDP) += $(obj)/acpitb.o
>>
>>and get rid of the most of the ifdeffery.
>>
>>Yes?
>>
>>> +   unsigned long long res;
>>> +   int len = 0;
>>> +   char *val;
>>> +
>>> +   val = malloc(19);
>>> +   len = cmdline_find_option("acpi_rsdp", val, 19);
>>> +
>>
>>^ Superfluous newline.
>>
>>> +   if (len == -1)
>>> +   return;
>>
>>That check is not needed since you do > 0 below.
>>
>>> +
>>> +   if (len > 0) {
>>> +   val[len] = 0;
>>> +   *rsdp_addr = (acpi_physical_address)kstrtoull(val, 16, );
>>> +   }
>>> +#endif
>>> +}
>>> diff --git a/arch/x86/boot/string.h b/arch/x86/boot/string.h
>>> index 3d78e27077f4..0ff3edb888e4 100644
>>> --- a/arch/x86/boot/string.h
>>> +++ b/arch/x86/boot/string.h
>>> @@ -30,3 +30,7 @@ extern unsigned long long simple_strtoull(const char *cp, 
>>> char **endp,
>>>   unsigned int base);
>>>  
>>>  #endif /* BOOT_STRING_H */
>>> +
>>> +#ifdef BOOT_STRING
>>> +#include "../../../lib/kstrtox.c"
>>> +#endif
>>> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
>>> index 1006bf70bf74..3804db9eed56 100644
>>> --- a/lib/kstrtox.c
>>> +++ b/lib/kstrtox.c
>>> @@ -126,6 +126,8 @@ int kstrtoull(const char *s, unsigned int base, 
>>> unsigned long long *res)
>>>  }
>>>  EXPORT_SYMBOL(kstrtoull);
>>
>>This needs a comment to explain what is that guard used for.
>>
>>> +#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 +410,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
>>
>>#endif /* BOOT_STRING */
>>
>>-- 
>>Regards/Gruss,
>>Boris.
>>
>>Good mailing practices for 400: avoid top-posting and trim the reply.
>>
>>




Re: [PATCH v11 3/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdlien from kexec

2018-11-13 Thread Chao Fan
On Tue, Nov 13, 2018 at 06:51:50PM +0100, Borislav Petkov wrote:
>On Mon, Nov 12, 2018 at 05:46:43PM +0800, Chao Fan wrote:
>> Imitate setup_acpi_rsdp() for the early_param of 'acpi_rsdp'.
>> KEXEC writes the RSDP pointer to cmdline for EFI booting.
>> So if 'acpi_rsdp' found in cmdline, use it directely.
>> 
>> Since function kstrtoull() is needed, include it in
>> arch/x86/boot/string.h. To solve the definition conflict
>> problem, set BOOT_STRING tag to expose only kstrtoull() and
>> functions used by it. Other functions in lib/kstrtox.c will
>> be covered.
>> 
>> Signed-off-by: Chao Fan 
>> ---
>>  arch/x86/boot/compressed/acpitb.c | 26 ++
>>  arch/x86/boot/string.h|  4 
>>  lib/kstrtox.c |  4 
>>  3 files changed, 34 insertions(+)
>> 
>> diff --git a/arch/x86/boot/compressed/acpitb.c 
>> b/arch/x86/boot/compressed/acpitb.c
>> index 50fa65cf824d..5cfb4efa5a19 100644
>> --- a/arch/x86/boot/compressed/acpitb.c
>> +++ b/arch/x86/boot/compressed/acpitb.c
>> @@ -8,6 +8,12 @@
>>  #include 
>>  #include 
>>  
>> +#define STATIC
>> +#include 
>> +
>> +#define BOOT_STRING
>> +#include "../string.h"
>> +
>>  /* Search EFI table for RSDP table. */
>>  static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
>>  {
>> @@ -200,3 +206,23 @@ static void bios_get_rsdp_addr(acpi_physical_address 
>> *rsdp_addr)
>>  *rsdp_addr = (acpi_physical_address)address;
>>  }
>>  }
>> +
>> +static void get_acpi_rsdp(acpi_physical_address *rsdp_addr)
>> +{
>> +#ifdef CONFIG_KEXEC
>
>Ok, why is that CONFIG_KEXEC dependency needed now too?
>

CONFIG_KEXEC is only needed in this function.

When searching RSDP, there are three methods in order:
1. When booting from KEXEC, 'acpi_rsdp' is added to cmdline by KEXEC,
   so it can be parsed and used. CONFIG_KEXEC is needed here.
2. When booting from EFI, parse EFI table and find RSDP.
3. When booting from BIOS, search memory for RSDP just like
   acpi_find_root_pointer() in drivers/acpi/acpica/tbxfroot.c did.

So, CONFIG_KEXEC is only needed in 1, exactly in this function
get_acpi_rsdp() of my PATCH.

Thanks,
Chao Fan

>Ok, let's recap: so far, for your use case you need:
>
>CONFIG_MEMORY_HOTREMOVE
>CONFIG_RANDOMIZE_BASE
>and now
>CONFIG_KEXEC
>
>So, you can clean up all that ifdeffery by defining a new config item
>CONFIG_EARLY_PARSE_RSDP or so which depends on all those three items and
>then you can do
>
>vmlinux-objs-$(CONFIG_EARLY_PARSE_RSDP) += $(obj)/acpitb.o
>
>and get rid of the most of the ifdeffery.
>
>Yes?
>
>> +unsigned long long res;
>> +int len = 0;
>> +char *val;
>> +
>> +val = malloc(19);
>> +len = cmdline_find_option("acpi_rsdp", val, 19);
>> +
>
>^ Superfluous newline.
>
>> +if (len == -1)
>> +return;
>
>That check is not needed since you do > 0 below.
>
>> +
>> +if (len > 0) {
>> +val[len] = 0;
>> +*rsdp_addr = (acpi_physical_address)kstrtoull(val, 16, );
>> +}
>> +#endif
>> +}
>> diff --git a/arch/x86/boot/string.h b/arch/x86/boot/string.h
>> index 3d78e27077f4..0ff3edb888e4 100644
>> --- a/arch/x86/boot/string.h
>> +++ b/arch/x86/boot/string.h
>> @@ -30,3 +30,7 @@ extern unsigned long long simple_strtoull(const char *cp, 
>> char **endp,
>>unsigned int base);
>>  
>>  #endif /* BOOT_STRING_H */
>> +
>> +#ifdef BOOT_STRING
>> +#include "../../../lib/kstrtox.c"
>> +#endif
>> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
>> index 1006bf70bf74..3804db9eed56 100644
>> --- a/lib/kstrtox.c
>> +++ b/lib/kstrtox.c
>> @@ -126,6 +126,8 @@ int kstrtoull(const char *s, unsigned int base, unsigned 
>> long long *res)
>>  }
>>  EXPORT_SYMBOL(kstrtoull);
>
>This needs a comment to explain what is that guard used for.
>
>> +#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 +410,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
>
>#endif /* BOOT_STRING */
>
>-- 
>Regards/Gruss,
>Boris.
>
>Good mailing practices for 400: avoid top-posting and trim the reply.
>
>




Re: [PATCH v11 4/5] x86/boot: Dig out SRAT table from RSDP and find immovable memory

2018-11-12 Thread Chao Fan
On Mon, Nov 12, 2018 at 04:51:59PM -0500, Masayoshi Mizuma wrote:
>On Mon, Nov 12, 2018 at 05:46:44PM +0800, Chao Fan wrote:
>> To avoid KASLR extracting kernel on movable memory, slove the
>> conflict between KASLR and movable_node feature, dig the SRAT tables
>> from RSDP pointer. Walk the SRAT tables and store the immovable
>> memory regions in immovable_mem[].
>> 
>> There are three methods to get RSDP pointer: KEXEC condition,
>> EFI confition, BIOS condition.
>> If KEXEC add 'acpi_rsdp' to cmdline, use it.
>> Otherwise, parse EFI table for RSDP.
>> Then, search memory for RSDP.
>> 
>> Imitate from ACPI code, based on acpi_os_get_root_pointer().
>> Process: RSDP->RSDT/XSDT->ACPI root table->SRAT.
>> 
>> Signed-off-by: Chao Fan 
>> ---
>>  arch/x86/boot/compressed/Makefile |   4 +
>>  arch/x86/boot/compressed/acpitb.c | 139 ++
>>  arch/x86/boot/compressed/kaslr.c  |   4 -
>>  arch/x86/boot/compressed/misc.h   |  15 
>>  4 files changed, 158 insertions(+), 4 deletions(-)
>> 
>> diff --git a/arch/x86/boot/compressed/Makefile 
>> b/arch/x86/boot/compressed/Makefile
>> index 466f66c8a7f8..b51f7629b8ef 100644
>> --- a/arch/x86/boot/compressed/Makefile
>> +++ b/arch/x86/boot/compressed/Makefile
>> @@ -84,6 +84,10 @@ ifdef CONFIG_X86_64
>>  vmlinux-objs-y += $(obj)/pgtable_64.o
>>  endif
>>  
>> +#if (defined CONFIG_MEMORY_HOTREMOVE) && (defined CONFIG_RANDOMIZE_BASE)
>> +vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/acpitb.o
>> +#endif
>
>'#' in Makefile means comment out...
>
>ifdef CONFIG_MEMORY_HOTREMOVE
>vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/acpitb.o
>endif
>

Thanks,

>> +
>>  $(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 5cfb4efa5a19..161f21a7fb3b 100644
>> --- a/arch/x86/boot/compressed/acpitb.c
>> +++ b/arch/x86/boot/compressed/acpitb.c
>> @@ -14,6 +14,11 @@
>>  #define BOOT_STRING
>>  #include "../string.h"
>>  
>
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +/* Store the immovable memory regions */
>> +struct mem_vector immovable_mem[MAX_NUMNODES*2];
>> +#endif
>
>This #ifdef isn't needed.
>
>Could you please check the other #ifdef statement as well?

Sure, I will check all the #ifdef.

Thanks,
Chao Fan

>
>Thanks,
>Masa
>
>> +
>>  /* Search EFI table for RSDP table. */
>>  static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
>>  {
>> @@ -226,3 +231,137 @@ static void get_acpi_rsdp(acpi_physical_address 
>> *rsdp_addr)
>>  }
>>  #endif
>>  }
>> +
>> +/*
>> + * Used to dig RSDP table from EFI table or BIOS.
>> + * If RSDP table found in EFI table, use it. Or search BIOS.
>> + * Based on acpi_os_get_root_pointer().
>> + */
>> +static acpi_physical_address get_rsdp_addr(void)
>> +{
>> +acpi_physical_address pa = 0;
>> +
>> +get_acpi_rsdp();
>> +
>> +if (!pa)
>> +efi_get_rsdp_addr();
>> +
>> +if (!pa)
>> +bios_get_rsdp_addr();
>> +
>> +return pa;
>> +}
>> +
>> +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;
>> +bool acpi_use_rsdt = false;
>> +char *signature;
>> +char arg[10];
>> +u8 *entry;
>> +u32 count;
>> +u32 size;
>> +int i, j;
>> +int ret;
>> +u32 len;
>> +
>> +rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
>> +if (!rsdp)
>> +return NULL;
>> +
>> +ret = cmdline_find_option("acpi", arg, sizeof(arg));
>> +if (ret == 4 && !strncmp(arg, "rsdt", 4))
>> +acpi_use_rsdt = true;
>> +
>> +/* Get RSDT or XSDT from RSDP. */
>> +if (!acpi_use_rsdt &&
>> +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_E

Re: [PATCH v11 3/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdlien from kexec

2018-11-12 Thread Chao Fan
On Mon, Nov 12, 2018 at 12:43:44PM -0500, Masayoshi Mizuma wrote:
>Hi Chao,
>
>On Mon, Nov 12, 2018 at 05:46:43PM +0800, Chao Fan wrote:
>> Imitate setup_acpi_rsdp() for the early_param of 'acpi_rsdp'.
>> KEXEC writes the RSDP pointer to cmdline for EFI booting.
>> So if 'acpi_rsdp' found in cmdline, use it directely.
>> 
>
>> Since function kstrtoull() is needed, include it in
>> arch/x86/boot/string.h. To solve the definition conflict
>> problem, set BOOT_STRING tag to expose only kstrtoull() and
>> functions used by it. Other functions in lib/kstrtox.c will
>> be covered.
>
>How about the following get_acpi_rsdp()...? It doesn't use kstrtoull().
>
>static void get_acpi_rsdp(acpi_physical_address *rsdp_addr)
>{
>#ifdef CONFIG_KEXEC
>unsigned long addr;
>char val[32];
>
>if (cmdline_find_option("acpi_rsdp", val, sizeof(val)) > 0) {
>char *e;
>
>if (!strncmp(val, "0x", 2)) {
>addr = simple_strtoull(val + 2, , 16);
>if ((addr == 0) || ((val + 2) == e))
>return;
>*rsdp_addr = (acpi_physical_address)addr;
>}
>}
>#endif
>}

Thanks for the suggestion.
I used this function. In the old version, Boris said simple_strtoull()
is the old function and told me use the new kstrtoull().

Thanks,
Chao Fan

>
>Thanks,
>Masa
>
>> 
>> Signed-off-by: Chao Fan 
>> ---
>>  arch/x86/boot/compressed/acpitb.c | 26 ++
>>  arch/x86/boot/string.h|  4 
>>  lib/kstrtox.c |  4 
>>  3 files changed, 34 insertions(+)
>> 
>> diff --git a/arch/x86/boot/compressed/acpitb.c 
>> b/arch/x86/boot/compressed/acpitb.c
>> index 50fa65cf824d..5cfb4efa5a19 100644
>> --- a/arch/x86/boot/compressed/acpitb.c
>> +++ b/arch/x86/boot/compressed/acpitb.c
>> @@ -8,6 +8,12 @@
>>  #include 
>>  #include 
>>  
>> +#define STATIC
>> +#include 
>> +
>> +#define BOOT_STRING
>> +#include "../string.h"
>> +
>>  /* Search EFI table for RSDP table. */
>>  static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
>>  {
>> @@ -200,3 +206,23 @@ static void bios_get_rsdp_addr(acpi_physical_address 
>> *rsdp_addr)
>>  *rsdp_addr = (acpi_physical_address)address;
>>  }
>>  }
>> +
>> +static void get_acpi_rsdp(acpi_physical_address *rsdp_addr)
>> +{
>> +#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 == -1)
>> +return;
>> +
>> +if (len > 0) {
>> +val[len] = 0;
>> +*rsdp_addr = (acpi_physical_address)kstrtoull(val, 16, );
>> +}
>> +#endif
>> +}
>> diff --git a/arch/x86/boot/string.h b/arch/x86/boot/string.h
>> index 3d78e27077f4..0ff3edb888e4 100644
>> --- a/arch/x86/boot/string.h
>> +++ b/arch/x86/boot/string.h
>> @@ -30,3 +30,7 @@ extern unsigned long long simple_strtoull(const char *cp, 
>> char **endp,
>>unsigned int base);
>>  
>>  #endif /* BOOT_STRING_H */
>> +
>> +#ifdef BOOT_STRING
>> +#include "../../../lib/kstrtox.c"
>> +#endif
>> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
>> index 1006bf70bf74..3804db9eed56 100644
>> --- a/lib/kstrtox.c
>> +++ b/lib/kstrtox.c
>> @@ -126,6 +126,8 @@ int kstrtoull(const char *s, unsigned int base, unsigned 
>> long long *res)
>>  }
>>  EXPORT_SYMBOL(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 +410,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
>> -- 
>> 2.19.1
>> 
>> 
>> 
>
>




Re: [PATCH v11 2/5] x86/boot: Add bios_get_rsdp_addr() to search RSDP in memory

2018-11-12 Thread Chao Fan
On Mon, Nov 12, 2018 at 04:27:44PM +0100, Borislav Petkov wrote:
>On Mon, Nov 12, 2018 at 05:46:42PM +0800, Chao Fan wrote:
>> Imitate ACPI code to search RSDP pointer from memory.
>> Walk memory and check the signature until get the RSDP signature.
>> Based on acpi_tb_scan_memory_for_rsdp() and acpi_find_root_pointer().
>> If didn't get RSDP from EFI table, will run this function.
>
>That's some very strange english. Please improve.
>
>> Used for later patch to dig out SRAT table and get the memory
>> information. And figure out the immovable memory regions
>> to avoid KASLR extracts kernel on movable memory, slove the
>   ^^
>
>Please introduce a spellchecker into your patch creation workflow.
>

Thanks.

>> conflict between KASLR and movable_node feature.
>
>Btw, this paragraph could be used for a CONFIG_ item you could define
>for your particular use case. Because right now you have funnies like:
>
>+#if (defined CONFIG_MEMORY_HOTREMOVE) && (defined CONFIG_RANDOMIZE_BASE)
>+vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/acpitb.o
>+#endif
>
>where CONFIG_RANDOMIZE_BASE is repeated for no good reason.
>
>But we'll see - need to get to the end of your patch series first.
>
>> Signed-off-by: Chao Fan 
>> ---
>>  arch/x86/boot/compressed/acpitb.c | 106 ++
>>  1 file changed, 106 insertions(+)
>> 
>> diff --git a/arch/x86/boot/compressed/acpitb.c 
>> b/arch/x86/boot/compressed/acpitb.c
>> index 56b54b0e0889..50fa65cf824d 100644
>> --- a/arch/x86/boot/compressed/acpitb.c
>> +++ b/arch/x86/boot/compressed/acpitb.c
>> @@ -94,3 +94,109 @@ static void efi_get_rsdp_addr(acpi_physical_address 
>> *rsdp_addr)
>>  }
>>  #endif
>>  }
>> +
>> +static u8 compute_checksum(u8 *buffer, u32 length)
>> +{
>> +u8 sum = 0;
>> +u8 *end = buffer + length;
>> +
>> +while (buffer < end)
>> +sum = (u8)(sum + *(buffer++));
>
>What's that cast for?
>
>Ah, this is the version in acpi_tb_checksum(). Well, I'd write this
>simply as:
>
>   sum += *(buffer++);

Thanks for your suggestion.

>
>> +
>> +return sum;
>> +}
>> +
>> +/*
>> + * Used to search a block of memory for the RSDP signature.
>> + * Return Pointer to the RSDP if found, otherwise NULL.
>
> "Returns pointer... "
>
>> + * Based on acpi_tb_scan_memory_for_rsdp().
>> + */
>> +static u8 *scan_mem_for_rsdp(u8 *start, u32 length)
>> +{
>> +struct acpi_table_rsdp *rsdp;
>> +u8 *end;
>> +u8 *rover;
>
>rover?
>
>> +
>> +end = start + length;
>> +
>> +/* Search from given start address for the requested length */
>> +for (rover = start; rover < end; rover += ACPI_RSDP_SCAN_STEP) {

The 'rover' was named as 'mem_rover', but the length of this line is too
long. So shorten it as 'rever' so that they can keep in one line.

>> +/*
>> + * The RSDP signature and checksum must both 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 *)rover;
>> +
>> +/* Nope, 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;
>> +
>> +/* Sig and checksum valid, we have found a real RSDP */
>> +return rover;
>> +}
>> +return NULL;
>> +}
>> +
>> +/*
>> + * Used to search RSDP physical address.
>> + * Based on acpi_find_root_pointer(). Since only use physical address
>> + * in this period, so there is no need to do the memory map jobs.
>
>You mean: "All addresses used here are physical."?
>
>"memory map jobs"?
>
>Please be more careful when writing comments which are going to be read
>by other people. "jobs" means a lot of things and you don't w

Re: [PATCH v11 1/5] x86/boot: Add efi_get_rsdp_addr() to dig out RSDP from EFI table

2018-11-12 Thread Chao Fan
On Mon, Nov 12, 2018 at 03:54:33PM +0100, Borislav Petkov wrote:
>On Mon, Nov 12, 2018 at 05:46:41PM +0800, Chao Fan wrote:
>> In order to parse SRAT table and get memory information, RSDP pointer
>> should be found. In kernel, there are three methods to get RSDP:
>> EFI condition, BIOS condition and KEXEC condition. The first works
>> for EFI condition.
>
>"condition"?
>
>Also, please explain shortly what all those abbreviations mean: think
>of a person reading your commit message who doesn't have any clue from
>ACPI.

I will try to improve in next version.
>
>> Imitate ACPI code and EFI code to dig RSDP pointer from EFI tables.
>> Process: boot_param->systab->efi_config_table->RSDP.
>> Based on efi_init(), efi_config_init(), efi_config_parse_tables().
>> 
>> Signed-off-by: Chao Fan 
>> ---
>>  arch/x86/boot/compressed/acpitb.c | 96 +++
>>  1 file changed, 96 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 ..56b54b0e0889
>> --- /dev/null
>> +++ b/arch/x86/boot/compressed/acpitb.c
>> @@ -0,0 +1,96 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#define BOOT_CTYPE_H
>> +#include "misc.h"
>> +#include "error.h"
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/* Search EFI table for RSDP table. */
>> +static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
>
>This is just silly: the function returns void and has a single parameter
>which is an *output* parameter?!
>
>Why isn't the signature
>
>static acpi_physical_address *efi_get_rsdp_addr(void)
>
>instead?

I will change the function style.

>
>> +{
>> +#ifdef CONFIG_EFI
>> +efi_system_table_t *systab;
>> +bool efi_64 = false;
>
>You're setting it below already, why here too?

I will drop.

>
>> +void *config_tables;
>> +struct efi_info *e;
>> +char *sig;
>> +int size;
>> +int i;
>> +
>> +e = _params->efi_info;
>> +sig = (char *)>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;
>> +}
>> +
>> +/* 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));
>
>No ugly line breaks with open braces trailing like that, pls - just let
>it stick out.

Thanks for this suggestion.

>
>> +#else
>> +if (e->efi_systab_hi || e->efi_memmap_hi) {
>> +debug_putstr("Table located above 4GB. EFI should be 
>> disabled.\n");
>
>You need to say here what really happens here:
>
>   debug_putstr("Error getting RSDP address: EFI system table 
> located above 4GB.\n");
>
>The same below.

I will change all the all the same message.
>
>> +return;
>> +}
>> +systab = (efi_system_table_t *)e->efi_systab;
>> +#endif
>> +
>> +if (!systab)
>> +return;
>> +
>> +/*
>> + * Get EFI tables from systab. Based on efi_config_init() and
>> + * efi_config_parse_tables(). Only dig out the config_table.
>> + */
>> +size = efi_64 ? sizeof(efi_config_table_64_t) :
>> +sizeof(efi_config_table_32_t);
>> +
>> +for (i = 0; i < systab->nr_tables; i++) {
>> +efi_guid_t guid;
>> +unsigned long table;
>
>Put the void *config_tables declaration here.

OK.

>
>> +
>> +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;
>> +#ifndef CONFIG_64BIT
>
>Above you have CONFIG_X86_64, here CONFIG_64BIT. Please use one only.
>
>Also, use IS_ENABLED() instead.

OK.

>
>> +if (table

Re: [PATCH v11 3/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdlien from kexec

2018-11-12 Thread Chao Fan
Hi Boris,

I try to include lib/kstrtox.c in arch/x86/boot/string.c and
define the kstrtoull() function in arch/x86/boot/string.h.
But the definition problem is hard to solve, so I include
it in arch/x86/boot/string.c directely.
Then use BOOT_STRING tag to cover other functions and only
kstrtoull() is exposed.
I am not sure whether this is OK.

Thanks,
Chao Fan

On Mon, Nov 12, 2018 at 05:46:43PM +0800, Chao Fan wrote:
>Imitate setup_acpi_rsdp() for the early_param of 'acpi_rsdp'.
>KEXEC writes the RSDP pointer to cmdline for EFI booting.
>So if 'acpi_rsdp' found in cmdline, use it directely.
>
>Since function kstrtoull() is needed, include it in
>arch/x86/boot/string.h. To solve the definition conflict
>problem, set BOOT_STRING tag to expose only kstrtoull() and
>functions used by it. Other functions in lib/kstrtox.c will
>be covered.
>
>Signed-off-by: Chao Fan 
>---
> arch/x86/boot/compressed/acpitb.c | 26 ++
> arch/x86/boot/string.h|  4 
> lib/kstrtox.c |  4 
> 3 files changed, 34 insertions(+)
>
>diff --git a/arch/x86/boot/compressed/acpitb.c 
>b/arch/x86/boot/compressed/acpitb.c
>index 50fa65cf824d..5cfb4efa5a19 100644
>--- a/arch/x86/boot/compressed/acpitb.c
>+++ b/arch/x86/boot/compressed/acpitb.c
>@@ -8,6 +8,12 @@
> #include 
> #include 
> 
>+#define STATIC
>+#include 
>+
>+#define BOOT_STRING
>+#include "../string.h"
>+
> /* Search EFI table for RSDP table. */
> static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
> {
>@@ -200,3 +206,23 @@ static void bios_get_rsdp_addr(acpi_physical_address 
>*rsdp_addr)
>   *rsdp_addr = (acpi_physical_address)address;
>   }
> }
>+
>+static void get_acpi_rsdp(acpi_physical_address *rsdp_addr)
>+{
>+#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 == -1)
>+  return;
>+
>+  if (len > 0) {
>+  val[len] = 0;
>+  *rsdp_addr = (acpi_physical_address)kstrtoull(val, 16, );
>+  }
>+#endif
>+}
>diff --git a/arch/x86/boot/string.h b/arch/x86/boot/string.h
>index 3d78e27077f4..0ff3edb888e4 100644
>--- a/arch/x86/boot/string.h
>+++ b/arch/x86/boot/string.h
>@@ -30,3 +30,7 @@ extern unsigned long long simple_strtoull(const char *cp, 
>char **endp,
> unsigned int base);
> 
> #endif /* BOOT_STRING_H */
>+
>+#ifdef BOOT_STRING
>+#include "../../../lib/kstrtox.c"
>+#endif
>diff --git a/lib/kstrtox.c b/lib/kstrtox.c
>index 1006bf70bf74..3804db9eed56 100644
>--- a/lib/kstrtox.c
>+++ b/lib/kstrtox.c
>@@ -126,6 +126,8 @@ int kstrtoull(const char *s, unsigned int base, unsigned 
>long long *res)
> }
> EXPORT_SYMBOL(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 +410,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
>-- 
>2.19.1
>




[PATCH v11 0/5] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory

2018-11-12 Thread Chao Fan
***Background:
People reported that kaslr may randomly chooses some positions
which are located in movable memory regions. This will break memory
hotplug feature and make the movable memory chosen by KASLR can't be
removed.

***Solutions:
There should be a method to limit kaslr to choosing immovable memory
regions, so there are 2 solutions:
1) Add a kernel parameter to specify the memory regions.
2) Get the information of memory hot-remove, then kaslr will know the
   right regions.
In method 2, information about memory hot-remove is in ACPI
tables, which will be parsed after start_kernel(), kaslr can't get
the information.
In method 1, users should know the regions address and specify in
kernel parameter.

In the earliest time, I tried to dig ACPI tabls to solve this problem.
But I didn't splite the code in 'compressed/' and ACPI code, so the patch
is hard to follow so refused by community.
Somebody suggest to add a kernel parameter to specify the
immovable memory so that limit kaslr in these regions. Then I make
a new 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/7 Add efi_get_rsdp_addr() to dig out RSDP from EFI table when
  booting from EFI.
PATCH 2/7 Add bios_get_rsdp_addr() to search RSDP in memory when EFI
  table not found.
PATCH 3/7 Add get_acpi_rsdp() to parse RSDP in cmdline from kexec
PATCH 4/7 Dig out SRAT table from RSDP and walk SRAT table to store
  the immovable memory regions.
PATCH 5/7 Calculate the intersection between memory regions from e820/efi
  memory table and immovable memory regions. Limit KASLR choose
  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 wraped function.

Any comments will be welcome.

Chao Fan (5):
  x86/boot: Add efi_get_rsdp_addr() to dig out RSDP from EFI table
  x86/boot: Add bios_get_rsdp_addr() to search RSDP in memory
  x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdlien from kexec
  x86/boot: Dig out SRAT table from RSDP and find immovable memory
  x86/boot/KASLR: Walk srat tables to filter immovable memory

 arch/x86/boot/compressed/Makefile |   4 +
 arch/x86/boot/compressed/acpitb.c | 367 ++
 arch/x86/boot/compressed/kaslr.c  |  81 +--
 arch/x86/boot/compressed/misc.h   |  15 ++
 arch/x86/boot/string.h|   4 +
 lib/kstrtox.c |   4 +
 6 files changed, 460 insertions(+), 15 deletions(-)
 create mode 100644 arch/x86/boot/compressed/acpitb.c

-- 
2.19.1





[PATCH v11 5/5] x86/boot/KASLR: Walk srat tables to filter immovable memory

2018-11-12 Thread Chao Fan
KASLR may randomly chooses 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.

The solution is limite KASLR to choose memory regions in immovable
node according to SRAT tables.

If CONFIG_MEMORY_HOTREMOVE enabled, walk through the SRAT memory
tables and store those immovable memory regions so that KASLR can get
where to choose for randomization.

If the amount of immovable memory regions is not zero, which
means the immovable memory regions existing. Calculate the intersection
between memory regions from e820/efi memory table and immovable memory
regions.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/kaslr.c | 77 +++-
 1 file changed, 66 insertions(+), 11 deletions(-)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index b251572e77af..174d2114045e 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,11 @@ static void mem_avoid_init(unsigned long input, unsigned 
long input_size,
/* Mark the memmap regions we need to avoid */
handle_mem_options();
 
+#ifdef CONFIG_MEMORY_HOTREMOVE
+   /* Mark the immovable regions we need to choose */
+   get_immovable_mem();
+#endif
+
 #ifdef CONFIG_X86_VERBOSE_BOOTUP
/* Make sure video RAM can be used. */
add_identity_map(0, PMD_SIZE);
@@ -568,9 +578,9 @@ static unsigned long slots_fetch_random(void)
return 0;
 }
 
-static void process_mem_region(struct mem_vector *entry,
-  unsigned long minimum,
-  unsigned long image_size)
+static void slots_count(struct mem_vector *entry,
+   unsigned long minimum,
+   unsigned long image_size)
 {
struct mem_vector region, overlap;
unsigned long start_orig, end;
@@ -646,6 +656,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 directely.
+*/
+   if (num_immovable_mem == 0) {
+   slots_count(region, minimum, image_size);
+
+   if (slot_area_index == MAX_SLOT_AREA) {
+   debug_putstr("Aborted e820/efi memmap scan (slot_areas 
full)!\n");
+   return 1;
+   }
+   return 0;
+   }
+
+#ifdef CONFIG_MEMORY_HOTREMOVE
+   /*
+* If immovable memory found, filter the intersection between
+* immovable memory and region to slots_count.
+* 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, _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;
+
+   slots_count(, 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 +772,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(, minimum, image_size);
-   if (slot_area_index == MAX_SLOT_AREA) {
-   debug_putstr("Aborted EFI scan (slot_areas full)!\n");
+   if (process_mem_region(, minimum, image_size))
break;
-   }
}
return true;
 }
@@ -742,11 +800,8 @@ static void process_e820_entries(unsigned long minimum,
  

[PATCH v11 3/5] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdlien from kexec

2018-11-12 Thread Chao Fan
Imitate setup_acpi_rsdp() for the early_param of 'acpi_rsdp'.
KEXEC writes the RSDP pointer to cmdline for EFI booting.
So if 'acpi_rsdp' found in cmdline, use it directely.

Since function kstrtoull() is needed, include it in
arch/x86/boot/string.h. To solve the definition conflict
problem, set BOOT_STRING tag to expose only kstrtoull() and
functions used by it. Other functions in lib/kstrtox.c will
be covered.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/acpitb.c | 26 ++
 arch/x86/boot/string.h|  4 
 lib/kstrtox.c |  4 
 3 files changed, 34 insertions(+)

diff --git a/arch/x86/boot/compressed/acpitb.c 
b/arch/x86/boot/compressed/acpitb.c
index 50fa65cf824d..5cfb4efa5a19 100644
--- a/arch/x86/boot/compressed/acpitb.c
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -8,6 +8,12 @@
 #include 
 #include 
 
+#define STATIC
+#include 
+
+#define BOOT_STRING
+#include "../string.h"
+
 /* Search EFI table for RSDP table. */
 static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
 {
@@ -200,3 +206,23 @@ static void bios_get_rsdp_addr(acpi_physical_address 
*rsdp_addr)
*rsdp_addr = (acpi_physical_address)address;
}
 }
+
+static void get_acpi_rsdp(acpi_physical_address *rsdp_addr)
+{
+#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 == -1)
+   return;
+
+   if (len > 0) {
+   val[len] = 0;
+   *rsdp_addr = (acpi_physical_address)kstrtoull(val, 16, );
+   }
+#endif
+}
diff --git a/arch/x86/boot/string.h b/arch/x86/boot/string.h
index 3d78e27077f4..0ff3edb888e4 100644
--- a/arch/x86/boot/string.h
+++ b/arch/x86/boot/string.h
@@ -30,3 +30,7 @@ extern unsigned long long simple_strtoull(const char *cp, 
char **endp,
  unsigned int base);
 
 #endif /* BOOT_STRING_H */
+
+#ifdef BOOT_STRING
+#include "../../../lib/kstrtox.c"
+#endif
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index 1006bf70bf74..3804db9eed56 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -126,6 +126,8 @@ int kstrtoull(const char *s, unsigned int base, unsigned 
long long *res)
 }
 EXPORT_SYMBOL(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 +410,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
-- 
2.19.1





[PATCH v11 1/5] x86/boot: Add efi_get_rsdp_addr() to dig out RSDP from EFI table

2018-11-12 Thread Chao Fan
In order to parse SRAT table and get memory information, RSDP pointer
should be found. In kernel, there are three methods to get RSDP:
EFI condition, BIOS condition and KEXEC condition. The first works
for EFI condition.

Imitate ACPI code and EFI code to dig RSDP pointer from EFI tables.
Process: boot_param->systab->efi_config_table->RSDP.
Based on efi_init(), efi_config_init(), efi_config_parse_tables().

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/acpitb.c | 96 +++
 1 file changed, 96 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 ..56b54b0e0889
--- /dev/null
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0
+#define BOOT_CTYPE_H
+#include "misc.h"
+#include "error.h"
+
+#include 
+#include 
+#include 
+#include 
+
+/* Search EFI table for RSDP table. */
+static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
+{
+#ifdef CONFIG_EFI
+   efi_system_table_t *systab;
+   bool efi_64 = false;
+   void *config_tables;
+   struct efi_info *e;
+   char *sig;
+   int size;
+   int i;
+
+   e = _params->efi_info;
+   sig = (char *)>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;
+   }
+
+   /* 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("Table located above 4GB. EFI should be 
disabled.\n");
+   return;
+   }
+   systab = (efi_system_table_t *)e->efi_systab;
+#endif
+
+   if (!systab)
+   return;
+
+   /*
+* Get EFI tables from systab. Based on efi_config_init() and
+* efi_config_parse_tables(). Only dig out the config_table.
+*/
+   size = efi_64 ? sizeof(efi_config_table_64_t) :
+   sizeof(efi_config_table_32_t);
+
+   for (i = 0; i < systab->nr_tables; i++) {
+   efi_guid_t guid;
+   unsigned long table;
+
+   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;
+#ifndef CONFIG_64BIT
+   if (table >> 32) {
+   debug_putstr("Table located above 4G. EFI 
should be disabled.\n");
+   return;
+   }
+#endif
+   } 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;
+   }
+
+   /*
+* Get RSDP from EFI tables.
+* If ACPI20 table found, use it.
+* If ACPI20 table not found, but ACPI table found,
+* use the ACPI table.
+*/
+   if (!(efi_guidcmp(guid, ACPI_TABLE_GUID))) {
+   *rsdp_addr = (acpi_physical_address)table;
+   } else if (!(efi_guidcmp(guid, ACPI_20_TABLE_GUID))) {
+   *rsdp_addr = (acpi_physical_address)table;
+   return;
+   }
+   }
+#endif
+}
-- 
2.19.1





[PATCH v11 4/5] x86/boot: Dig out SRAT table from RSDP and find immovable memory

2018-11-12 Thread Chao Fan
To avoid KASLR extracting kernel on movable memory, slove the
conflict between KASLR and movable_node feature, dig the SRAT tables
from RSDP pointer. Walk the SRAT tables and store the immovable
memory regions in immovable_mem[].

There are three methods to get RSDP pointer: KEXEC condition,
EFI confition, BIOS condition.
If KEXEC add 'acpi_rsdp' to cmdline, use it.
Otherwise, parse EFI table for RSDP.
Then, search memory for RSDP.

Imitate from ACPI code, based on acpi_os_get_root_pointer().
Process: RSDP->RSDT/XSDT->ACPI root table->SRAT.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/Makefile |   4 +
 arch/x86/boot/compressed/acpitb.c | 139 ++
 arch/x86/boot/compressed/kaslr.c  |   4 -
 arch/x86/boot/compressed/misc.h   |  15 
 4 files changed, 158 insertions(+), 4 deletions(-)

diff --git a/arch/x86/boot/compressed/Makefile 
b/arch/x86/boot/compressed/Makefile
index 466f66c8a7f8..b51f7629b8ef 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -84,6 +84,10 @@ ifdef CONFIG_X86_64
vmlinux-objs-y += $(obj)/pgtable_64.o
 endif
 
+#if (defined CONFIG_MEMORY_HOTREMOVE) && (defined CONFIG_RANDOMIZE_BASE)
+vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/acpitb.o
+#endif
+
 $(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 5cfb4efa5a19..161f21a7fb3b 100644
--- a/arch/x86/boot/compressed/acpitb.c
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -14,6 +14,11 @@
 #define BOOT_STRING
 #include "../string.h"
 
+#ifdef CONFIG_MEMORY_HOTREMOVE
+/* Store the immovable memory regions */
+struct mem_vector immovable_mem[MAX_NUMNODES*2];
+#endif
+
 /* Search EFI table for RSDP table. */
 static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
 {
@@ -226,3 +231,137 @@ static void get_acpi_rsdp(acpi_physical_address 
*rsdp_addr)
}
 #endif
 }
+
+/*
+ * Used to dig RSDP table from EFI table or BIOS.
+ * If RSDP table found in EFI table, use it. Or search BIOS.
+ * Based on acpi_os_get_root_pointer().
+ */
+static acpi_physical_address get_rsdp_addr(void)
+{
+   acpi_physical_address pa = 0;
+
+   get_acpi_rsdp();
+
+   if (!pa)
+   efi_get_rsdp_addr();
+
+   if (!pa)
+   bios_get_rsdp_addr();
+
+   return pa;
+}
+
+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;
+   bool acpi_use_rsdt = false;
+   char *signature;
+   char arg[10];
+   u8 *entry;
+   u32 count;
+   u32 size;
+   int i, j;
+   int ret;
+   u32 len;
+
+   rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
+   if (!rsdp)
+   return NULL;
+
+   ret = cmdline_find_option("acpi", arg, sizeof(arg));
+   if (ret == 4 && !strncmp(arg, "rsdt", 4))
+   acpi_use_rsdt = true;
+
+   /* Get RSDT or XSDT from RSDP. */
+   if (!acpi_use_rsdt &&
+   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;
+   len = header->length;
+   count = (u32)((len - sizeof(struct acpi_table_header)) / size);
+   entry = ACPI_ADD_PTR(u8, header, sizeof(struct acpi_table_header));
+
+   for (i = 0; i < count; i++) {
+   u64 address64;
+
+   if (size == ACPI_RSDT_ENTRY_SIZE)
+   acpi_table = ((acpi_physical_address)
+ (*ACPI_CAST_PTR(u32, entry)));
+   else {
+   *(u64 *)(void *) = *(u64 *)(void *)entry;
+   acpi_table = (acpi_physical_address) address64;
+   }
+
+   if (acpi_table) {
+   header = (struct acpi_table_header *)acpi_table;
+   signature = header->signature;
+
+   if (ACPI_COMPARE_NAME(header->signature, ACPI_SIG_SRAT))
+   return header;
+   }
+   entry += size;
+   }
+   return NULL;
+}
+
+/*
+ * According to ACPI table, filter the immvoable 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;
+ 

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

2018-11-12 Thread Chao Fan
Imitate ACPI code to search RSDP pointer from memory.
Walk memory and check the signature until get the RSDP signature.
Based on acpi_tb_scan_memory_for_rsdp() and acpi_find_root_pointer().
If didn't get RSDP from EFI table, will run this function.

Used for later patch to dig out SRAT table and get the memory
information. And figure out the immovable memory regions
to avoid KASLR extracts kernel on movable memory, slove the
conflict between KASLR and movable_node feature.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/acpitb.c | 106 ++
 1 file changed, 106 insertions(+)

diff --git a/arch/x86/boot/compressed/acpitb.c 
b/arch/x86/boot/compressed/acpitb.c
index 56b54b0e0889..50fa65cf824d 100644
--- a/arch/x86/boot/compressed/acpitb.c
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -94,3 +94,109 @@ static void efi_get_rsdp_addr(acpi_physical_address 
*rsdp_addr)
}
 #endif
 }
+
+static u8 compute_checksum(u8 *buffer, u32 length)
+{
+   u8 sum = 0;
+   u8 *end = buffer + length;
+
+   while (buffer < end)
+   sum = (u8)(sum + *(buffer++));
+
+   return sum;
+}
+
+/*
+ * Used to search a block of memory for the RSDP signature.
+ * Return Pointer to the RSDP if found, otherwise NULL.
+ * Based on acpi_tb_scan_memory_for_rsdp().
+ */
+static u8 *scan_mem_for_rsdp(u8 *start, u32 length)
+{
+   struct acpi_table_rsdp *rsdp;
+   u8 *end;
+   u8 *rover;
+
+   end = start + length;
+
+   /* Search from given start address for the requested length */
+   for (rover = start; rover < end; rover += ACPI_RSDP_SCAN_STEP) {
+   /*
+* The RSDP signature and checksum must both 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 *)rover;
+
+   /* Nope, 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;
+
+   /* Sig and checksum valid, we have found a real RSDP */
+   return rover;
+   }
+   return NULL;
+}
+
+/*
+ * Used to search RSDP physical address.
+ * Based on acpi_find_root_pointer(). Since only use physical address
+ * in this period, so there is no need to do the memory map jobs.
+ */
+static void bios_get_rsdp_addr(acpi_physical_address *rsdp_addr)
+{
+   struct acpi_table_rsdp *rsdp;
+   u8 *table_ptr;
+   u8 *mem_rover;
+   u32 address;
+
+   /*
+* Get the location of the Extended BIOS Data Area (EBDA)
+* Since we use physical address directely, so
+* acpi_os_map_memory() and acpi_os_unmap_memory() are
+* not needed here.
+*/
+   table_ptr = (u8 *)ACPI_EBDA_PTR_LOCATION;
+   *(u32 *)(void *) = *(u16 *)(void *)table_ptr;
+   address <<= 4;
+   table_ptr = (u8 *)address;
+
+   /*
+* Search EBDA paragraphs (EBDA is required to be a minimum of
+* 1K length)
+*/
+   if (address > 0x400) {
+   mem_rover = scan_mem_for_rsdp(table_ptr, ACPI_EBDA_WINDOW_SIZE);
+
+   if (mem_rover) {
+   address += (u32)ACPI_PTR_DIFF(mem_rover, table_ptr);
+   *rsdp_addr = (acpi_physical_address)address;
+   return;
+   }
+   }
+
+   table_ptr = (u8 *)ACPI_HI_RSDP_WINDOW_BASE;
+   mem_rover = scan_mem_for_rsdp(table_ptr, ACPI_HI_RSDP_WINDOW_SIZE);
+
+   /*
+* Search upper memory: 16-byte boundaries in Eh-Fh
+* Since we use physical address directely, so
+* acpi_os_map_memory() and acpi_os_unmap_memory() are
+* not needed here.
+*/
+   if (mem_rover) {
+   address = (u32)(ACPI_HI_RSDP_WINDOW_BASE +
+   ACPI_PTR_DIFF(mem_rover, table_ptr));
+   *rsdp_addr = (acpi_physical_address)address;
+   }
+}
-- 
2.19.1





Re: [PATCH v10 2/7] x86/boot: Copy kstrtoull() to compressed period

2018-11-06 Thread Chao Fan
On Tue, Nov 06, 2018 at 08:13:03PM +0100, Borislav Petkov wrote:
>On Mon, Oct 22, 2018 at 05:37:15PM +0800, Chao Fan wrote:
>> kstrtoull() lives in 'uncompressed' period, used to
>> convert a string to an unsigned long long.
>> Copy to 'compressed' so that we can use it to
>> convert the memory address from sting to unsigned
>
>sting?

oops, typo, string.
>
>> long long in 'compressed' period.
>> 
>> Signed-off-by: Chao Fan 
>> ---
>>  arch/x86/boot/compressed/misc.c | 88 +
>>  arch/x86/boot/compressed/misc.h |  4 ++
>>  2 files changed, 92 insertions(+)
>
>Why do you need to copy things?
>
>You can link that file into compressed/ as lib/kstrtox.c is a library or
>include it similar to what arch/x86/boot/compressed/cmdline.c does.
>
>Still better than copying the code.

I will have a try, thanks for your suggestion.

>
>> diff --git a/arch/x86/boot/compressed/misc.h 
>> b/arch/x86/boot/compressed/misc.h
>> index 008fdc47a29c..40378408d980 100644
>> --- a/arch/x86/boot/compressed/misc.h
>> +++ b/arch/x86/boot/compressed/misc.h
>> @@ -63,6 +63,10 @@ static inline void debug_puthex(const char *s)
>>  
>>  #endif
>>  
>> +#if (defined CONFIG_RANDOMIZE_BASE) && (defined CONFIG_RANDOMIZE_BASE)
>
>CONFIG_RANDOMIZE_BASE twice huh? Once not enough?

Sorry for that, the second should be CONFIG_MEMORY_HOTREMOVE.

Thanks,
Chao Fan

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




Re: [PATCH v10 1/7] x86/boot: Introduce cmdline_find_option_arg()to detect if option=arg in cmdline

2018-11-06 Thread Chao Fan
On Tue, Nov 06, 2018 at 01:22:53PM +0100, Borislav Petkov wrote:
>On Mon, Oct 22, 2018 at 05:37:14PM +0800, Chao Fan wrote:
>> Now, there are cmdline_find_option() and cmdline_find_option_bool() in
>> cmdline.c. Sometimes, when detecting such as whether 'acpi=off' is
>> in cmdline, we need to cmdline_find_option() first, then compare
>> the argument. Now splite the operation as a independent function.
>> Introduce a new function cmdline_find_option_arg() to detect whether
>> option is in command line and the value is arg.
>
>For all future commit messages you write:
>
>Use passive tone in your commit message: no "we", etc.

Got it.

>
>Also, pls read section "2) Describe your changes" in
>Documentation/process/submitting-patches.rst.
OK.
>
>> Signed-off-by: Chao Fan 
>> ---
>>  arch/x86/boot/compressed/cmdline.c | 15 +++
>>  arch/x86/boot/compressed/misc.h|  1 +
>>  2 files changed, 16 insertions(+)
>> 
>> diff --git a/arch/x86/boot/compressed/cmdline.c 
>> b/arch/x86/boot/compressed/cmdline.c
>> index af6cda0b7900..61118c69feb8 100644
>> --- a/arch/x86/boot/compressed/cmdline.c
>> +++ b/arch/x86/boot/compressed/cmdline.c
>> @@ -1,5 +1,7 @@
>>  // SPDX-License-Identifier: GPL-2.0
>>  #include "misc.h"
>> +#define STATIC
>> +#include 
>>  
>>  #if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE || CONFIG_X86_5LEVEL
>>  
>> @@ -30,5 +32,18 @@ int cmdline_find_option_bool(const char *option)
>>  {
>>  return __cmdline_find_option_bool(get_cmd_line_ptr(), option);
>>  }
>> +bool cmdline_find_option_arg(const char *option, const char *arg, int 
>> argsize)
>> +{
>> +char *buffer = malloc(argsize+1);
>> +bool find = false;
>> +int ret;
>> +
>> +ret = cmdline_find_option(option, buffer, argsize+1);
>> +if (ret == argsize && !strncmp(buffer, arg, argsize))
>> +find = true;
>> +
>> +free(buffer);
>> +return find;
>> +}
>
>I don't think such wrapper is needed. Simply calling
>cmdline_find_option() and then examining the buffer - like other call
>sites do - is perfectly fine.

I will change it.

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




Re: [PATCH v8 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory

2018-11-06 Thread Chao Fan
On Tue, Nov 06, 2018 at 10:07:31PM +0800, Baoquan He wrote:
>On 11/06/18 at 01:10pm, Borislav Petkov wrote:
>> > I have another idea to solve this issue. Adding a SRAT parsing code
>> > to arch/x86/mm/kaslr.c. It is useful for both EFI and BIOS and
>> > also we don't need a new kernel parameter...
>> > Dose the idea make sense?
>> 
>> The more automatic stuff we do and we don't have to involve the user,
>> the better.
>> 
>> However, lemme look at Chao's current patchset first - we should not go
>> nuts by putting SRAT parsing everywhere :)
>
>arch/x86/mm/ident_map.c is a good example, it's shared between
>arch/x86/boot/compressed and arch/x86/mm/init_64.c

Thanks to Baoquan, I think we can try this idea.
How about you, Masa?

Thanks,
Chao Fan

>
>




Re: [PATCH v10 0/7] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory

2018-10-31 Thread Chao Fan
On Wed, Oct 31, 2018 at 11:05:37AM +0100, Borislav Petkov wrote:
>On Wed, Oct 31, 2018 at 03:42:34PM +0800, Chao Fan wrote:
>> Hi all,
>> 
>> Sorry for disturbance.
>> No reply to this version for more than one week,
>> any comments?
>
>You do realize that we're right in the merge window right now, right?
>
>And people are busy sending pull requests and fixing fallout.

So sorry for disturbance, I am not so familiar with merge cycle.

>
>So sending new stuff during the two weeks of the merge window, will
>kinda put you on the backburner. Try to remember that next time and be

Thanks for your advise, I got it.

Thanks,
Chao Fan

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




Re: [PATCH v10 0/7] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory

2018-10-31 Thread Chao Fan
Hi all,

Sorry for disturbance.
No reply to this version for more than one week,
any comments?

Thanks,
Chao Fan

On Mon, Oct 22, 2018 at 05:37:13PM +0800, Chao Fan wrote:
>***Background:
>People reported that kaslr may randomly chooses some positions
>which are located in movable memory regions. This will break memory
>hotplug feature and make the movable memory chosen by KASLR can't be
>removed.
>
>***Solutions:
>There should be a method to limit kaslr to choosing immovable memory
>regions, so there are 2 solutions:
>1) Add a kernel parameter to specify the memory regions.
>2) Get the information of memory hot-remove, then kaslr will know the
>   right regions.
>In method 2, information about memory hot-remove is in ACPI
>tables, which will be parsed after start_kernel(), kaslr can't get
>the information.
>In method 1, users should know the regions address and specify in
>kernel parameter.
>
>In the earliest time, I tried to dig ACPI tabls to solve this problem.
>But I didn't splite the code in 'compressed/' and ACPI code, so the patch
>is hard to follow so refused by community.
>Somebody suggest to add a kernel parameter to specify the
>immovable memory so that limit kaslr in these regions. Then I make
>a new 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/7 Introduce cmdline_find_option_arg() to detect if option=arg
>  in cmdline
>PATCH 2/7 Copy kstrtoull() to compressed period, used to convert memory
>  address from string to unsigned long long.
>PATCH 3/7 Add efi_get_rsdp_addr() to dig out RSDP from EFI table when
>  booting from EFI.
>PATCH 4/7 Add bios_get_rsdp_addr() to search RSDP in memory when EFI
>  table not found.
>PATCH 5/7 Add get_acpi_rsdp() to parse RSDP in cmdline from kexec
>PATCH 6/7 Dig out SRAT table from RSDP and walk SRAT table to store
>  the immovable memory regions.
>PATCH 7/7 Calculate the intersection between memory regions from e820/efi
>  memory table and immovable memory regions. Limit KASLR choose
>  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' suggetion:
> - 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.
>
>Any comments will be welcome.
>
>Chao Fan (7):
>  x86/boot: Introduce cmdline_find_option_arg()to detect if option=arg
>in cmdline
>  x86/boot: Copy kstrtoull() to compressed period
>  x86/boot: Add efi_get_rsdp_addr() to dig out RSDP from EFI table
>  x86/boot: Add bios_get_rsdp_addr() to search RSDP in memory
>  x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdlien from kexec
>  x86/boot: Dig out SRAT table from RSDP and find immovable memory
>  x86/boot/KASLR: Limit kaslr to choosing the immovable memory
>
> arch/x86/boot/compressed/Makefile  |   4 +
> arch/x86/boot/compressed/acpitb.c  | 352 +
> arch/x86/boot/compressed/cmdline.c |  15 ++
> arch/x86/boot/compressed/kaslr.c   |  81 +--
> arch/x86/boot/compressed/misc.c|  88 
> arch/x86/boot/compressed/misc.h|  20 ++
> 6 files changed, 545 insertions(+), 15 deletions(-)
> create mode 100644 arch/x86/boot/compressed/acpitb.c
>
>-- 
>2.17.2
>




Re: [PATCH v8 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory

2018-10-24 Thread Chao Fan
On Wed, Oct 24, 2018 at 03:21:36PM -0400, Masayoshi Mizuma wrote:
>On Tue, Oct 23, 2018 at 10:48:02AM +0800, Chao Fan wrote:
>> On Mon, Oct 22, 2018 at 11:42:05AM -0400, Masayoshi Mizuma wrote:
>> >Hi Boris,
>> 
>> Hi Mizuma-san,
>> 
>> I have several questions:
>
>Thank you for your comments! I think your suggestions are
>right.
>However, the prototype patch works EFI environment only.

Yes, I agree. But I think this method is much better than
adding code to arch/x86/mm/kaslr.c.

>The memory hot-plug affinity in SRAT and KASLR are also available
>on legacy BIOS environment, so I need to get the patch useful
>for legacy BIOS as well, but I have no idea to add such things...
>If you have ideas, could you let me know?

I have no idea. I will work on it, try to help figure out the BIOS issue.

Thanks,
Chao Fan

>
>Probably I should have another idea, for example,
>add the SRAT parsing code, looks like you are adding to
>arch/x86/boot/compressed/acpitb.c, to arch/x86/mm/kaslr.c.
>
>Thanks,
>Masa
>
>> 
>> >+static void store_possible_addr(unsigned long long possible)
>> >+{
>> >+   struct setup_data *data;
>> >+
>> >+   data = (struct setup_data *)(unsigned long)boot_params->hdr.setup_data;
>> I suggest you add check:
>> 
>>  if (!data) {
>>  debug_putstr("No setup_data found.\n");
>>  return;
>>  }
>> 
>> >+   while (data) {
>> >+   if (data->type == SETUP_KASLR) {
>> >+   *(unsigned long long *)data->data = possible;
>> >+   return;
>> >+   }
>> >+   data = (struct setup_data *)(unsigned long)data->next;
>> >+   }
>> >+}
>> >+
>> > /*
>> >  * According to ACPI table, filter the immvoable memory regions
>> >  * and store them in immovable_mem[].
>> >@@ -319,6 +333,7 @@ void get_immovable_mem(void)
>> >struct acpi_subtable_header *table;
>> >struct acpi_srat_mem_affinity *ma;
>> >unsigned long table_end;
>> >+   unsigned long long possible_addr, max_possible_addr = 0;
>> >int i = 0;
>> >
>> >if (!cmdline_find_option_bool("movable_node") ||
>> >@@ -338,7 +353,12 @@ void get_immovable_mem(void)
>> >   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)) {
>> >+
>> >+   if (ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) {
>> >+   possible_addr = ma->base_address + ma->length;
>> >+   if (possible_addr > max_possible_addr)
>> >+   max_possible_addr = possible_addr;
>> >+   } else {
>> >immovable_mem[i].start = ma->base_address;
>> >immovable_mem[i].size = ma->length;
>> >i++;
>> >@@ -351,4 +371,5 @@ void get_immovable_mem(void)
>> >((unsigned long)table + table->length);
>> >}
>> >num_immovable_mem = i;
>> >+   store_possible_addr(max_possible_addr);
>> > }
>> >diff --git a/arch/x86/boot/compressed/eboot.c 
>> >b/arch/x86/boot/compressed/eboot.c
>> >index 1458b17..9b95fba 100644
>> >--- a/arch/x86/boot/compressed/eboot.c
>> >+++ b/arch/x86/boot/compressed/eboot.c
>> >@@ -192,6 +192,40 @@ static void setup_efi_pci(struct boot_params *params)
>> >efi_call_early(free_pool, pci_handle);
>> > }
>> >
>> >+#ifdef CONFIG_RANDOMIZE_MEMORY
>> >+static void setup_kaslr(struct boot_params *params)
>> >+{
>> >+   struct setup_data *kaslr_data = NULL;
>> >+   struct setup_data *data;
>> >+   unsigned long size;
>> >+   efi_status_t status;
>> >+
>> >+   size = sizeof(struct setup_data) + sizeof(unsigned long long);
>> >+
>> >+   status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
>> >+   size, (void **)_data);
>> >+   if (status != EFI_SUCCESS) {
>> >+   efi_printk(sys_table, "Failed to allocate memory for 
>> >'kaslr_data'\n");
>> >+   return;
>> >+   }
>> >+
>> &g

Re: [PATCH v8 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory

2018-10-22 Thread Chao Fan
On Mon, Oct 22, 2018 at 11:42:05AM -0400, Masayoshi Mizuma wrote:
>Hi Boris,

Hi Mizuma-san,

I have several questions:

>+static void store_possible_addr(unsigned long long possible)
>+{
>+  struct setup_data *data;
>+
>+  data = (struct setup_data *)(unsigned long)boot_params->hdr.setup_data;
I suggest you add check:

if (!data) {
debug_putstr("No setup_data found.\n");
return;
}

>+  while (data) {
>+  if (data->type == SETUP_KASLR) {
>+  *(unsigned long long *)data->data = possible;
>+  return;
>+  }
>+  data = (struct setup_data *)(unsigned long)data->next;
>+  }
>+}
>+
> /*
>  * According to ACPI table, filter the immvoable memory regions
>  * and store them in immovable_mem[].
>@@ -319,6 +333,7 @@ void get_immovable_mem(void)
>   struct acpi_subtable_header *table;
>   struct acpi_srat_mem_affinity *ma;
>   unsigned long table_end;
>+  unsigned long long possible_addr, max_possible_addr = 0;
>   int i = 0;
>
>   if (!cmdline_find_option_bool("movable_node") ||
>@@ -338,7 +353,12 @@ void get_immovable_mem(void)
>  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)) {
>+
>+  if (ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) {
>+  possible_addr = ma->base_address + ma->length;
>+  if (possible_addr > max_possible_addr)
>+  max_possible_addr = possible_addr;
>+  } else {
>   immovable_mem[i].start = ma->base_address;
>   immovable_mem[i].size = ma->length;
>   i++;
>@@ -351,4 +371,5 @@ void get_immovable_mem(void)
>   ((unsigned long)table + table->length);
>   }
>   num_immovable_mem = i;
>+  store_possible_addr(max_possible_addr);
> }
>diff --git a/arch/x86/boot/compressed/eboot.c 
>b/arch/x86/boot/compressed/eboot.c
>index 1458b17..9b95fba 100644
>--- a/arch/x86/boot/compressed/eboot.c
>+++ b/arch/x86/boot/compressed/eboot.c
>@@ -192,6 +192,40 @@ static void setup_efi_pci(struct boot_params *params)
>   efi_call_early(free_pool, pci_handle);
> }
>
>+#ifdef CONFIG_RANDOMIZE_MEMORY
>+static void setup_kaslr(struct boot_params *params)
>+{
>+  struct setup_data *kaslr_data = NULL;
>+  struct setup_data *data;
>+  unsigned long size;
>+  efi_status_t status;
>+
>+  size = sizeof(struct setup_data) + sizeof(unsigned long long);
>+
>+  status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
>+  size, (void **)_data);
>+  if (status != EFI_SUCCESS) {
>+  efi_printk(sys_table, "Failed to allocate memory for 
>'kaslr_data'\n");
>+  return;
>+  }
>+
>+  kaslr_data->type = SETUP_KASLR;
>+  kaslr_data->next = 0;
>+  kaslr_data->len = size;
>+
>+  data = (struct setup_data *)(unsigned long)params->hdr.setup_data;
>+  if (data)
>+  data->next = (unsigned long)kaslr_data;
Why just put the kaslr_data in data->next. You can't make sure
data->next was NULL.
>+  else {
If data is NULL, go to this else{}, so these two lines below work?
>+  while (data->next)
>+  data = (struct setup_data *)(unsigned long)data->next;
>+  data->next = (unsigned long)kaslr_data;
>+  }
If my understanding is not wrong, it should be:

data = (struct setup_data *)(unsigned long)params->hdr.setup_data;
if (!data)
params->hdr.setup_data = (unsigned long)kaslr_data;
else {
while (data->next)
data = (struct setup_data *)(unsigned long)data->next;
data->next = (unsigned long)kaslr_data;
}

If I misunderstand something, please tell me.

Thanks,
Chao Fan




Re: [PATCH v9 8/8] x86/boot/KASLR: Limit kaslr to choosing the immovable memory

2018-10-22 Thread Chao Fan
On Mon, Oct 22, 2018 at 06:24:55PM +0800, Baoquan He wrote:
>On 10/22/18 at 06:13pm, Chao Fan wrote:
>> >> +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 directely.
>> >> +  */
>> >> + if (num_immovable_mem == 0) {
>> >> + slots_count(region, minimum, image_size);
>> >> +
>> >> + if (slot_area_index == MAX_SLOT_AREA) {
>> >> + debug_putstr("Aborted e820/efi memmap scan (slot_areas 
>> >> full)!\n");
>> >> + return 1;
>> >> + }
>> >> + return 0;
>> >> + }
>> >> +
>> >> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> >> + /*
>> >> +  * If immovable memory found, filter the intersection between
>> >> +  * immovable memory and region to slots_count.
>> >> +  * Otherwise, go on old code.
>> >
>> >Could you explain more about what is the old code in otherwise case you
>> >want to go on?
>> 
>> Sure,
>> 1. 'movable_node' not specified in cmdline.
>> 2. CONFIG_HOT_REMOVE not difned.
>> 3. Just one node in this machine.
>
>So these cases are not covered by 'if (num_immovable_mem == 0)' code?
They are covered by 'if (num_immovable_mem == 0)' code.
>In thise ifdef code block, where do you handle above three cases?
"go on old code." means above:
+ if (num_immovable_mem == 0) {
+ slots_count(region, minimum, image_size);
+
+ if (slot_area_index == MAX_SLOT_AREA) {
+ debug_putstr("Aborted e820/efi memmap scan (slot_areas 
full)!\n");
+ return 1;
+ }
+ return 0;
+ }

Thanks,
Chao Fan

>
>Thanks
>Baoquan
>
>




Re: [PATCH v9 8/8] x86/boot/KASLR: Limit kaslr to choosing the immovable memory

2018-10-22 Thread Chao Fan
On Thu, Oct 18, 2018 at 12:21:23PM +0800, Baoquan He wrote:
>On 10/17/18 at 06:20pm, Chao Fan wrote:
>> If CONFIG_MEMORY_HOTREMOVE enabled and the amount of immovable
>> memory regions is not zero. Calculate the intersection between memory
>
>This if conditional adverbial clauses is not an complete sentence.
>
>> regions from e820/efi memory table and immovable memory regions.
>   ^ get?
>> 
>> Signed-off-by: Chao Fan 
>> ---
>>  arch/x86/boot/compressed/kaslr.c | 72 +++-
>>  1 file changed, 61 insertions(+), 11 deletions(-)
>> 
>> diff --git a/arch/x86/boot/compressed/kaslr.c 
>> b/arch/x86/boot/compressed/kaslr.c
>> index 0c3567bc231c..3ebb150f61eb 100644
>> --- a/arch/x86/boot/compressed/kaslr.c
>> +++ b/arch/x86/boot/compressed/kaslr.c
>> @@ -101,6 +101,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];
>
>Sorry, Chao. I may not follow your old patch change, why the length of
>immovable_mem is MAX_NUMNODES*2, is there any reason or basis?
>
>> +#endif
>> +
>>  
>>  enum mem_avoid_index {
>>  MEM_AVOID_ZO_RANGE = 0,
>> @@ -577,9 +582,9 @@ static unsigned long slots_fetch_random(void)
>>  return 0;
>>  }
>>  
>> -static void process_mem_region(struct mem_vector *entry,
>> -   unsigned long minimum,
>> -   unsigned long image_size)
>> +static void slots_count(struct mem_vector *entry,
>> +unsigned long minimum,
>> +unsigned long image_size)
>>  {
>>  struct mem_vector region, overlap;
>>  unsigned long start_orig, end;
>> @@ -655,6 +660,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 directely.
>> + */
>> +if (num_immovable_mem == 0) {
>> +slots_count(region, minimum, image_size);
>> +
>> +if (slot_area_index == MAX_SLOT_AREA) {
>> +debug_putstr("Aborted e820/efi memmap scan (slot_areas 
>> full)!\n");
>> +return 1;
>> +}
>> +return 0;
>> +}
>> +
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +/*
>> + * If immovable memory found, filter the intersection between
>> + * immovable memory and region to slots_count.
>> + * Otherwise, go on old code.
>
>Could you explain more about what is the old code in otherwise case you
>want to go on?

Sure,
1. 'movable_node' not specified in cmdline.
2. CONFIG_HOT_REMOVE not difned.
3. Just one node in this machine.
>
>> + */
>> +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, _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;
>> +
>> +slots_count(, minimum, image_size);
>
>Obviously, your patch log only covers this place of code. About renaming
>process_mem_region() to  slot_count(), and add another level of wrapper 
>process_mem_region(), may also need be mentioned in patch log.
>
Sorry for missing the comment.
Rename process_mem_region to slots_count to match slots_fetch_random,
and name new function as process_mem_region.

Thanks,
Chao Fan

>> +
>> +if (slot_area_index == MAX_SLOT_AREA) {
>> +debug_putstr("Aborted e820/efi memmap scan (slot_areas 
>> full)!\n");
>> +   

[PATCH v10 5/7] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from kexec

2018-10-22 Thread Chao Fan
Imitate setup_acpi_rsdp() for the early_param of "acpi_rsdp".
KEXEC writes the RSDP pointer to cmdline for EFI booting.
So if "acpi_rsdp" found in cmdline, use it directely.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/acpitb.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/arch/x86/boot/compressed/acpitb.c 
b/arch/x86/boot/compressed/acpitb.c
index 50fa65cf824d..fa63a584d7ec 100644
--- a/arch/x86/boot/compressed/acpitb.c
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -8,6 +8,9 @@
 #include 
 #include 
 
+#define STATIC
+#include 
+
 /* Search EFI table for RSDP table. */
 static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
 {
@@ -200,3 +203,23 @@ static void bios_get_rsdp_addr(acpi_physical_address 
*rsdp_addr)
*rsdp_addr = (acpi_physical_address)address;
}
 }
+
+static void get_acpi_rsdp(acpi_physical_address *rsdp_addr)
+{
+#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 == -1)
+   return;
+
+   if (len > 0) {
+   val[len] = 0;
+   *rsdp_addr = (acpi_physical_address)kstrtoull(val, 16, );
+   }
+#endif
+}
-- 
2.17.2





[PATCH v10 6/7] x86/boot: Dig out SRAT table from RSDP and find immovable memory

2018-10-22 Thread Chao Fan
To avoid KASLR extracting kernel on movable memory, and slove the
conflict between KASLR and movable_node feature, dig the SRAT tables
from RSDP pointer. Walk the SRAT tables and store the immovable
memory regions in immovable_mem[].

The code is imitated from ACPI code, based on acpi_os_get_root_pointer().
Process: RSDP->RSDT/XSDT->ACPI root table->SRAT.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/Makefile |   4 +
 arch/x86/boot/compressed/acpitb.c | 127 ++
 arch/x86/boot/compressed/kaslr.c  |   4 -
 arch/x86/boot/compressed/misc.h   |  15 
 4 files changed, 146 insertions(+), 4 deletions(-)

diff --git a/arch/x86/boot/compressed/Makefile 
b/arch/x86/boot/compressed/Makefile
index 28764dacf018..0f631c5613d7 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -83,6 +83,10 @@ ifdef CONFIG_X86_64
vmlinux-objs-y += $(obj)/pgtable_64.o
 endif
 
+#if (defined CONFIG_MEMORY_HOTREMOVE) && (defined CONFIG_RANDOMIZE_BASE)
+vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/acpitb.o
+#endif
+
 $(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 fa63a584d7ec..a9dbd42e5ab9 100644
--- a/arch/x86/boot/compressed/acpitb.c
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -11,6 +11,11 @@
 #define STATIC
 #include 
 
+#ifdef CONFIG_MEMORY_HOTREMOVE
+/* Store the immovable memory regions */
+struct mem_vector immovable_mem[MAX_NUMNODES*2];
+#endif
+
 /* Search EFI table for RSDP table. */
 static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
 {
@@ -223,3 +228,125 @@ static void get_acpi_rsdp(acpi_physical_address 
*rsdp_addr)
}
 #endif
 }
+
+/*
+ * Used to dig RSDP table from EFI table or BIOS.
+ * If RSDP table found in EFI table, use it. Or search BIOS.
+ * Based on acpi_os_get_root_pointer().
+ */
+static acpi_physical_address get_rsdp_addr(void)
+{
+   acpi_physical_address pa = 0;
+
+   get_acpi_rsdp();
+
+   if (!pa)
+   efi_get_rsdp_addr();
+
+   if (!pa)
+   bios_get_rsdp_addr();
+
+   return pa;
+}
+
+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;
+   char *signature;
+   u8 *entry;
+   u32 count;
+   u32 size;
+   int i, j;
+   u32 len;
+
+   rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
+   if (!rsdp)
+   return NULL;
+
+   /* Get RSDT or XSDT from RSDP. */
+   if (!cmdline_find_option_arg("acpi", "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;
+   len = header->length;
+   count = (u32)((len - sizeof(struct acpi_table_header)) / size);
+   entry = ACPI_ADD_PTR(u8, header, sizeof(struct acpi_table_header));
+
+   for (i = 0; i < count; i++) {
+   u64 address64;
+
+   if (size == ACPI_RSDT_ENTRY_SIZE)
+   acpi_table = ((acpi_physical_address)
+ (*ACPI_CAST_PTR(u32, entry)));
+   else {
+   *(u64 *)(void *) = *(u64 *)(void *)entry;
+   acpi_table = (acpi_physical_address) address64;
+   }
+
+   if (acpi_table) {
+   header = (struct acpi_table_header *)acpi_table;
+   signature = header->signature;
+
+   if (ACPI_COMPARE_NAME(header->signature, ACPI_SIG_SRAT))
+   return header;
+   }
+   entry += size;
+   }
+   return NULL;
+}
+
+/*
+ * According to ACPI table, filter the immvoable 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;
+   int i = 0;
+
+   if (!cmdline_find_option_bool("movable_node") ||
+   cmdline_find_option_arg("acpi", "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_s

[PATCH v10 7/7] x86/boot/KASLR: Limit kaslr to choosing the immovable memory

2018-10-22 Thread Chao Fan
If CONFIG_MEMORY_HOTREMOVE enabled, walk through the SRAT memory
tables and store those immovable memory regions so that KASLR can get
where to choose for randomization.

If the amount of immovable memory regions is not zero, which
means the immovable memory regions existing. Calculate the intersection
between memory regions from e820/efi memory table and immovable memory
regions.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/kaslr.c | 77 +++-
 1 file changed, 66 insertions(+), 11 deletions(-)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index b251572e77af..174d2114045e 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,11 @@ static void mem_avoid_init(unsigned long input, unsigned 
long input_size,
/* Mark the memmap regions we need to avoid */
handle_mem_options();
 
+#ifdef CONFIG_MEMORY_HOTREMOVE
+   /* Mark the immovable regions we need to choose */
+   get_immovable_mem();
+#endif
+
 #ifdef CONFIG_X86_VERBOSE_BOOTUP
/* Make sure video RAM can be used. */
add_identity_map(0, PMD_SIZE);
@@ -568,9 +578,9 @@ static unsigned long slots_fetch_random(void)
return 0;
 }
 
-static void process_mem_region(struct mem_vector *entry,
-  unsigned long minimum,
-  unsigned long image_size)
+static void slots_count(struct mem_vector *entry,
+   unsigned long minimum,
+   unsigned long image_size)
 {
struct mem_vector region, overlap;
unsigned long start_orig, end;
@@ -646,6 +656,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 directely.
+*/
+   if (num_immovable_mem == 0) {
+   slots_count(region, minimum, image_size);
+
+   if (slot_area_index == MAX_SLOT_AREA) {
+   debug_putstr("Aborted e820/efi memmap scan (slot_areas 
full)!\n");
+   return 1;
+   }
+   return 0;
+   }
+
+#ifdef CONFIG_MEMORY_HOTREMOVE
+   /*
+* If immovable memory found, filter the intersection between
+* immovable memory and region to slots_count.
+* 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, _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;
+
+   slots_count(, 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 +772,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(, minimum, image_size);
-   if (slot_area_index == MAX_SLOT_AREA) {
-   debug_putstr("Aborted EFI scan (slot_areas full)!\n");
+   if (process_mem_region(, minimum, image_size))
break;
-   }
}
return true;
 }
@@ -742,11 +800,8 @@ static void process_e820_entries(unsigned long minimum,
continue;
region.start = entry->addr;
region.size = entry->size;
-   process_mem_region(, minimum, image_size);
-   if (slot_area_index == MAX_SLOT_AREA) {
-   debug_putstr("Abort

[PATCH v10 3/7] x86/boot: Add efi_get_rsdp_addr() to dig out RSDP from EFI table

2018-10-22 Thread Chao Fan
Imitate ACPI code and EFI code to dig RSDP pointer from EFI tables.
Process: boot_param->systab->efi_config_table->RSDP.
Based on efi_init(), efi_config_init(), efi_config_parse_tables().

Used for later patch to dig out SRAT table and get the memory
information. So that we can figure out the immovable memory regions
to avoid KASLR extracts kernel on movable memory, so slove the
conflict between KASLR and movable_node feature.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/acpitb.c | 96 +++
 1 file changed, 96 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 ..56b54b0e0889
--- /dev/null
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0
+#define BOOT_CTYPE_H
+#include "misc.h"
+#include "error.h"
+
+#include 
+#include 
+#include 
+#include 
+
+/* Search EFI table for RSDP table. */
+static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
+{
+#ifdef CONFIG_EFI
+   efi_system_table_t *systab;
+   bool efi_64 = false;
+   void *config_tables;
+   struct efi_info *e;
+   char *sig;
+   int size;
+   int i;
+
+   e = _params->efi_info;
+   sig = (char *)>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;
+   }
+
+   /* 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("Table located above 4GB. EFI should be 
disabled.\n");
+   return;
+   }
+   systab = (efi_system_table_t *)e->efi_systab;
+#endif
+
+   if (!systab)
+   return;
+
+   /*
+* Get EFI tables from systab. Based on efi_config_init() and
+* efi_config_parse_tables(). Only dig out the config_table.
+*/
+   size = efi_64 ? sizeof(efi_config_table_64_t) :
+   sizeof(efi_config_table_32_t);
+
+   for (i = 0; i < systab->nr_tables; i++) {
+   efi_guid_t guid;
+   unsigned long table;
+
+   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;
+#ifndef CONFIG_64BIT
+   if (table >> 32) {
+   debug_putstr("Table located above 4G. EFI 
should be disabled.\n");
+   return;
+   }
+#endif
+   } 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;
+   }
+
+   /*
+* Get RSDP from EFI tables.
+* If ACPI20 table found, use it.
+* If ACPI20 table not found, but ACPI table found,
+* use the ACPI table.
+*/
+   if (!(efi_guidcmp(guid, ACPI_TABLE_GUID))) {
+   *rsdp_addr = (acpi_physical_address)table;
+   } else if (!(efi_guidcmp(guid, ACPI_20_TABLE_GUID))) {
+   *rsdp_addr = (acpi_physical_address)table;
+   return;
+   }
+   }
+#endif
+}
-- 
2.17.2





[PATCH v10 1/7] x86/boot: Introduce cmdline_find_option_arg()to detect if option=arg in cmdline

2018-10-22 Thread Chao Fan
Now, there are cmdline_find_option() and cmdline_find_option_bool() in
cmdline.c. Sometimes, when detecting such as whether 'acpi=off' is
in cmdline, we need to cmdline_find_option() first, then compare
the argument. Now splite the operation as a independent function.
Introduce a new function cmdline_find_option_arg() to detect whether
option is in command line and the value is arg.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/cmdline.c | 15 +++
 arch/x86/boot/compressed/misc.h|  1 +
 2 files changed, 16 insertions(+)

diff --git a/arch/x86/boot/compressed/cmdline.c 
b/arch/x86/boot/compressed/cmdline.c
index af6cda0b7900..61118c69feb8 100644
--- a/arch/x86/boot/compressed/cmdline.c
+++ b/arch/x86/boot/compressed/cmdline.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include "misc.h"
+#define STATIC
+#include 
 
 #if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE || CONFIG_X86_5LEVEL
 
@@ -30,5 +32,18 @@ int cmdline_find_option_bool(const char *option)
 {
return __cmdline_find_option_bool(get_cmd_line_ptr(), option);
 }
+bool cmdline_find_option_arg(const char *option, const char *arg, int argsize)
+{
+   char *buffer = malloc(argsize+1);
+   bool find = false;
+   int ret;
+
+   ret = cmdline_find_option(option, buffer, argsize+1);
+   if (ret == argsize && !strncmp(buffer, arg, argsize))
+   find = true;
+
+   free(buffer);
+   return find;
+}
 
 #endif
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index a1d5918765f3..008fdc47a29c 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -67,6 +67,7 @@ static inline void debug_puthex(const char *s)
 /* cmdline.c */
 int cmdline_find_option(const char *option, char *buffer, int bufsize);
 int cmdline_find_option_bool(const char *option);
+bool cmdline_find_option_arg(const char *option, const char *arg, int argsize);
 #endif
 
 
-- 
2.17.2





[PATCH v10 4/7] x86/boot: Add bios_get_rsdp_addr() to search RSDP in memory

2018-10-22 Thread Chao Fan
Imitate ACPI code to search RSDP pointer from memory.
Walk memory and check the signature until get the RSDP signature.
Based on acpi_tb_scan_memory_for_rsdp() and acpi_find_root_pointer().
If didn't get RSDP from EFI table, will run this function.

Used for later patch to dig out SRAT table and get the memory
information. So that we can figure out the immovable memory regions
to avoid KASLR extracts kernel on movable memory, so slove the
conflict between KASLR and movable_node feature.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/acpitb.c | 106 ++
 1 file changed, 106 insertions(+)

diff --git a/arch/x86/boot/compressed/acpitb.c 
b/arch/x86/boot/compressed/acpitb.c
index 56b54b0e0889..50fa65cf824d 100644
--- a/arch/x86/boot/compressed/acpitb.c
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -94,3 +94,109 @@ static void efi_get_rsdp_addr(acpi_physical_address 
*rsdp_addr)
}
 #endif
 }
+
+static u8 compute_checksum(u8 *buffer, u32 length)
+{
+   u8 sum = 0;
+   u8 *end = buffer + length;
+
+   while (buffer < end)
+   sum = (u8)(sum + *(buffer++));
+
+   return sum;
+}
+
+/*
+ * Used to search a block of memory for the RSDP signature.
+ * Return Pointer to the RSDP if found, otherwise NULL.
+ * Based on acpi_tb_scan_memory_for_rsdp().
+ */
+static u8 *scan_mem_for_rsdp(u8 *start, u32 length)
+{
+   struct acpi_table_rsdp *rsdp;
+   u8 *end;
+   u8 *rover;
+
+   end = start + length;
+
+   /* Search from given start address for the requested length */
+   for (rover = start; rover < end; rover += ACPI_RSDP_SCAN_STEP) {
+   /*
+* The RSDP signature and checksum must both 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 *)rover;
+
+   /* Nope, 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;
+
+   /* Sig and checksum valid, we have found a real RSDP */
+   return rover;
+   }
+   return NULL;
+}
+
+/*
+ * Used to search RSDP physical address.
+ * Based on acpi_find_root_pointer(). Since only use physical address
+ * in this period, so there is no need to do the memory map jobs.
+ */
+static void bios_get_rsdp_addr(acpi_physical_address *rsdp_addr)
+{
+   struct acpi_table_rsdp *rsdp;
+   u8 *table_ptr;
+   u8 *mem_rover;
+   u32 address;
+
+   /*
+* Get the location of the Extended BIOS Data Area (EBDA)
+* Since we use physical address directely, so
+* acpi_os_map_memory() and acpi_os_unmap_memory() are
+* not needed here.
+*/
+   table_ptr = (u8 *)ACPI_EBDA_PTR_LOCATION;
+   *(u32 *)(void *) = *(u16 *)(void *)table_ptr;
+   address <<= 4;
+   table_ptr = (u8 *)address;
+
+   /*
+* Search EBDA paragraphs (EBDA is required to be a minimum of
+* 1K length)
+*/
+   if (address > 0x400) {
+   mem_rover = scan_mem_for_rsdp(table_ptr, ACPI_EBDA_WINDOW_SIZE);
+
+   if (mem_rover) {
+   address += (u32)ACPI_PTR_DIFF(mem_rover, table_ptr);
+   *rsdp_addr = (acpi_physical_address)address;
+   return;
+   }
+   }
+
+   table_ptr = (u8 *)ACPI_HI_RSDP_WINDOW_BASE;
+   mem_rover = scan_mem_for_rsdp(table_ptr, ACPI_HI_RSDP_WINDOW_SIZE);
+
+   /*
+* Search upper memory: 16-byte boundaries in Eh-Fh
+* Since we use physical address directely, so
+* acpi_os_map_memory() and acpi_os_unmap_memory() are
+* not needed here.
+*/
+   if (mem_rover) {
+   address = (u32)(ACPI_HI_RSDP_WINDOW_BASE +
+   ACPI_PTR_DIFF(mem_rover, table_ptr));
+   *rsdp_addr = (acpi_physical_address)address;
+   }
+}
-- 
2.17.2





[PATCH v10 0/7] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory

2018-10-22 Thread Chao Fan
***Background:
People reported that kaslr may randomly chooses some positions
which are located in movable memory regions. This will break memory
hotplug feature and make the movable memory chosen by KASLR can't be
removed.

***Solutions:
There should be a method to limit kaslr to choosing immovable memory
regions, so there are 2 solutions:
1) Add a kernel parameter to specify the memory regions.
2) Get the information of memory hot-remove, then kaslr will know the
   right regions.
In method 2, information about memory hot-remove is in ACPI
tables, which will be parsed after start_kernel(), kaslr can't get
the information.
In method 1, users should know the regions address and specify in
kernel parameter.

In the earliest time, I tried to dig ACPI tabls to solve this problem.
But I didn't splite the code in 'compressed/' and ACPI code, so the patch
is hard to follow so refused by community.
Somebody suggest to add a kernel parameter to specify the
immovable memory so that limit kaslr in these regions. Then I make
a new 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/7 Introduce cmdline_find_option_arg() to detect if option=arg
  in cmdline
PATCH 2/7 Copy kstrtoull() to compressed period, used to convert memory
  address from string to unsigned long long.
PATCH 3/7 Add efi_get_rsdp_addr() to dig out RSDP from EFI table when
  booting from EFI.
PATCH 4/7 Add bios_get_rsdp_addr() to search RSDP in memory when EFI
  table not found.
PATCH 5/7 Add get_acpi_rsdp() to parse RSDP in cmdline from kexec
PATCH 6/7 Dig out SRAT table from RSDP and walk SRAT table to store
  the immovable memory regions.
PATCH 7/7 Calculate the intersection between memory regions from e820/efi
  memory table and immovable memory regions. Limit KASLR choose
  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' suggetion:
 - 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.

Any comments will be welcome.

Chao Fan (7):
  x86/boot: Introduce cmdline_find_option_arg()to detect if option=arg
in cmdline
  x86/boot: Copy kstrtoull() to compressed period
  x86/boot: Add efi_get_rsdp_addr() to dig out RSDP from EFI table
  x86/boot: Add bios_get_rsdp_addr() to search RSDP in memory
  x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdlien from kexec
  x86/boot: Dig out SRAT table from RSDP and find immovable memory
  x86/boot/KASLR: Limit kaslr to choosing the immovable memory

 arch/x86/boot/compressed/Makefile  |   4 +
 arch/x86/boot/compressed/acpitb.c  | 352 +
 arch/x86/boot/compressed/cmdline.c |  15 ++
 arch/x86/boot/compressed/kaslr.c   |  81 +--
 arch/x86/boot/compressed/misc.c|  88 
 arch/x86/boot/compressed/misc.h|  20 ++
 6 files changed, 545 insertions(+), 15 deletions(-)
 create mode 100644 arch/x86/boot/compressed/acpitb.c

-- 
2.17.2





Re: [PATCH v9 5/8] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdlien from kexec

2018-10-22 Thread Chao Fan
On Mon, Oct 22, 2018 at 02:06:13PM +0800, Baoquan He wrote:
>On 10/22/18 at 05:30am, Fan, Chao wrote:
>> >> +static void get_acpi_rsdp(acpi_physical_address *rsdp_addr)
>> >> +{
>> >> +#ifdef CONFIG_KEXEC
>> >> + unsigned long long res;
>> >> + int len = 0;
>> >> + char *val;
>> >> +
>> >> + val = malloc(20);
>> >
>> >Why is the length 20? Defined a macro?
>> >
>> 
>> Not a calculation, if it's enough to store the address, that will be OK.
>
>Sorry, I didn't catch. It's 16 in setup_acpi_rsdp() of
>drivers/acpi/osl.c . What does 'that' mean?

The second parameter of kstrtoull(), the 16 you mentioned means
hexadecimal, not the length.
I checked my host and guest, the value are ACPI20=0xbfbfa014, ACPI20=0xdb807000.
The length of memory is 8. Well the max memory address is 16, add
"0x" and '\0' is 19. So I set it as 20.
I am not sure whether 8 is enough for the address, if OK, 11 will
be enough, or 19 is OK.
If my understanding is wrong, please tell me.

Thanks,
Chao Fan

>
>Wondering why not making it 200, it's also enough to store the address.
>
>




Re: [PATCH v9 1/8] x86/boot: Introduce cmdline_find_option_arg()to detect if option=arg in cmdline

2018-10-18 Thread Chao Fan
On Thu, Oct 18, 2018 at 12:01:20PM +0800, Baoquan He wrote:
>On 10/17/18 at 06:20pm, Chao Fan wrote:
>> +bool cmdline_find_option_arg(const char *option, const char *arg, int 
>> argsize)
>> +{
>> +char *buffer = malloc(argsize+1);
>> +bool find = false;
>> +int ret;
>> +
>> +ret = cmdline_find_option(option, buffer, argsize+1);
>> +if (ret == argsize && !strncmp(buffer, arg, argsize))
>> +find = true;
>
>Wondering if we really need a wrapper like this. 

Well, my opinion is there are many code in kaslr.c can replaced by
cmdline_find_option_bool() and cmdline_find_option() and this new
cmdline_find_option_arg().
If this function introduced, we can clean up more code.

Thanks,
Chao Fan

>
>> +
>> +free(buffer);
>> +return find;
>> +}
>>  
>>  #endif
>> diff --git a/arch/x86/boot/compressed/misc.h 
>> b/arch/x86/boot/compressed/misc.h
>> index a1d5918765f3..008fdc47a29c 100644
>> --- a/arch/x86/boot/compressed/misc.h
>> +++ b/arch/x86/boot/compressed/misc.h
>> @@ -67,6 +67,7 @@ static inline void debug_puthex(const char *s)
>>  /* cmdline.c */
>>  int cmdline_find_option(const char *option, char *buffer, int bufsize);
>>  int cmdline_find_option_bool(const char *option);
>> +bool cmdline_find_option_arg(const char *option, const char *arg, int 
>> argsize);
>>  #endif
>>  
>>  
>> -- 
>> 2.17.2
>> 
>> 
>> 
>
>




Re: [PATCH v9 6/8] x86/boot: Dig out SRAT table from RSDP and find immovable memory

2018-10-18 Thread Chao Fan
On Wed, Oct 17, 2018 at 06:20:10PM +0800, Chao Fan wrote:
>Dig out SRAT table from RSDP, and then walk all memory to find
>the immovable memory regions, and fill in the immovable_mem[].
>So that we can use it to select memory for KASLR.
>
>Signed-off-by: Chao Fan 
>---
> arch/x86/boot/compressed/Makefile |   4 +
> arch/x86/boot/compressed/acpitb.c | 129 ++
> arch/x86/boot/compressed/misc.h   |  10 +++
> 3 files changed, 143 insertions(+)
>
>diff --git a/arch/x86/boot/compressed/Makefile 
>b/arch/x86/boot/compressed/Makefile
>index 28764dacf018..f67674d7d2bd 100644
>--- a/arch/x86/boot/compressed/Makefile
>+++ b/arch/x86/boot/compressed/Makefile
>@@ -83,6 +83,10 @@ ifdef CONFIG_X86_64
>   vmlinux-objs-y += $(obj)/pgtable_64.o
> endif
> 
>+#ifdef CONFIG_MEMORY_HOTREMOVE && CONFIG_RANDOMIZE_BASE

Sorry for this, should be
#if (defined CONFIG_MEMORY_HOTREMOVE) && (defined CONFIG_RANDOMIZE_BASE)

I have change it in another file but forget here.

Thanks,
Chao Fan

>+vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/acpitb.o
>+#endif
>+
> $(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 37b1f4407be8..d119663c05bb 100644
>--- a/arch/x86/boot/compressed/acpitb.c
>+++ b/arch/x86/boot/compressed/acpitb.c
>@@ -11,6 +11,15 @@
> #define STATIC
> #include 
> 
>+#ifdef CONFIG_MEMORY_HOTREMOVE
>+struct mem_vector {
>+  unsigned long long start;
>+  unsigned long long size;
>+};
>+/* Store the immovable memory regions */
>+struct mem_vector immovable_mem[MAX_NUMNODES*2];
>+#endif
>+
> /* Search EFI table for RSDP table. */
> static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
> {
>@@ -223,3 +232,123 @@ static void get_acpi_rsdp(acpi_physical_address 
>*rsdp_addr)
>   }
> #endif
> }
>+
>+/*
>+ * Used to dig RSDP table from EFI table or BIOS.
>+ * If RSDP table found in EFI table, use it. Or search BIOS.
>+ * Based on acpi_os_get_root_pointer().
>+ */
>+static acpi_physical_address get_rsdp_addr(void)
>+{
>+  acpi_physical_address pa = 0;
>+
>+  get_acpi_rsdp();
>+
>+  if (!pa)
>+  efi_get_rsdp_addr();
>+
>+  if (!pa)
>+  bios_get_rsdp_addr();
>+
>+  return pa;
>+}
>+
>+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;
>+  char *signature;
>+  u8 *entry;
>+  u32 count;
>+  u32 size;
>+  int i, j;
>+  u32 len;
>+
>+  rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
>+  if (!rsdp)
>+  return NULL;
>+
>+  /* Get RSDT or XSDT from RSDP. */
>+  if (!cmdline_find_option_arg("acpi", "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;
>+  len = header->length;
>+  count = (u32)((len - sizeof(struct acpi_table_header)) / size);
>+  entry = ACPI_ADD_PTR(u8, header, sizeof(struct acpi_table_header));
>+
>+  for (i = 0; i < count; i++) {
>+  u64 address64;
>+
>+  if (size == ACPI_RSDT_ENTRY_SIZE)
>+  acpi_table = ((acpi_physical_address)
>+(*ACPI_CAST_PTR(u32, entry)));
>+  else {
>+  *(u64 *)(void *) = *(u64 *)(void *)entry;
>+  acpi_table = (acpi_physical_address) address64;
>+  }
>+
>+  if (acpi_table) {
>+  header = (struct acpi_table_header *)acpi_table;
>+  signature = header->signature;
>+
>+  if (ACPI_COMPARE_NAME(header->signature, ACPI_SIG_SRAT))
>+  return header;
>+  }
>+  entry += size;
>+  }
>+  return NULL;
>+}
>+
>+/*
>+ * According to ACPI table, filter the immvoable memory regions
>+ * and store them in immovable_mem[].
>+ */
>+void get_immovable_mem(void)
>+{
>+  s

Re: [PATCH v9 3/8] x86/boot: Add efi_get_rsdp_addr() to dig out RSDP from EFI table

2018-10-18 Thread Chao Fan
On Thu, Oct 18, 2018 at 01:56:44PM +0800, Baoquan He wrote:
>On 10/18/18 at 01:54pm, Chao Fan wrote:
>> On Thu, Oct 18, 2018 at 12:35:39PM +0800, Baoquan He wrote:
>> >On 10/17/18 at 06:20pm, Chao Fan wrote:
>> >> There is a bug 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. So dig SRAT table from ACPI tables to get memory information.
>> >
>> >This patch is only adding a function efi_get_rsdp_addr() which will be
>> >used in later patch. Do we need to describe bug here?
>> 
>> Yes, it's the first mail of this serial, so I add more log.
>
>patch 3/8?

Since 1/8 and 2/8 are two functions introduced.
I will consider more about the log.

Thanks,
Chao Fan

>
>
>




Re: [PATCH v9 2/8] x86/boot: Copy kstrtoull() to compressed period

2018-10-18 Thread Chao Fan
On Thu, Oct 18, 2018 at 02:01:28PM +0800, Baoquan He wrote:
>On 10/18/18 at 01:51pm, Chao Fan wrote:
>> On Thu, Oct 18, 2018 at 12:03:38PM +0800, Baoquan He wrote:
>> >On 10/17/18 at 06:20pm, Chao Fan wrote:
>> >> Copy kstrtoull() to 'compressed' directory so that
>> >> we can use it to change the address in cmdline from
>> >> string to unsigned long long.
>> >
>> >So you don't like simple_strtoull() in arch/x86/boot/string.c which has
>> >been used in boot/compressed/kaslr.c . Why? Are you going to clean up
>> >simple_strtoull()?
>> 
>> Boris told me use the new function in old version's review.
>> Both work well for me.
>
>OK, then maybe you can clear simple_strtoull() out in a separate patch
>since you have introduced a new one. Or after this patchset.

Yes, after this patchset, some cleanup will be needed.

Thanks,
Chao Fan
>
>




Re: [PATCH v9 7/8] x86/boot/KASLR: Walk srat tables to filter immovable memory

2018-10-17 Thread Chao Fan
On Thu, Oct 18, 2018 at 12:23:20PM +0800, Baoquan He wrote:
>On 10/17/18 at 06:20pm, Chao Fan wrote:
>> If CONFIG_MEMORY_HOTREMOVE enabled, walk through the acpi srat memory
>> tables and store those immovable memory regions so that kaslr can get
>> where to choose for randomization.
>
>This patch only adds invocation of get_immovable_mem() inside
>mem_avoid_init(). It's not doing what you said in patch log. Can it be
>merged with other patch, e.g patch 6 or 8?

Yes, in old version, this patch will do more job.
Later I move more functions to acpitb.c, just this left.
So I can move it to 8/8.

Thanks,
Chao Fan

>
>> 
>> Signed-off-by: Chao Fan 
>> ---
>>  arch/x86/boot/compressed/kaslr.c | 5 +
>>  1 file changed, 5 insertions(+)
>> 
>> diff --git a/arch/x86/boot/compressed/kaslr.c 
>> b/arch/x86/boot/compressed/kaslr.c
>> index 9ed9709d9947..0c3567bc231c 100644
>> --- a/arch/x86/boot/compressed/kaslr.c
>> +++ b/arch/x86/boot/compressed/kaslr.c
>> @@ -417,6 +417,11 @@ static void mem_avoid_init(unsigned long input, 
>> unsigned long input_size,
>>  /* Mark the memmap regions we need to avoid */
>>  handle_mem_options();
>>  
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +/* Mark the immovable regions we need to choose */
>> +get_immovable_mem();
>> +#endif
>> +
>>  #ifdef CONFIG_X86_VERBOSE_BOOTUP
>>  /* Make sure video RAM can be used. */
>>  add_identity_map(0, PMD_SIZE);
>> -- 
>> 2.17.2
>> 
>> 
>> 
>
>




Re: [PATCH v9 3/8] x86/boot: Add efi_get_rsdp_addr() to dig out RSDP from EFI table

2018-10-17 Thread Chao Fan
On Thu, Oct 18, 2018 at 12:35:39PM +0800, Baoquan He wrote:
>On 10/17/18 at 06:20pm, Chao Fan wrote:
>> There is a bug 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. So dig SRAT table from ACPI tables to get memory information.
>
>This patch is only adding a function efi_get_rsdp_addr() which will be
>used in later patch. Do we need to describe bug here?

Yes, it's the first mail of this serial, so I add more log.
>
>Can we focus on what is this function, and why it's written like that,
>and why it's here?

Sure.
I am just afraid lack of log.

Thanks,
Chao Fan

>> 
>> Imitate the ACPI code of parsing ACPI tables to dig and read ACPI
>> tables. Since some operations are not needed here, functions are
>> simplified. Functions will be used to dig only SRAT tables to get
>> information of memory, so that KASLR can the memory in immovable node.
>> 
>
>




Re: [PATCH v9 2/8] x86/boot: Copy kstrtoull() to compressed period

2018-10-17 Thread Chao Fan
On Thu, Oct 18, 2018 at 12:03:38PM +0800, Baoquan He wrote:
>On 10/17/18 at 06:20pm, Chao Fan wrote:
>> Copy kstrtoull() to 'compressed' directory so that
>> we can use it to change the address in cmdline from
>> string to unsigned long long.
>
>So you don't like simple_strtoull() in arch/x86/boot/string.c which has
>been used in boot/compressed/kaslr.c . Why? Are you going to clean up
>simple_strtoull()?

Boris told me use the new function in old version's review.
Both work well for me.

Thanks,
Chao Fan

>
>




Re: [PATCH v9 0/8] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory

2018-10-17 Thread Chao Fan
On Thu, Oct 18, 2018 at 11:59:58AM +0800, Baoquan He wrote:
>On 10/17/18 at 06:20pm, Chao Fan wrote:
>> In the earliest time, I tried to dig ACPI tabls to solve this problem.
>> But I didn't splite the code in 'compressed/' and ACPI code, so the patch
>> is hard to follow so refused by community.
>> Somebody suggest to add a kernel parameter to specify the
>> immovable memory so that limit kaslr in these regions. Then I make
>> a new 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/3 Add acpitb.c to provide functions to parse ACPI code.
>> PATCH 2/3 If CONFIG_MEMORY_HOTREMOVE enabled, walk all nodes and
>>   store the information of immovable memory regions.
>> PATCH 3/3 According to the immovable memory regions, filter the
>>   immovable regions which KASLR can choose.
>
>Chao, seems you didn't update above accordingly.

oops, sorry for that.

Thanks,
Chao Fan

>
>> 
>> 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.
>>  - Simplify two functions in head file.
>> 
>> 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' suggetion:
>>  - 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
>> 
>> Any comments will be welcome.
>> 
>> 
>> Chao Fan (8):
>>   x86/boot: Introduce cmdline_find_option_arg()to detect if option=arg
>> in cmdline
>>   x86/boot: Copy kstrtoull() to compressed period
>>   x86/boot: Add efi_get_rsdp_addr() to dig out RSDP from EFI table
>>   x86/boot: Add bios_get_rsdp_addr() to search RSDP in memory
>>   x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdlien from kexec
>>   x86/boot: Dig out SRAT table from RSDP and find immovable memory
>>   x86/boot/KASLR: Walk srat tables to filter immovable memory
>>   x86/boot/KASLR: Limit kaslr to choosing the immovable memory
>> 
>>  arch/x86/boot/compressed/Makefile  |   4 +
>>  arch/x86/boot/compressed/acpitb.c  | 354 +
>>  arch/x86/boot/compressed/cmdline.c |  15 ++
>>  arch/x86/boot/compressed/kaslr.c   |  77 ++-
>>  arch/x86/boot/compressed/misc.c|  88 +++
>>  arch/x86/boot/compressed/misc.h|  15 ++
>>  6 files changed, 542 insertions(+), 11 deletions(-)
>>  create mode 100644 arch/x86/boot/compressed/acpitb.c
>> 
>> -- 
>> 2.17.2
>> 
>> 
>> 
>
>




[PATCH v9 1/8] x86/boot: Introduce cmdline_find_option_arg()to detect if option=arg in cmdline

2018-10-17 Thread Chao Fan
Introduce a new function cmdline_find_option_arg() to detect whether
option is in command line and the value is arg.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/cmdline.c | 15 +++
 arch/x86/boot/compressed/misc.h|  1 +
 2 files changed, 16 insertions(+)

diff --git a/arch/x86/boot/compressed/cmdline.c 
b/arch/x86/boot/compressed/cmdline.c
index af6cda0b7900..61118c69feb8 100644
--- a/arch/x86/boot/compressed/cmdline.c
+++ b/arch/x86/boot/compressed/cmdline.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include "misc.h"
+#define STATIC
+#include 
 
 #if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE || CONFIG_X86_5LEVEL
 
@@ -30,5 +32,18 @@ int cmdline_find_option_bool(const char *option)
 {
return __cmdline_find_option_bool(get_cmd_line_ptr(), option);
 }
+bool cmdline_find_option_arg(const char *option, const char *arg, int argsize)
+{
+   char *buffer = malloc(argsize+1);
+   bool find = false;
+   int ret;
+
+   ret = cmdline_find_option(option, buffer, argsize+1);
+   if (ret == argsize && !strncmp(buffer, arg, argsize))
+   find = true;
+
+   free(buffer);
+   return find;
+}
 
 #endif
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index a1d5918765f3..008fdc47a29c 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -67,6 +67,7 @@ static inline void debug_puthex(const char *s)
 /* cmdline.c */
 int cmdline_find_option(const char *option, char *buffer, int bufsize);
 int cmdline_find_option_bool(const char *option);
+bool cmdline_find_option_arg(const char *option, const char *arg, int argsize);
 #endif
 
 
-- 
2.17.2





[PATCH v9 0/8] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory

2018-10-17 Thread Chao Fan
***Background:
People reported that kaslr may randomly chooses some positions
which are located in movable memory regions. This will break memory
hotplug feature and make the movable memory chosen by KASLR can't be
removed.

***Solutions:
There should be a method to limit kaslr to choosing immovable memory
regions, so there are 2 solutions:
1) Add a kernel parameter to specify the memory regions.
2) Get the information of memory hot-remove, then kaslr will know the
   right regions.
In method 2, information about memory hot-remove is in ACPI
tables, which will be parsed after start_kernel(), kaslr can't get
the information.
In method 1, users should know the regions address and specify in
kernel parameter.

In the earliest time, I tried to dig ACPI tabls to solve this problem.
But I didn't splite the code in 'compressed/' and ACPI code, so the patch
is hard to follow so refused by community.
Somebody suggest to add a kernel parameter to specify the
immovable memory so that limit kaslr in these regions. Then I make
a new 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/3 Add acpitb.c to provide functions to parse ACPI code.
PATCH 2/3 If CONFIG_MEMORY_HOTREMOVE enabled, walk all nodes and
  store the information of immovable memory regions.
PATCH 3/3 According to the immovable memory regions, filter the
  immovable regions which KASLR can choose.

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.
 - Simplify two functions in head file.

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' suggetion:
 - 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

Any comments will be welcome.


Chao Fan (8):
  x86/boot: Introduce cmdline_find_option_arg()to detect if option=arg
in cmdline
  x86/boot: Copy kstrtoull() to compressed period
  x86/boot: Add efi_get_rsdp_addr() to dig out RSDP from EFI table
  x86/boot: Add bios_get_rsdp_addr() to search RSDP in memory
  x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdlien from kexec
  x86/boot: Dig out SRAT table from RSDP and find immovable memory
  x86/boot/KASLR: Walk srat tables to filter immovable memory
  x86/boot/KASLR: Limit kaslr to choosing the immovable memory

 arch/x86/boot/compressed/Makefile  |   4 +
 arch/x86/boot/compressed/acpitb.c  | 354 +
 arch/x86/boot/compressed/cmdline.c |  15 ++
 arch/x86/boot/compressed/kaslr.c   |  77 ++-
 arch/x86/boot/compressed/misc.c|  88 +++
 arch/x86/boot/compressed/misc.h|  15 ++
 6 files changed, 542 insertions(+), 11 deletions(-)
 create mode 100644 arch/x86/boot/compressed/acpitb.c

-- 
2.17.2





[PATCH v9 4/8] x86/boot: Add bios_get_rsdp_addr() to search RSDP in memory

2018-10-17 Thread Chao Fan
Imitate acpi_find_root_pointer() and acpi_tb_scan_memory_for_rsdp()
to search RSDP table pointer in memory. This function only works
when RSDP not found in EFI table.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/acpitb.c | 106 ++
 1 file changed, 106 insertions(+)

diff --git a/arch/x86/boot/compressed/acpitb.c 
b/arch/x86/boot/compressed/acpitb.c
index 56b54b0e0889..50fa65cf824d 100644
--- a/arch/x86/boot/compressed/acpitb.c
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -94,3 +94,109 @@ static void efi_get_rsdp_addr(acpi_physical_address 
*rsdp_addr)
}
 #endif
 }
+
+static u8 compute_checksum(u8 *buffer, u32 length)
+{
+   u8 sum = 0;
+   u8 *end = buffer + length;
+
+   while (buffer < end)
+   sum = (u8)(sum + *(buffer++));
+
+   return sum;
+}
+
+/*
+ * Used to search a block of memory for the RSDP signature.
+ * Return Pointer to the RSDP if found, otherwise NULL.
+ * Based on acpi_tb_scan_memory_for_rsdp().
+ */
+static u8 *scan_mem_for_rsdp(u8 *start, u32 length)
+{
+   struct acpi_table_rsdp *rsdp;
+   u8 *end;
+   u8 *rover;
+
+   end = start + length;
+
+   /* Search from given start address for the requested length */
+   for (rover = start; rover < end; rover += ACPI_RSDP_SCAN_STEP) {
+   /*
+* The RSDP signature and checksum must both 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 *)rover;
+
+   /* Nope, 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;
+
+   /* Sig and checksum valid, we have found a real RSDP */
+   return rover;
+   }
+   return NULL;
+}
+
+/*
+ * Used to search RSDP physical address.
+ * Based on acpi_find_root_pointer(). Since only use physical address
+ * in this period, so there is no need to do the memory map jobs.
+ */
+static void bios_get_rsdp_addr(acpi_physical_address *rsdp_addr)
+{
+   struct acpi_table_rsdp *rsdp;
+   u8 *table_ptr;
+   u8 *mem_rover;
+   u32 address;
+
+   /*
+* Get the location of the Extended BIOS Data Area (EBDA)
+* Since we use physical address directely, so
+* acpi_os_map_memory() and acpi_os_unmap_memory() are
+* not needed here.
+*/
+   table_ptr = (u8 *)ACPI_EBDA_PTR_LOCATION;
+   *(u32 *)(void *) = *(u16 *)(void *)table_ptr;
+   address <<= 4;
+   table_ptr = (u8 *)address;
+
+   /*
+* Search EBDA paragraphs (EBDA is required to be a minimum of
+* 1K length)
+*/
+   if (address > 0x400) {
+   mem_rover = scan_mem_for_rsdp(table_ptr, ACPI_EBDA_WINDOW_SIZE);
+
+   if (mem_rover) {
+   address += (u32)ACPI_PTR_DIFF(mem_rover, table_ptr);
+   *rsdp_addr = (acpi_physical_address)address;
+   return;
+   }
+   }
+
+   table_ptr = (u8 *)ACPI_HI_RSDP_WINDOW_BASE;
+   mem_rover = scan_mem_for_rsdp(table_ptr, ACPI_HI_RSDP_WINDOW_SIZE);
+
+   /*
+* Search upper memory: 16-byte boundaries in Eh-Fh
+* Since we use physical address directely, so
+* acpi_os_map_memory() and acpi_os_unmap_memory() are
+* not needed here.
+*/
+   if (mem_rover) {
+   address = (u32)(ACPI_HI_RSDP_WINDOW_BASE +
+   ACPI_PTR_DIFF(mem_rover, table_ptr));
+   *rsdp_addr = (acpi_physical_address)address;
+   }
+}
-- 
2.17.2





[PATCH v9 6/8] x86/boot: Dig out SRAT table from RSDP and find immovable memory

2018-10-17 Thread Chao Fan
Dig out SRAT table from RSDP, and then walk all memory to find
the immovable memory regions, and fill in the immovable_mem[].
So that we can use it to select memory for KASLR.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/Makefile |   4 +
 arch/x86/boot/compressed/acpitb.c | 129 ++
 arch/x86/boot/compressed/misc.h   |  10 +++
 3 files changed, 143 insertions(+)

diff --git a/arch/x86/boot/compressed/Makefile 
b/arch/x86/boot/compressed/Makefile
index 28764dacf018..f67674d7d2bd 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -83,6 +83,10 @@ ifdef CONFIG_X86_64
vmlinux-objs-y += $(obj)/pgtable_64.o
 endif
 
+#ifdef CONFIG_MEMORY_HOTREMOVE && CONFIG_RANDOMIZE_BASE
+vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/acpitb.o
+#endif
+
 $(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 37b1f4407be8..d119663c05bb 100644
--- a/arch/x86/boot/compressed/acpitb.c
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -11,6 +11,15 @@
 #define STATIC
 #include 
 
+#ifdef CONFIG_MEMORY_HOTREMOVE
+struct mem_vector {
+   unsigned long long start;
+   unsigned long long size;
+};
+/* Store the immovable memory regions */
+struct mem_vector immovable_mem[MAX_NUMNODES*2];
+#endif
+
 /* Search EFI table for RSDP table. */
 static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
 {
@@ -223,3 +232,123 @@ static void get_acpi_rsdp(acpi_physical_address 
*rsdp_addr)
}
 #endif
 }
+
+/*
+ * Used to dig RSDP table from EFI table or BIOS.
+ * If RSDP table found in EFI table, use it. Or search BIOS.
+ * Based on acpi_os_get_root_pointer().
+ */
+static acpi_physical_address get_rsdp_addr(void)
+{
+   acpi_physical_address pa = 0;
+
+   get_acpi_rsdp();
+
+   if (!pa)
+   efi_get_rsdp_addr();
+
+   if (!pa)
+   bios_get_rsdp_addr();
+
+   return pa;
+}
+
+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;
+   char *signature;
+   u8 *entry;
+   u32 count;
+   u32 size;
+   int i, j;
+   u32 len;
+
+   rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
+   if (!rsdp)
+   return NULL;
+
+   /* Get RSDT or XSDT from RSDP. */
+   if (!cmdline_find_option_arg("acpi", "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;
+   len = header->length;
+   count = (u32)((len - sizeof(struct acpi_table_header)) / size);
+   entry = ACPI_ADD_PTR(u8, header, sizeof(struct acpi_table_header));
+
+   for (i = 0; i < count; i++) {
+   u64 address64;
+
+   if (size == ACPI_RSDT_ENTRY_SIZE)
+   acpi_table = ((acpi_physical_address)
+ (*ACPI_CAST_PTR(u32, entry)));
+   else {
+   *(u64 *)(void *) = *(u64 *)(void *)entry;
+   acpi_table = (acpi_physical_address) address64;
+   }
+
+   if (acpi_table) {
+   header = (struct acpi_table_header *)acpi_table;
+   signature = header->signature;
+
+   if (ACPI_COMPARE_NAME(header->signature, ACPI_SIG_SRAT))
+   return header;
+   }
+   entry += size;
+   }
+   return NULL;
+}
+
+/*
+ * According to ACPI table, filter the immvoable 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;
+   int i = 0;
+
+   if (!cmdline_find_option_bool("movable_node") ||
+   cmdline_find_option_arg("acpi", "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(struc

[PATCH v9 5/8] x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdlien from kexec

2018-10-17 Thread Chao Fan
If KEXEC write the RSDP pointer to cmdline, parse the cmdline
and use it.
Imitate from early_param of "acpi_rsdp".

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/acpitb.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/arch/x86/boot/compressed/acpitb.c 
b/arch/x86/boot/compressed/acpitb.c
index 50fa65cf824d..37b1f4407be8 100644
--- a/arch/x86/boot/compressed/acpitb.c
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -8,6 +8,9 @@
 #include 
 #include 
 
+#define STATIC
+#include 
+
 /* Search EFI table for RSDP table. */
 static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
 {
@@ -200,3 +203,23 @@ static void bios_get_rsdp_addr(acpi_physical_address 
*rsdp_addr)
*rsdp_addr = (acpi_physical_address)address;
}
 }
+
+static void get_acpi_rsdp(acpi_physical_address *rsdp_addr)
+{
+#ifdef CONFIG_KEXEC
+   unsigned long long res;
+   int len = 0;
+   char *val;
+
+   val = malloc(20);
+   len = cmdline_find_option("acpi_rsdp", val, 20);
+
+   if (len == -1)
+   return;
+
+   if (len > 0) {
+   val[len] = 0;
+   *rsdp_addr = (acpi_physical_address)kstrtoull(val, 0, );
+   }
+#endif
+}
-- 
2.17.2





[PATCH v9 3/8] x86/boot: Add efi_get_rsdp_addr() to dig out RSDP from EFI table

2018-10-17 Thread Chao Fan
There is a bug 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. So dig SRAT table from ACPI tables to get memory information.

Imitate the ACPI code of parsing ACPI tables to dig and read ACPI
tables. Since some operations are not needed here, functions are
simplified. Functions will be used to dig only SRAT tables to get
information of memory, so that KASLR can the memory in immovable node.

This function works for EFI. Dig RSDP from EFI tabler, based on efi_init().

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/acpitb.c | 96 +++
 1 file changed, 96 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 ..56b54b0e0889
--- /dev/null
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0
+#define BOOT_CTYPE_H
+#include "misc.h"
+#include "error.h"
+
+#include 
+#include 
+#include 
+#include 
+
+/* Search EFI table for RSDP table. */
+static void efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
+{
+#ifdef CONFIG_EFI
+   efi_system_table_t *systab;
+   bool efi_64 = false;
+   void *config_tables;
+   struct efi_info *e;
+   char *sig;
+   int size;
+   int i;
+
+   e = _params->efi_info;
+   sig = (char *)>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;
+   }
+
+   /* 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("Table located above 4GB. EFI should be 
disabled.\n");
+   return;
+   }
+   systab = (efi_system_table_t *)e->efi_systab;
+#endif
+
+   if (!systab)
+   return;
+
+   /*
+* Get EFI tables from systab. Based on efi_config_init() and
+* efi_config_parse_tables(). Only dig out the config_table.
+*/
+   size = efi_64 ? sizeof(efi_config_table_64_t) :
+   sizeof(efi_config_table_32_t);
+
+   for (i = 0; i < systab->nr_tables; i++) {
+   efi_guid_t guid;
+   unsigned long table;
+
+   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;
+#ifndef CONFIG_64BIT
+   if (table >> 32) {
+   debug_putstr("Table located above 4G. EFI 
should be disabled.\n");
+   return;
+   }
+#endif
+   } 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;
+   }
+
+   /*
+* Get RSDP from EFI tables.
+* If ACPI20 table found, use it.
+* If ACPI20 table not found, but ACPI table found,
+* use the ACPI table.
+*/
+   if (!(efi_guidcmp(guid, ACPI_TABLE_GUID))) {
+   *rsdp_addr = (acpi_physical_address)table;
+   } else if (!(efi_guidcmp(guid, ACPI_20_TABLE_GUID))) {
+   *rsdp_addr = (acpi_physical_address)table;
+   return;
+   }
+   }
+#endif
+}
-- 
2.17.2





[PATCH v9 2/8] x86/boot: Copy kstrtoull() to compressed period

2018-10-17 Thread Chao Fan
Copy kstrtoull() to 'compressed' directory so that
we can use it to change the address in cmdline from
string to unsigned long long.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/misc.c | 88 +
 arch/x86/boot/compressed/misc.h |  4 ++
 2 files changed, 92 insertions(+)

diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 8dd1d5ccae58..5b9b24949337 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -426,3 +426,91 @@ void fortify_panic(const char *name)
 {
error("detected buffer overflow");
 }
+
+#define KSTRTOX_OVERFLOW(1U << 31)
+
+static inline char _tolower(const char c)
+{
+   return c | 0x20;
+}
+
+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;
+}
+
+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;
+}
+
+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, );
+   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;
+}
+
+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/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 008fdc47a29c..40378408d980 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -63,6 +63,10 @@ static inline void debug_puthex(const char *s)
 
 #endif
 
+#if (defined CONFIG_RANDOMIZE_BASE) && (defined CONFIG_RANDOMIZE_BASE)
+int kstrtoull(const char *s, unsigned int base, unsigned long long *res);
+#endif
+
 #if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE
 /* cmdline.c */
 int cmdline_find_option(const char *option, char *buffer, int bufsize);
-- 
2.17.2





[PATCH v9 8/8] x86/boot/KASLR: Limit kaslr to choosing the immovable memory

2018-10-17 Thread Chao Fan
If CONFIG_MEMORY_HOTREMOVE enabled and the amount of immovable
memory regions is not zero. Calculate the intersection between memory
regions from e820/efi memory table and immovable memory regions.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/kaslr.c | 72 +++-
 1 file changed, 61 insertions(+), 11 deletions(-)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 0c3567bc231c..3ebb150f61eb 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -101,6 +101,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,
@@ -577,9 +582,9 @@ static unsigned long slots_fetch_random(void)
return 0;
 }
 
-static void process_mem_region(struct mem_vector *entry,
-  unsigned long minimum,
-  unsigned long image_size)
+static void slots_count(struct mem_vector *entry,
+   unsigned long minimum,
+   unsigned long image_size)
 {
struct mem_vector region, overlap;
unsigned long start_orig, end;
@@ -655,6 +660,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 directely.
+*/
+   if (num_immovable_mem == 0) {
+   slots_count(region, minimum, image_size);
+
+   if (slot_area_index == MAX_SLOT_AREA) {
+   debug_putstr("Aborted e820/efi memmap scan (slot_areas 
full)!\n");
+   return 1;
+   }
+   return 0;
+   }
+
+#ifdef CONFIG_MEMORY_HOTREMOVE
+   /*
+* If immovable memory found, filter the intersection between
+* immovable memory and region to slots_count.
+* 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, _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;
+
+   slots_count(, 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
@@ -720,11 +776,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(, minimum, image_size);
-   if (slot_area_index == MAX_SLOT_AREA) {
-   debug_putstr("Aborted EFI scan (slot_areas full)!\n");
+   if (process_mem_region(, minimum, image_size))
break;
-   }
}
return true;
 }
@@ -751,11 +804,8 @@ static void process_e820_entries(unsigned long minimum,
continue;
region.start = entry->addr;
region.size = entry->size;
-   process_mem_region(, minimum, image_size);
-   if (slot_area_index == MAX_SLOT_AREA) {
-   debug_putstr("Aborted e820 scan (slot_areas full)!\n");
+   if (process_mem_region(, minimum, image_size))
break;
-   }
}
 }
 
-- 
2.17.2





[PATCH v9 7/8] x86/boot/KASLR: Walk srat tables to filter immovable memory

2018-10-17 Thread Chao Fan
If CONFIG_MEMORY_HOTREMOVE enabled, walk through the acpi srat memory
tables and store those immovable memory regions so that kaslr can get
where to choose for randomization.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/kaslr.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 9ed9709d9947..0c3567bc231c 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -417,6 +417,11 @@ static void mem_avoid_init(unsigned long input, unsigned 
long input_size,
/* Mark the memmap regions we need to avoid */
handle_mem_options();
 
+#ifdef CONFIG_MEMORY_HOTREMOVE
+   /* Mark the immovable regions we need to choose */
+   get_immovable_mem();
+#endif
+
 #ifdef CONFIG_X86_VERBOSE_BOOTUP
/* Make sure video RAM can be used. */
add_identity_map(0, PMD_SIZE);
-- 
2.17.2





Re: [PATCH v8 1/3] x86/boot: Add acpitb.c to parse acpi tables

2018-10-16 Thread Chao Fan
On Tue, Oct 16, 2018 at 02:40:36PM +0200, Borislav Petkov wrote:
>On Tue, Oct 16, 2018 at 10:48:44AM +0800, Chao Fan wrote:
>> Sorry for disturbing you again, I want to make sure this detail with you.
>> You mean that I need splite this as a function and put it to
>> cmdline.c, right?
>
>Extract that functionality into a generic helper so that
>handle_mem_options() and your get_acpi_rsdp() can call it instead
>of duplicating the code. Also, why aren't they both using
>cmdline_find_option() directly?

Yes, I found cmdline_find_option() is enough for me.

Thanks,
Chao Fan

>
>If something's missing, extend cmdline_find_option() to serve your
>purposes too instead of copying the same code.
>
>Make more sense?
>
>-- 
>Regards/Gruss,
>Boris.
>
>Good mailing practices for 400: avoid top-posting and trim the reply.
>
>




Re: [PATCH v8 1/3] x86/boot: Add acpitb.c to parse acpi tables

2018-10-15 Thread Chao Fan
On Thu, Oct 11, 2018 at 12:57:08PM +0200, Borislav Petkov wrote:
>On Wed, Oct 10, 2018 at 04:41:17PM +0800, Chao Fan wrote:
[...]
>> +#ifdef CONFIG_KEXEC
>> +static bool get_acpi_rsdp(acpi_physical_address *rsdp_addr)
>> +{
>> +char *args = (char *)get_cmd_line_ptr();
>> +size_t len = strlen((char *)args);
>> +char *tmp_cmdline, *param, *val;
>> +unsigned long long addr = 0;
>> +char *endptr;
>> +
>> +if (!strstr(args, "acpi_rsdp="))
>> +return false;
>> +
>> +tmp_cmdline = malloc(len+1);
>> +if (!tmp_cmdline)
>> +error("Failed to allocate space for tmp_cmdline");
>
>Why do you even need to allocate a tmp cmdline?
>
>Ah, I see what you've done - you've copied handle_mem_options() in
>kaslr.c. Well no, not really.
>
>That functionality needs to get extracted into a separate facility. Oh
>look, there's arch/x86/boot/compressed/cmdline.c which is begging to get
>extended.
>
>:-)
>

Hi Boris,

Sorry for disturbing you again, I want to make sure this detail with you.
You mean that I need splite this as a function and put it to
cmdline.c, right?
If my understand is wrong, please let me know.

Thanks,
Chao Fan





Re: [PATCH v8 1/3] x86/boot: Add acpitb.c to parse acpi tables

2018-10-15 Thread Chao Fan
On Mon, Oct 15, 2018 at 04:26:09PM -0400, Masayoshi Mizuma wrote:
>Hi Chao,
>
>Let me add some suggestions.

Thanks for your review and suggestion.
I will change them in next version.

Thanks,
Chao Fan

>
>On Wed, Oct 10, 2018 at 04:41:17PM +0800, Chao Fan wrote:
>> There is a bug that kaslr may randomly chooses 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. So dig SRAT table from ACPI tables to get memory information.
>> 
>> Imitate the ACPI code of parsing ACPI tables to dig and read ACPI
>> tables. Since some operations are not needed here, functions are
>> simplified. Functions will be used to dig only SRAT tables to get
>> information of memory, so that KASLR can the memory in immovable node.
>> 
>> And also, these functions won't influence the initialization of
>> ACPI after start_kernel().
>> 
>> Since use physical address directely, so acpi_os_map_memory()
>> and acpi_os_unmap_memory() are not needed.
>> 
>> Signed-off-by: Chao Fan 
>> ---
>>  arch/x86/boot/compressed/Makefile |   2 +
>>  arch/x86/boot/compressed/acpitb.c | 405 ++
>>  arch/x86/boot/compressed/misc.h   |   8 +
>>  3 files changed, 415 insertions(+)
>>  create mode 100644 arch/x86/boot/compressed/acpitb.c
>> 
>...cut...
>> +static struct acpi_table_header *get_acpi_srat_table(void)
>> +{
>> +char *args = (char *)get_cmd_line_ptr();
>> +acpi_physical_address acpi_table;
>> +acpi_physical_address root_table;
>> +struct acpi_table_header *header;
>> +struct acpi_table_rsdp *rsdp;
>> +char *signature;
>> +u8 *entry;
>> +u32 count;
>> +u32 size;
>> +int i, j;
>> +u32 len;
>> +
>> +rsdp = (struct acpi_table_rsdp *)get_rsdp_addr();
>> +if (!rsdp)
>> +return NULL;
>> +
>> +/* Get rsdt or xsdt from rsdp. */
>> +if (!strstr(args, "acpi=rsdt") &&
>> +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;
>> +len = header->length;
>> +count = (u32)((len - sizeof(struct acpi_table_header)) / size);
>> +entry = ACPI_ADD_PTR(u8, header, sizeof(struct acpi_table_header));
>> +
>> +for (i = 0; i < count; i++) {
>> +u64 address64;
>> +
>> +if (size == ACPI_RSDT_ENTRY_SIZE)
>> +acpi_table = ((acpi_physical_address)
>> +  (*ACPI_CAST_PTR(u32, entry)));
>> +else {
>> +*(u64 *)(void *) = *(u64 *)(void *)entry;
>> +acpi_table = (acpi_physical_address) address64;
>> +}
>> +
>> +if (acpi_table) {
>> +header = (struct acpi_table_header *)acpi_table;
>
>> +signature = header->signature;
>> +
>> +if (!strncmp(signature, "SRAT", 4))
>
>   if (ACPI_COMPARE_NAME(header->signature, ACPI_SIG_SRAT))
>
>> +return header;
>> +}
>> +entry += size;
>> +}
>> +return NULL;
>> +}
>> +
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +/*
>> + * According to ACPI table, filter the immvoable memory regions
>> + * and store them in immovable_mem[].
>> + */
>> +void get_immovable_mem(void)
>> +{
>> +char *args = (char *)get_cmd_line_ptr();
>> +struct acpi_table_header *table_header;
>> +struct acpi_subtable_header *table;
>> +struct acpi_srat_mem_affinity *ma;
>> +unsigned long table_end;
>> +int i = 0;
>> +
>> +if (!strstr(args, "movable_node") || strstr(args, "acpi=off"))
>> +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 *)
>> +((u

Re: [PATCH v8 1/3] x86/boot: Add acpitb.c to parse acpi tables

2018-10-12 Thread Chao Fan
On Fri, Oct 12, 2018 at 11:46:55AM +0200, Borislav Petkov wrote:
>On Fri, Oct 12, 2018 at 05:36:38PM +0800, Chao Fan wrote:
>> Prefer to compile out entire functions, rather than portions of functions or
>> portions of expressions.  Rather than putting an ifdef in an expression, 
>> factor
>> out part or all of the expression into a separate helper function and apply 
>> the
>> conditional to that function.
>> 
>> So I am puzzled. If my understanding is wrong, please let me know.
>
>If you do it the way I suggested, you simply have one ifdeffery branch
>less. And ifdeffery is ugly. So less clutter. Also, this way you do
>compile out entire functions too.
>
>HTH.

Got it, thanks for your rapid reply.

Thanks,
Chao Fan

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




Re: [PATCH v8 1/3] x86/boot: Add acpitb.c to parse acpi tables

2018-10-12 Thread Chao Fan
On Thu, Oct 11, 2018 at 12:57:08PM +0200, Borislav Petkov wrote:
>On Wed, Oct 10, 2018 at 04:41:17PM +0800, Chao Fan wrote:
[...]
>> + * If ACPI20 table not found, but ACPI table found,
>> + * use the ACPI table and return true.
>> + * If neither ACPI table nor ACPI20 table found,
>> + * return false.
>> + */
>> +if (!(efi_guidcmp(guid, ACPI_TABLE_GUID))) {
>> +*rsdp_addr = (acpi_physical_address)table;
>> +find_rsdp = true;
>> +} else if (!(efi_guidcmp(guid, ACPI_20_TABLE_GUID))) {
>> +*rsdp_addr = (acpi_physical_address)table;
>> +return true;
>> +}
>> +}
>> +return find_rsdp;
>> +}
>> +#else
>> +static bool efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
>> +{
>> +return false;
>> +}
>> +#endif
>
>Instead of doing this, move the ifdef inside the function:
>
>static bool efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
>{
>#ifdef CONFIG_EFI
>
>   /* function body */
>#endif
>}
>

Hi Borislav,

Thank you for review the detail, but may I ask you why this style is better?

Since the Documentation/process/coding-style.rst said:
Instead,
use such conditionals in a header file defining functions for use in those .c
files, providing no-op stub versions in the #else case, and then call those
functions unconditionally from .c files.  The compiler will avoid generating
any code for the stub calls, producing identical results, but the logic will
remain easy to follow.

Prefer to compile out entire functions, rather than portions of functions or
portions of expressions.  Rather than putting an ifdef in an expression, factor
out part or all of the expression into a separate helper function and apply the
conditional to that function.

So I am puzzled. If my understanding is wrong, please let me know.

Thanks,
Chao Fan

>> +
>> +static u8 checksum(u8 *buffer, u32 length)
>
>compute_checksum(...)
>
>> +{
>> +u8 sum = 0;
>> +u8 *end = buffer + length;
>> +
>> +while (buffer < end)
>> +sum = (u8)(sum + *(buffer++));
>> +
>> +return sum;
>> +}
>> +
>> +/*
>> + * Used to search a block of memory for the RSDP signature.
>> + * Return Pointer to the RSDP if found, otherwise NULL.
>> + * Based on acpi_tb_scan_memory_for_rsdp().
>> + */
>> +static u8 *scan_mem_for_rsdp(u8 *start_address, u32 length)
>> +{
>> +struct acpi_table_rsdp *rsdp;
>> +u8 *end_address;
>> +u8 *mem_rover;
>> +
>> +end_address = start_address + length;
>> +
>> +/* Search from given start address for the requested length */
>> +for (mem_rover = start_address; mem_rover < end_address;
>> + mem_rover += ACPI_RSDP_SCAN_STEP) {
>
>Shorten those variable names so that the loop fits on one line.
>
>> +/*
>> + * The RSDP signature and checksum must both 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 *)mem_rover;
>> +
>> +/* Nope, BAD Signature */
>> +if (!ACPI_VALIDATE_RSDP_SIG(rsdp->signature))
>> +continue;
>> +
>> +/* Check the standard checksum */
>> +if (checksum((u8 *) rsdp, ACPI_RSDP_CHECKSUM_LENGTH) != 0)
>
>No need for "!= 0" at the end. Fix all other tests too.
>
>> +continue;
>> +
>> +/* Check extended checksum if table version >= 2 */
>> +if ((rsdp->revision >= 2) &&
>> +(checksum((u8 *) rsdp, ACPI_RSDP_XCHECKSUM_LENGTH) != 0))
>> +continue;
>> +
>> +/* Sig and checksum valid, we have found a real RSDP */
>> +return mem_rover;
>> +}
>> +return NULL;
>> +}
>> +
>> +/*
>> + * Used to search RSDP physical address.
>> + * Based on acpi_find_root_pointer(). Since only use physical address
>> + * in this period, so there is no need to do the memory map jobs.
>> + */
>> +static void bios_get_rsdp_addr(acpi_physical_address *rsdp_addr)
>> +{
>> +struct acpi_table_rsdp *rsdp;
>> +u8 *table_ptr;
>> +u8 *mem_rover;
>> +u32 address;
>> +

Re: [PATCH v8 1/3] x86/boot: Add acpitb.c to parse acpi tables

2018-10-11 Thread Chao Fan
On Thu, Oct 11, 2018 at 12:57:08PM +0200, Borislav Petkov wrote:
>On Wed, Oct 10, 2018 at 04:41:17PM +0800, Chao Fan wrote:
>> There is a bug that kaslr may randomly chooses 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. So dig SRAT table from ACPI tables to get memory information.
>> 
>> Imitate the ACPI code of parsing ACPI tables to dig and read ACPI
>> tables. Since some operations are not needed here, functions are
>> simplified. Functions will be used to dig only SRAT tables to get
>> information of memory, so that KASLR can the memory in immovable node.
>> 
>> And also, these functions won't influence the initialization of
>> ACPI after start_kernel().
>> 
>> Since use physical address directely, so acpi_os_map_memory()
>> and acpi_os_unmap_memory() are not needed.
>> 
>> Signed-off-by: Chao Fan 
>> ---
>>  arch/x86/boot/compressed/Makefile |   2 +
>>  arch/x86/boot/compressed/acpitb.c | 405 ++
>>  arch/x86/boot/compressed/misc.h   |   8 +
>>  3 files changed, 415 insertions(+)
>>  create mode 100644 arch/x86/boot/compressed/acpitb.c
>> 
>> diff --git a/arch/x86/boot/compressed/Makefile 
>> b/arch/x86/boot/compressed/Makefile
>> index 28764dacf018..1609e4efcaed 100644
>> --- a/arch/x86/boot/compressed/Makefile
>> +++ b/arch/x86/boot/compressed/Makefile
>> @@ -83,6 +83,8 @@ ifdef CONFIG_X86_64
>>  vmlinux-objs-y += $(obj)/pgtable_64.o
>>  endif
>>  
>> +vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/acpitb.o
>

So many thanks for your review.

>This should be CONFIG_MEMORY_HOTREMOVE *and* CONFIG_RANDOMIZE_BASE.
>Otherwise we don't need all that code.

Thanks, I will add CONFIG_RANDOMIZE_BASE.
In V7, I ever added CONFIG_MEMORY_HOTREMOVE, then I need add in kaslr.c:

+#ifdef CONFIG_MEMORY_HOTREMOVE
+   /* Mark the immovable regions we need to choose */
+   get_immovable_mem();
+#endif

Then in V8, follow Kees Cook's suggestion, change the #ifdef to the
definition of get_immovable_mem() in acpitb.c
So I drop the CONFIG_MEMORY_HOTREMOVE.

I will splite it to more patch in next version.

Thanks,
Chao Fan

>
>>  $(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
>> new file mode 100644
>> index ..6b869e3f9780
>> --- /dev/null
>> +++ b/arch/x86/boot/compressed/acpitb.c
>> @@ -0,0 +1,405 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#define BOOT_CTYPE_H
>> +#include "misc.h"
>> +#include "error.h"
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +extern unsigned long get_cmd_line_ptr(void);
>> +
>> +#define STATIC
>> +#include 
>> +
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +struct mem_vector {
>> +unsigned long long start;
>> +unsigned long long size;
>> +};
>> +/* Store the immovable memory regions */
>> +struct mem_vector immovable_mem[MAX_NUMNODES*2];
>> +#endif
>> +
>> +#ifdef CONFIG_EFI
>> +/* Search EFI table for rsdp table. */
>> +static bool efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
>> +{
>> +efi_system_table_t *systab;
>> +bool find_rsdp = false;
>> +bool efi_64 = false;
>> +void *config_tables;
>> +struct efi_info *e;
>> +char *sig;
>> +int size;
>> +int i;
>> +
>> +e = _params->efi_info;
>> +sig = (char *)>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 false;
>> +}
>> +
>> +/* Get systab from boot params. Based on efi_init(). */
>> +#ifdef CONFIG_X86_32
>
>Why the efi_64 detection above but the ifdeffery here? Why not test
>efi_64 instead?

The efi_64 is used for the efi table size.

>
>> +if (e->efi_systab_hi || e->efi_memmap_hi) {
>> +debug_putstr("Table located above 4GB, disabling EFI.\n");
>
>Are you disabling EFI? Where?
>
>Ah, I see, this code is copied from arch/x86/platform/efi/efi.c.
>
>

Re: [PATCH v8 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory

2018-10-10 Thread Chao Fan
On Thu, Oct 11, 2018 at 08:29:55AM +0800, Baoquan He wrote:
>On 10/10/18 at 03:44pm, Masayoshi Mizuma wrote:
>> On Wed, Oct 10, 2018 at 05:30:57PM +0800, Baoquan He wrote:
>> > On 10/10/18 at 11:19am, Borislav Petkov wrote:
>> > > On Wed, Oct 10, 2018 at 11:14:26AM +0200, Thomas Gleixner wrote:
>> > > > Yes, it's different, but if the SRAT information is available early, 
>> > > > then
>> > > > the command line parameter can go away because then the required
>> > > > information for Masa's problem is available as well.
>> > > 
>> > > Exactly. And I'd prefer we delayed the command line parameter until we
>> > > figure out we really need it and not expose it to upstream and then
>> > > remove it shortly after.
>> > > 
>> > > So I'd suggest we move Masa's patches to a separate branch and not send
>> > > it up this round.
>> > 
>> > Yes, sounds more reasonable if we can reuse functions in Chao's patch 1/3
>> > to solve the padding issue.
>> 
>> Thanks for your comments! Yes, immovable_mem[num_immovable_mem] in Chao's
>> patch may be useful for calculating the padding size. If so, we don't
>> need the new kernel parameter. It's nice!
>> 
>> Do you happen to have ideas how we access immovable_mem[num_immovable_mem]
>> from arch/x86/mm/kaslr.c ? It is located to arch/x86/boot/compressed/*, so
>> I suppose it is not easy to access it... 
>> I would appreciate if you could give some advice.
>
>Hmm, they are living in different life cycle and space. So we can only
>reuse the code from Chao's patch, but not the variable. Means we need
>go through the SRAT accessing again in arch/x86/mm/kaslr.c and fill
>immovable_mem[num_immovable_mem] for mm/kaslr.c usage, if we decide to
>do like that.

Reading three times is redundant, but reading two times is needed.
Becasue the ACPI code run very stable, but we need more information
before that.
As for Masa's issue, I am wondering whether we can tranfer the
information or only the address of SRAT table from compressed period
to the period after start_kernel().

Thanks,
Chao Fan

>
>Thanks
>Baoquan
>
>




Re: [PATCH v8 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory

2018-10-10 Thread Chao Fan
On Wed, Oct 10, 2018 at 05:06:20PM +0800, Baoquan He wrote:
>Hi Boris,
>
>On 10/10/18 at 10:59am, Borislav Petkov wrote:
>> ... and we just picked up
>> 
>> https://lkml.kernel.org/r/20181001140843.26137-1-msys.miz...@gmail.com
>> 
>> and without having looked at the rest of your stuff, if people accept
>> your solution, we don't need the silly parameter anymore, right?
>> 
>> Which means, we should not rush the whole thing yet until the whole
>> KASLR vs movable memory gets solved properly.
>
>Masa's patches solves the problem in memory region KASLR which later hot
>added memory may be big than the default padding 10 TB.
>
>Chao's patches is trying to fix a conflict between 'movable_node' and
>kernel text KASLR. If 'movable_node' specified, we rely on SRAT to get
>which memory region is movable or immovable, and movable region can be
>hot removed. But if kernel is randomized into movable memory, it can't
>be hot removed any more, this is a regression after KASLR introduced.
>So this is a different issue than Masa's.

Yes, they are two issues.
But if we can get more memory information by the function in
the new file acpi.c, semms it's helfpul to Masa's issue.

Thanks,
Chao Fan

>
>Thanks
>Baoquan
>
>




[PATCH v8 2/3] x86/boot/KASLR: Walk srat tables to filter immovable memory

2018-10-10 Thread Chao Fan
If CONFIG_MEMORY_HOTREMOVE enabled, walk through the acpi srat memory
tables and store those immovable memory regions so that kaslr can get
where to choose for randomization.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/kaslr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 9ed9709d9947..573e582e8709 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -417,6 +417,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);
-- 
2.17.1





[PATCH v8 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory

2018-10-10 Thread Chao Fan
***Background:
People reported that kaslr may randomly chooses some positions
which are located in movable memory regions. This will break memory
hotplug feature and make the movable memory chosen by KASLR can't be
removed.

***Solutions:
There should be a method to limit kaslr to choosing immovable memory
regions, so there are 2 solutions:
1) Add a kernel parameter to specify the memory regions.
2) Get the information of memory hot-remove, then kaslr will know the
   right regions.
In method 2, information about memory hot-remove is in ACPI
tables, which will be parsed after start_kernel(), kaslr can't get
the information.
In method 1, users should know the regions address and specify in
kernel parameter.

In the earliest time, I tried to dig ACPI tabls to solve this problem.
But I didn't splite the code in 'compressed/' and ACPI code, so the patch
is hard to follow so refused by community.
Somebody suggest to add a kernel parameter to specify the
immovable memory so that limit kaslr in these regions. Then I make
a new 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 copy the codes
and functions to 'compressed/' directory, so that kaslr won't
influence the initialization of ACPI.

PATCH 1/3 Add acpitb.c to provide functions to parse ACPI code.
PATCH 2/3 If CONFIG_MEMORY_HOTREMOVE enabled, walk all nodes and
  store the information of immovable memory regions.
PATCH 3/3 According to the immovable memory regions, filter the
  immovable regions which KASLR can choose.

***Test results:
 - I did a very simple test, and it can get the memory information in
   bios and efi KVM guest machine, and put it by early printk. But no
   more tests, so it's with RFC tag.

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.
 - Simplify two functions in head file.

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.

Any comments will be welcome.

Chao Fan (3):
  x86/boot: Add acpitb.c to parse acpi tables
  x86/boot/KASLR: Walk srat tables to filter immovable memory
  x86/boot/KASLR: Limit kaslr to choosing the immovable memory

 arch/x86/boot/compressed/Makefile |   2 +
 arch/x86/boot/compressed/acpitb.c | 405 ++
 arch/x86/boot/compressed/kaslr.c  |  75 +-
 arch/x86/boot/compressed/misc.h   |   8 +
 4 files changed, 479 insertions(+), 11 deletions(-)
 create mode 100644 arch/x86/boot/compressed/acpitb.c

-- 
2.17.1





[PATCH v8 3/3] x86/boot/KASLR: Limit kaslr to choosing the immovable memory

2018-10-10 Thread Chao Fan
If CONFIG_MEMORY_HOTREMOVE enabled and the amount of immovable
memory regions is not zero. Calculate the intersection between memory
regions from e820/efi memory table and immovable memory regions.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/kaslr.c | 72 +++-
 1 file changed, 61 insertions(+), 11 deletions(-)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 573e582e8709..61486aad39b6 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -101,6 +101,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,
@@ -575,9 +580,9 @@ static unsigned long slots_fetch_random(void)
return 0;
 }
 
-static void process_mem_region(struct mem_vector *entry,
-  unsigned long minimum,
-  unsigned long image_size)
+static void slots_count(struct mem_vector *entry,
+   unsigned long minimum,
+   unsigned long image_size)
 {
struct mem_vector region, overlap;
unsigned long start_orig, end;
@@ -653,6 +658,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 directely.
+*/
+   if (num_immovable_mem == 0) {
+   slots_count(region, minimum, image_size);
+
+   if (slot_area_index == MAX_SLOT_AREA) {
+   debug_putstr("Aborted e820/efi memmap scan (slot_areas 
full)!\n");
+   return 1;
+   }
+   return 0;
+   }
+
+#ifdef CONFIG_MEMORY_HOTREMOVE
+   /*
+* If immovable memory found, filter the intersection between
+* immovable memory and region to slots_count.
+* 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, _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;
+
+   slots_count(, 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
@@ -718,11 +774,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(, minimum, image_size);
-   if (slot_area_index == MAX_SLOT_AREA) {
-   debug_putstr("Aborted EFI scan (slot_areas full)!\n");
+   if (process_mem_region(, minimum, image_size))
break;
-   }
}
return true;
 }
@@ -749,11 +802,8 @@ static void process_e820_entries(unsigned long minimum,
continue;
region.start = entry->addr;
region.size = entry->size;
-   process_mem_region(, minimum, image_size);
-   if (slot_area_index == MAX_SLOT_AREA) {
-   debug_putstr("Aborted e820 scan (slot_areas full)!\n");
+   if (process_mem_region(, minimum, image_size))
break;
-   }
}
 }
 
-- 
2.17.1





Re: [PATCH v7 1/3] x86/boot: Add acpitb.c to parse acpi tables

2018-09-24 Thread Chao Fan
On Sat, Sep 22, 2018 at 08:45:12AM -0700, Kees Cook wrote:
>On Thu, Sep 13, 2018 at 3:46 AM, Chao Fan  wrote:
>> There is a bug that kaslr may randomly chooses 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. So dig SRAT table from ACPI tables to get memory information.
>>
>> Imitate the ACPI code of parsing ACPI tables to dig and read ACPI
>> tables. Since some operations are not needed here, functions are
>> simplified. Functions will be used to dig only SRAT tables to get
>> information of memory, so that KASLR can the memory in immovable node.
>>
>> And also, these functions won't influence the initialization of
>> ACPI after start_kernel().
>>
>> Since use physical address directely, so acpi_os_map_memory()
>> and acpi_os_unmap_memory() are not needed.
>>
>> Signed-off-by: Chao Fan 
>> ---
>>  arch/x86/boot/compressed/Makefile |   4 +
>>  arch/x86/boot/compressed/acpitb.c | 401 ++
>
>Does this logic live anywhere else in the kernel already? (i.e. could
>other code be reused?)

Live in uncompressec period, but not completely same.
The map between physical address and virtual address is not needed here.
So I thins it's hard to reuse, that's why I made this new file.

Thanks,
Chao Fan

>
>-Kees
>
>-- 
>Kees Cook
>Pixel Security
>
>




Re: [PATCH v7 3/3] x86/boot/KASLR: Limit kaslr to choosing the immovable memory

2018-09-24 Thread Chao Fan
On Sat, Sep 22, 2018 at 08:48:53AM -0700, Kees Cook wrote:
>On Thu, Sep 13, 2018 at 3:46 AM, Chao Fan  wrote:
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +   /*
>> +* If immovable memory found, filter the intersection between
>> +* immovable memory and region to slots_count.
>> +* 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;
>> +
>> +   start = immovable_mem[i].start;
>> +   end = start + immovable_mem[i].size;
>> +   region_end = region->start + region->size;
>> +
>> +   entry.start = clamp(region->start, start, end);
>> +   entry_end = clamp(region_end, start, end);
>> +
>> +   if (entry.start + image_size < entry_end) {
>
>Can this logic be rewritten to use the existing mem_overlaps() check
>instead? I think that would make it much more readable.
>

Sure, I will reuse mem_overlaps() instead of this logical.

>Otherwise, yes, this all looks fine.

Thank you for the review.

Thanks,
Chao Fan

>
>-Kees
>
>> +   entry.size = entry_end - entry.start;
>> +   slots_count(, 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
>> @@ -720,11 +775,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(, minimum, image_size);
>> -   if (slot_area_index == MAX_SLOT_AREA) {
>> -   debug_putstr("Aborted EFI scan (slot_areas 
>> full)!\n");
>> +   if (process_mem_region(, minimum, image_size))
>> break;
>> -   }
>> }
>> return true;
>>  }
>> @@ -751,11 +803,8 @@ static void process_e820_entries(unsigned long minimum,
>> continue;
>> region.start = entry->addr;
>> region.size = entry->size;
>> -   process_mem_region(, minimum, image_size);
>> -   if (slot_area_index == MAX_SLOT_AREA) {
>> -   debug_putstr("Aborted e820 scan (slot_areas 
>> full)!\n");
>> +   if (process_mem_region(, minimum, image_size))
>> break;
>> -   }
>> }
>>  }
>>
>> --
>> 2.17.1
>>
>>
>>
>
>
>
>-- 
>Kees Cook
>Pixel Security
>
>




[PATCH v7 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory

2018-09-13 Thread Chao Fan
***Background:
People reported that kaslr may randomly chooses some positions
which are located in movable memory regions. This will break memory
hotplug feature and make the movable memory chosen by KASLR can't be
removed.

***Solutions:
There should be a method to limit kaslr to choosing immovable memory
regions, so there are 2 solutions:
1) Add a kernel parameter to specify the memory regions.
2) Get the information of memory hot-remove, then kaslr will know the
   right regions.
In method 2, information about memory hot-remove is in ACPI
tables, which will be parsed after start_kernel(), kaslr can't get
the information.
In method 1, users should know the regions address and specify in
kernel parameter.

In the earliest time, I tried to dig ACPI tabls to solve this problem.
But I didn't splite the code in 'compressed/' and ACPI code, so the patch
is hard to follow so refused by community.
Somebody suggest to add a kernel parameter to specify the
immovable memory so that limit kaslr in these regions. Then I make
a new 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 copy the codes
and functions to 'compressed/' directory, so that kaslr won't
influence the initialization of ACPI.

PATCH 1/3 Add acpitb.c to provide functions to parse ACPI code.
PATCH 2/3 If CONFIG_MEMORY_HOTREMOVE enabled, walk all nodes and
  store the information of immovable memory regions.
PATCH 3/3 According to the immovable memory regions, filter the
  immovable regions which KASLR can choose.

***Test results:
 - I did a very simple test, and it can get the memory information in
   bios and efi KVM guest machine, and put it by early printk. But no
   more tests, so it's with RFC tag.

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.
 - Simplify two functions in head file.

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

Any comments will be welcome.


Chao Fan (3):
  x86/boot: Add acpitb.c to parse acpi tables
  x86/boot/KASLR: Walk srat tables to filter immovable memory
  x86/boot/KASLR: Limit kaslr to choosing the immovable memory

 arch/x86/boot/compressed/Makefile |   4 +
 arch/x86/boot/compressed/acpitb.c | 401 ++
 arch/x86/boot/compressed/kaslr.c  |  76 +-
 arch/x86/boot/compressed/misc.h   |   8 +
 4 files changed, 478 insertions(+), 11 deletions(-)
 create mode 100644 arch/x86/boot/compressed/acpitb.c

-- 
2.17.1





[PATCH v7 2/3] x86/boot/KASLR: Walk srat tables to filter immovable memory

2018-09-13 Thread Chao Fan
If CONFIG_MEMORY_HOTREMOVE enabled, walk through the acpi srat memory
tables and store those immovable memory regions so that kaslr can get
where to choose for randomization.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/kaslr.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 9ed9709d9947..0c3567bc231c 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -417,6 +417,11 @@ static void mem_avoid_init(unsigned long input, unsigned 
long input_size,
/* Mark the memmap regions we need to avoid */
handle_mem_options();
 
+#ifdef CONFIG_MEMORY_HOTREMOVE
+   /* Mark the immovable regions we need to choose */
+   get_immovable_mem();
+#endif
+
 #ifdef CONFIG_X86_VERBOSE_BOOTUP
/* Make sure video RAM can be used. */
add_identity_map(0, PMD_SIZE);
-- 
2.17.1





[PATCH v7 3/3] x86/boot/KASLR: Limit kaslr to choosing the immovable memory

2018-09-13 Thread Chao Fan
If CONFIG_MEMORY_HOTREMOVE enabled and the amount of immovable
memory regions is not zero. Calculate the intersection between memory
regions from e820/efi memory table and immovable memory regions.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/kaslr.c | 71 +++-
 1 file changed, 60 insertions(+), 11 deletions(-)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 0c3567bc231c..0a7ef2daf169 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -101,6 +101,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,
@@ -577,9 +582,9 @@ static unsigned long slots_fetch_random(void)
return 0;
 }
 
-static void process_mem_region(struct mem_vector *entry,
-  unsigned long minimum,
-  unsigned long image_size)
+static void slots_count(struct mem_vector *entry,
+   unsigned long minimum,
+   unsigned long image_size)
 {
struct mem_vector region, overlap;
unsigned long start_orig, end;
@@ -655,6 +660,56 @@ 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 directely.
+*/
+   if (num_immovable_mem == 0) {
+   slots_count(region, minimum, image_size);
+
+   if (slot_area_index == MAX_SLOT_AREA) {
+   debug_putstr("Aborted e820/efi memmap scan (slot_areas 
full)!\n");
+   return 1;
+   }
+   return 0;
+   }
+
+#ifdef CONFIG_MEMORY_HOTREMOVE
+   /*
+* If immovable memory found, filter the intersection between
+* immovable memory and region to slots_count.
+* 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;
+
+   start = immovable_mem[i].start;
+   end = start + immovable_mem[i].size;
+   region_end = region->start + region->size;
+
+   entry.start = clamp(region->start, start, end);
+   entry_end = clamp(region_end, start, end);
+
+   if (entry.start + image_size < entry_end) {
+   entry.size = entry_end - entry.start;
+   slots_count(, 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
@@ -720,11 +775,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(, minimum, image_size);
-   if (slot_area_index == MAX_SLOT_AREA) {
-   debug_putstr("Aborted EFI scan (slot_areas full)!\n");
+   if (process_mem_region(, minimum, image_size))
break;
-   }
}
return true;
 }
@@ -751,11 +803,8 @@ static void process_e820_entries(unsigned long minimum,
continue;
region.start = entry->addr;
region.size = entry->size;
-   process_mem_region(, minimum, image_size);
-   if (slot_area_index == MAX_SLOT_AREA) {
-   debug_putstr("Aborted e820 scan (slot_areas full)!\n");
+   if (process_mem_region(, minimum, image_size))
break;
-   }
}
 }
 
-- 
2.17.1





[PATCH v7 1/3] x86/boot: Add acpitb.c to parse acpi tables

2018-09-13 Thread Chao Fan
There is a bug that kaslr may randomly chooses 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. So dig SRAT table from ACPI tables to get memory information.

Imitate the ACPI code of parsing ACPI tables to dig and read ACPI
tables. Since some operations are not needed here, functions are
simplified. Functions will be used to dig only SRAT tables to get
information of memory, so that KASLR can the memory in immovable node.

And also, these functions won't influence the initialization of
ACPI after start_kernel().

Since use physical address directely, so acpi_os_map_memory()
and acpi_os_unmap_memory() are not needed.

Signed-off-by: Chao Fan 
---
 arch/x86/boot/compressed/Makefile |   4 +
 arch/x86/boot/compressed/acpitb.c | 401 ++
 arch/x86/boot/compressed/misc.h   |   8 +
 3 files changed, 413 insertions(+)
 create mode 100644 arch/x86/boot/compressed/acpitb.c

diff --git a/arch/x86/boot/compressed/Makefile 
b/arch/x86/boot/compressed/Makefile
index 28764dacf018..394d7d93da7a 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -83,6 +83,10 @@ ifdef CONFIG_X86_64
vmlinux-objs-y += $(obj)/pgtable_64.o
 endif
 
+ifdef CONFIG_MEMORY_HOTREMOVE
+vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/acpitb.o
+endif
+
 $(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
new file mode 100644
index ..109c59b82cde
--- /dev/null
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -0,0 +1,401 @@
+// SPDX-License-Identifier: GPL-2.0
+#define BOOT_CTYPE_H
+#include "misc.h"
+#include "error.h"
+
+#include 
+#include 
+#include 
+#include 
+
+extern unsigned long get_cmd_line_ptr(void);
+
+#define STATIC
+#include 
+
+#ifdef CONFIG_MEMORY_HOTREMOVE
+struct mem_vector {
+   unsigned long long start;
+   unsigned long long size;
+};
+/* Store the immovable memory regions */
+struct mem_vector immovable_mem[MAX_NUMNODES*2];
+#endif
+
+#ifdef CONFIG_EFI
+/* Search EFI table for rsdp table. */
+static bool efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
+{
+   efi_system_table_t *systab;
+   bool find_rsdp = false;
+   bool efi_64 = false;
+   void *config_tables;
+   struct efi_info *e;
+   char *sig;
+   int size;
+   int i;
+
+   e = _params->efi_info;
+   sig = (char *)>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 false;
+   }
+
+   /* Get systab from boot params. Based on efi_init(). */
+#ifdef CONFIG_X86_32
+   if (e->efi_systab_hi || e->efi_memmap_hi) {
+   debug_putstr("Table located above 4GB, disabling EFI.\n");
+   return false;
+   }
+   systab = (efi_system_table_t *)e->efi_systab;
+#else
+   systab = (efi_system_table_t *)(
+   e->efi_systab | ((__u64)e->efi_systab_hi<<32));
+#endif
+
+   if (systab == NULL)
+   return false;
+
+   /*
+* Get EFI tables from systab. Based on efi_config_init() and
+* efi_config_parse_tables(). Only dig the config_table.
+*/
+   size = efi_64 ? sizeof(efi_config_table_64_t) :
+   sizeof(efi_config_table_32_t);
+
+   for (i = 0; i < systab->nr_tables; i++) {
+   efi_guid_t guid;
+   unsigned long table;
+
+   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;
+#ifndef CONFIG_64BIT
+   if (table >> 32) {
+   debug_putstr("Table located above 4G, disabling 
EFI.\n");
+   return false;
+   }
+#endif
+   } 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;
+   }
+
+   /*
+* Get rsdp from EFI tables.
+* If ACPI20 table found, use it and return true.
+* If ACPI20 table not found, but ACPI table found,
+ 

Re: [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables

2018-09-10 Thread Chao Fan
On Mon, Sep 10, 2018 at 10:13:49PM +0200, Rafael J. Wysocki wrote:
>On Mon, Sep 10, 2018 at 2:41 PM Chao Fan  wrote:
>>
>> Imitate the ACPI code of parsing ACPI tables to dig and read ACPI
>> tables.
>
>Why?

Sorry for that, I have sent the cover letter to you.

>
>> Since some operations are not needed here,
>
>What operations?

Such as some operations related to the map between physical address
and virtual address. Here in compressed period, use physical address
directely.

>
>> functions are simplified. Functions will be used to dig SRAT tables to get
>> information of memory, so that KASLR can the memory in immovable node.
>
>So do you want to parse SRAT only or do something in addition to that?

Only read and get memory information, won't change SRAT.
So ACPI code will not be changed.

Thanks,
Chao Fan

>
>> And also, these functions won't influence the initialization of
>> ACPI after start_kernel().
>
>




[PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables

2018-09-10 Thread Chao Fan
Imitate the ACPI code of parsing ACPI tables to dig and read ACPI
tables. Since some operations are not needed here, functions are
simplified. Functions will be used to dig SRAT tables to get
information of memory, so that KASLR can the memory in immovable node.

And also, these functions won't influence the initialization of
ACPI after start_kernel().

Signed-off-by: Chao Fan 
Cc: "Rafael J. Wysocki" 
Cc: Len Brown 
Cc: Ard Biesheuvel 
Cc: linux-efi@vger.kernel.org
Cc: linux-a...@vger.kernel.org
---
 arch/x86/boot/compressed/Makefile |   4 +
 arch/x86/boot/compressed/acpitb.c | 382 ++
 arch/x86/boot/compressed/misc.h   |   8 +
 3 files changed, 394 insertions(+)
 create mode 100644 arch/x86/boot/compressed/acpitb.c

diff --git a/arch/x86/boot/compressed/Makefile 
b/arch/x86/boot/compressed/Makefile
index 28764dacf018..394d7d93da7a 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -83,6 +83,10 @@ ifdef CONFIG_X86_64
vmlinux-objs-y += $(obj)/pgtable_64.o
 endif
 
+ifdef CONFIG_MEMORY_HOTREMOVE
+vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/acpitb.o
+endif
+
 $(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
new file mode 100644
index ..66c515401076
--- /dev/null
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -0,0 +1,382 @@
+// SPDX-License-Identifier: GPL-2.0
+#define BOOT_CTYPE_H
+#include "misc.h"
+
+#include 
+#include 
+#include 
+#include 
+
+extern unsigned long get_cmd_line_ptr(void);
+
+#define STATIC
+#include 
+
+#ifdef CONFIG_MEMORY_HOTREMOVE
+struct mem_vector {
+   unsigned long long start;
+   unsigned long long size;
+};
+/* Store the immovable memory regions */
+struct mem_vector immovable_mem[MAX_NUMNODES*2];
+#endif
+
+#ifdef CONFIG_EFI
+/* Search EFI table for rsdp table. */
+static bool efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
+{
+   efi_system_table_t *systab;
+   bool find_rsdp = false;
+   bool efi_64 = false;
+   void *config_tables;
+   struct efi_info *e;
+   char *sig;
+   int size;
+   int i;
+
+   e = _params->efi_info;
+   sig = (char *)>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 false;
+   }
+
+   /* Get systab from boot params. Based on efi_init(). */
+#ifdef CONFIG_X86_32
+   if (e->efi_systab_hi || e->efi_memmap_hi) {
+   debug_putstr("Table located above 4GB, disabling EFI.\n");
+   return false;
+   }
+   systab = (efi_system_table_t *)e->efi_systab;
+#else
+   systab = (efi_system_table_t *)(
+   e->efi_systab | ((__u64)e->efi_systab_hi<<32));
+#endif
+
+   if (systab == NULL)
+   return false;
+
+   /*
+* Get EFI tables from systab. Based on efi_config_init() and
+* efi_config_parse_tables(). Only dig the useful tables but not
+* do the initialization jobs.
+*/
+   size = efi_64 ? sizeof(efi_config_table_64_t) :
+   sizeof(efi_config_table_32_t);
+
+   for (i = 0; i < systab->nr_tables; i++) {
+   efi_guid_t guid;
+   unsigned long table;
+
+   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;
+#ifndef CONFIG_64BIT
+   if (table >> 32) {
+   debug_putstr("Table located above 4G, disabling 
EFI.\n");
+   return false;
+   }
+#endif
+   } 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;
+   }
+
+   /*
+* Get rsdp from EFI tables.
+* If ACPI20 table found, use it and return true.
+* If ACPI20 table not found, but ACPI table found,
+* use the ACPI table and return true.
+* If neither ACPI table nor ACPI20 table found,
+* return false.
+*/
+   if (!(efi_guidcmp(guid, ACPI_TABLE_GUID))) {
+   *rs

Re: [PATCH v8 0/2] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions

2017-08-04 Thread Chao Fan
On Fri, Aug 04, 2017 at 05:22:41PM +0800, Baoquan He wrote:
>On 08/04/17 at 03:52pm, Baoquan He wrote:
>> Sorry, NACK this series, there's error to hang system. Before I just
>> halt system intentionally with error() to check the efi memmap printing,
>> didn't notice this. Checking the cause.
>
>I rebuilt the code or change the one of the replacement back, hang never
>seen again. Not sure if I copied the wrong kernel or any other mistakes.
>Have got a hardware system with efi enabled to try again, see if
>anything wrong will happen. It's really weird. And Chao also is helping
>to try on his side.

Hi Bao,

After testing for 10 times, no problem happened.
Maybe you did something wrong in the first time.

Thanks,
Chao Fan

>
>> 
>> On 08/04/17 at 03:26pm, Baoquan He wrote:
>> > Patch 1/2 is newly added to add efi_memdesc_ptr helper to wrap the
>> > open code which gets the start of efi memmap descriptor and also
>> > explain why it need be done like that, Ingo suggested it. 
>> > 
>> > And also replace several places of the open code with efi_memdesc_ptr
>> > helper.
>> > 
>> > And also use efi_memdesc_ptr in process_efi_entries() which handle efi
>> > mirror issue during KASLR.
>> > 
>> > 
>> > Change:
>> > v7->v8:
>> > Add efi_memdesc_ptr helper to wrap the open code which gets the
>> > start of map descriptor according to Ingo's suggestion.
>> > 
>> > v6->v7:
>> >   Ingo pointed out several incorrect line break issues and unclear
>> >   description of patch log. Correct them and rewrite patch log.
>> > 
>> >   And also rewrite the EFI warning message that if EFI memmap is above
>> >   4G in 32bit system since 32bit system can not handle data above 4G at
>> >   kernel decompression stage. This is suggested by Ingo too.
>> > 
>> > v5->v6:
>> >   Code style issue fix according to Kees's comment.
>> > 
>> >   This is based on tip/x86/boot, patch 1,2,3/4 in v5 post has
>> >   been put into tip/x86/boot now.
>> > 
>> > 
>> > 
>> > Baoquan He (2):
>> >   efi: Introduce efi_memdesc_ptr to get pointer to memmap descriptor
>> >   x86/boot/KASLR: Restrict kernel to be randomized in mirror regions
>> > 
>> >  arch/x86/boot/compressed/eboot.c   |  2 +-
>> >  arch/x86/boot/compressed/kaslr.c   | 68 
>> > +-
>> >  drivers/firmware/efi/libstub/efi-stub-helper.c |  4 +-
>> >  include/linux/efi.h| 19 +++
>> >  4 files changed, 88 insertions(+), 5 deletions(-)
>> > 
>> > -- 
>> > 2.5.5
>> > 
>
>


--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v8 0/2] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions

2017-08-04 Thread Chao Fan
On Fri, Aug 04, 2017 at 05:22:41PM +0800, Baoquan He wrote:
>On 08/04/17 at 03:52pm, Baoquan He wrote:
>> Sorry, NACK this series, there's error to hang system. Before I just
>> halt system intentionally with error() to check the efi memmap printing,
>> didn't notice this. Checking the cause.
>
>I rebuilt the code or change the one of the replacement back, hang never
>seen again. Not sure if I copied the wrong kernel or any other mistakes.
>Have got a hardware system with efi enabled to try again, see if
>anything wrong will happen. It's really weird. And Chao also is helping

Maybe a little later, cause from tommorow, I will be on vacation for 9
days.

Thanks,
Chao Fan

>to try on his side.
>
>> 
>> On 08/04/17 at 03:26pm, Baoquan He wrote:
>> > Patch 1/2 is newly added to add efi_memdesc_ptr helper to wrap the
>> > open code which gets the start of efi memmap descriptor and also
>> > explain why it need be done like that, Ingo suggested it. 
>> > 
>> > And also replace several places of the open code with efi_memdesc_ptr
>> > helper.
>> > 
>> > And also use efi_memdesc_ptr in process_efi_entries() which handle efi
>> > mirror issue during KASLR.
>> > 
>> > 
>> > Change:
>> > v7->v8:
>> > Add efi_memdesc_ptr helper to wrap the open code which gets the
>> > start of map descriptor according to Ingo's suggestion.
>> > 
>> > v6->v7:
>> >   Ingo pointed out several incorrect line break issues and unclear
>> >   description of patch log. Correct them and rewrite patch log.
>> > 
>> >   And also rewrite the EFI warning message that if EFI memmap is above
>> >   4G in 32bit system since 32bit system can not handle data above 4G at
>> >   kernel decompression stage. This is suggested by Ingo too.
>> > 
>> > v5->v6:
>> >   Code style issue fix according to Kees's comment.
>> > 
>> >   This is based on tip/x86/boot, patch 1,2,3/4 in v5 post has
>> >   been put into tip/x86/boot now.
>> > 
>> > 
>> > 
>> > Baoquan He (2):
>> >   efi: Introduce efi_memdesc_ptr to get pointer to memmap descriptor
>> >   x86/boot/KASLR: Restrict kernel to be randomized in mirror regions
>> > 
>> >  arch/x86/boot/compressed/eboot.c   |  2 +-
>> >  arch/x86/boot/compressed/kaslr.c   | 68 
>> > +-
>> >  drivers/firmware/efi/libstub/efi-stub-helper.c |  4 +-
>> >  include/linux/efi.h| 19 +++
>> >  4 files changed, 88 insertions(+), 5 deletions(-)
>> > 
>> > -- 
>> > 2.5.5
>> > 
>
>


--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html