***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 memory 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 hotremove, 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 patchset. After several versions, Ingo gave a suggestion:
https://www.mail-archive.com/[email protected]/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.
Since I think ACPI code is independent part, so copy the codes
and functions to 'compressed/' directory, so that kaslr won't
influence the initialization of ACPI.
PATCH 1/4 Reuse the head file of linux/acpi.h, and copy a fcuntion from
ACPI code.
PATCH 2/4 Functions to parse ACPI code.
PATCH 3/4 If 'CONFIG_MEMORY_HOTREMOVE' specified, walk all nodes and
store the information of immovable memory regions.
PATCH 4/4 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.
Any comments will be welcome.
Chao Fan (4):
x86/boot: Add acpitb.h to help parse acpi tables
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 | 251 ++++++++++++++++++++++++++++++
arch/x86/boot/compressed/acpitb.h | 7 +
arch/x86/boot/compressed/kaslr.c | 121 ++++++++++++--
4 files changed, 372 insertions(+), 11 deletions(-)
create mode 100644 arch/x86/boot/compressed/acpitb.c
create mode 100644 arch/x86/boot/compressed/acpitb.h
--
2.17.1
In order to parse ACPI tables, reuse the head file linux/acpi.h,
so that the files in 'compressed' directory can read ACPI table
by including this head file.
Signed-off-by: Chao Fan <[email protected]>
---
arch/x86/boot/compressed/acpitb.h | 7 +++++++
1 file changed, 7 insertions(+)
create mode 100644 arch/x86/boot/compressed/acpitb.h
diff --git a/arch/x86/boot/compressed/acpitb.h b/arch/x86/boot/compressed/acpitb.h
new file mode 100644
index 000000000000..f8ab6e5b3e06
--- /dev/null
+++ b/arch/x86/boot/compressed/acpitb.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <linux/acpi.h>
+
+#define ACPI_MAX_TABLES 128
+
+/* Function to get ACPI SRAT table pointer. */
+struct acpi_table_header *get_acpi_srat_table(void);
--
2.17.1
If 'CONFIG_MEMORY_HOTREMOVE' specified, walk the acpi srat memory
tables, store the immovable memory regions, so that kaslr can get
the information abouth where can be selected or not.
If 'CONFIG_MEMORY_HOTREMOVE' not specified, go on the old code.
Signed-off-by: Chao Fan <[email protected]>
---
arch/x86/boot/compressed/kaslr.c | 55 ++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 531c9876f573..4705682caf1f 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -31,6 +31,7 @@
#include "misc.h"
#include "error.h"
+#include "acpitb.h"
#include "../string.h"
#include <generated/compile.h>
@@ -104,6 +105,14 @@ static bool memmap_too_large;
/* Store memory limit specified by "mem=nn[KMG]" or "memmap=nn[KMG]" */
unsigned long long mem_limit = ULLONG_MAX;
+#ifdef CONFIG_MEMORY_HOTREMOVE
+/* Store the immovable memory regions */
+struct mem_vector immovable_mem[MAX_NUMNODES*2];
+
+/* Store the amount of immovable memory regions */
+static int num_immovable_mem;
+#endif
+
enum mem_avoid_index {
MEM_AVOID_ZO_RANGE = 0,
@@ -298,6 +307,47 @@ static int handle_mem_options(void)
return 0;
}
+#ifdef CONFIG_MEMORY_HOTREMOVE
+/*
+ * According to ACPI table, filter the immvoable memory regions
+ * and store them in immovable_mem[].
+ */
+static void handle_immovable_mem(void)
+{
+ struct acpi_table_header *table_header;
+ struct acpi_subtable_header *table;
+ struct acpi_srat_mem_affinity *ma;
+ unsigned long table_size;
+ unsigned long table_end;
+ int i = 0;
+
+ 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));
+
+ table_size = sizeof(struct acpi_subtable_header);
+ while (((unsigned long)table) + table->length < table_end) {
+ if (table->type == 1) {
+ ma = (struct acpi_srat_mem_affinity *)table;
+ if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)) {
+ immovable_mem[i].start = ma->base_address;
+ immovable_mem[i].size = ma->length;
+ i++;
+ }
+ }
+ table = (struct acpi_subtable_header *)
+ ((unsigned long)table + table->length);
+ }
+ num_immovable_mem = i;
+}
+#endif
+
/*
* In theory, KASLR can put the kernel anywhere in the range of [16M, 64T).
* The mem_avoid array is used to store the ranges that need to be avoided
@@ -421,6 +471,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 */
+ handle_immovable_mem();
+#endif
+
#ifdef CONFIG_X86_VERBOSE_BOOTUP
/* Make sure video RAM can be used. */
add_identity_map(0, PMD_SIZE);
--
2.17.1
If 'CONFIG_MEMORY_HOTREMOVE' specified and the account of immovable
memory regions is not zero. Calculate the intersection between memory
regions from e820/efi memory table and immovable memory regions.
Or go on the old code.
Rename process_mem_region to slots_count to match slots_fetch_random,
and name new function as process_mem_region.
Signed-off-by: Chao Fan <[email protected]>
---
arch/x86/boot/compressed/kaslr.c | 66 ++++++++++++++++++++++++++------
1 file changed, 55 insertions(+), 11 deletions(-)
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 4705682caf1f..10bda3a1fcaa 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -631,9 +631,9 @@ static unsigned long slots_fetch_random(void)
return 0;
}
-static void process_mem_region(struct mem_vector *entry,
- unsigned long minimum,
- unsigned long image_size)
+static void slots_count(struct mem_vector *entry,
+ unsigned long minimum,
+ unsigned long image_size)
{
struct mem_vector region, overlap;
struct slot_area slot_area;
@@ -710,6 +710,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)
+{
+#ifdef CONFIG_MEMORY_HOTREMOVE
+ /*
+ * If immovable memory found, filter the intersection between
+ * immovable memory and region to slots_count.
+ * Otherwise, go on old code.
+ */
+ if (num_immovable_mem > 0) {
+ int i;
+
+ 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(&entry, minimum, image_size);
+
+ if (slot_area_index == MAX_SLOT_AREA) {
+ debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
+ return 1;
+ }
+ }
+ }
+ return 0;
+ }
+#endif
+ /*
+ * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
+ * walk all the regions, so use region directely.
+ */
+ slots_count(region, minimum, image_size);
+
+ if (slot_area_index == MAX_SLOT_AREA) {
+ debug_putstr("Aborted e820/efi memmap scan (slot_areas full)!\n");
+ return 1;
+ }
+ return 0;
+}
+
#ifdef CONFIG_EFI
/*
* Returns true if mirror region found (and must have been processed
@@ -775,11 +825,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(®ion, minimum, image_size);
- if (slot_area_index == MAX_SLOT_AREA) {
- debug_putstr("Aborted EFI scan (slot_areas full)!\n");
+ if (process_mem_region(®ion, minimum, image_size))
break;
- }
}
return true;
}
@@ -806,11 +853,8 @@ static void process_e820_entries(unsigned long minimum,
continue;
region.start = entry->addr;
region.size = entry->size;
- process_mem_region(®ion, minimum, image_size);
- if (slot_area_index == MAX_SLOT_AREA) {
- debug_putstr("Aborted e820 scan (slot_areas full)!\n");
+ if (process_mem_region(®ion, minimum, image_size))
break;
- }
}
}
--
2.17.1
Imitate the ACPI code to parse ACPI tables. Functions are simplified
cause some operations are not needed here.
And also, this method won't influence the initialization of ACPI.
Signed-off-by: Chao Fan <[email protected]>
---
arch/x86/boot/compressed/Makefile | 4 +
arch/x86/boot/compressed/acpitb.c | 251 ++++++++++++++++++++++++++++++
2 files changed, 255 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 fa42f895fdde..41017dc98b61 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 000000000000..5f73c8711f89
--- /dev/null
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -0,0 +1,251 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#define BOOT_CTYPE_H
+#include "misc.h"
+#include "acpitb.h"
+
+#include <linux/efi.h>
+#include <asm/efi.h>
+#include <linux/numa.h>
+
+extern unsigned long get_cmd_line_ptr(void);
+
+#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 acpi_20 = false;
+ bool efi_64 = false;
+ void *config_tables;
+ struct efi_info *e;
+ char *sig;
+ int size;
+ int i;
+
+ e = &boot_params->efi_info;
+ sig = (char *)&e->efi_loader_signature;
+
+ if (!strncmp(sig, EFI64_LOADER_SIGNATURE, 4))
+ efi_64 = true;
+ else if (!strncmp(sig, EFI32_LOADER_SIGNATURE, 4))
+ efi_64 = false;
+ else {
+ debug_putstr("Wrong efi loader signature.\n");
+ return false;
+ }
+
+ /* Get systab from boot params. */
+#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. */
+ 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 we find acpi table, go on searching for acpi20 table.
+ * If we didn't get acpi20 table then use acpi table.
+ * If neither acpi table nor acpi20 table found,
+ * return false.
+ */
+ if (!(efi_guidcmp(guid, ACPI_TABLE_GUID)) && !acpi_20) {
+ *rsdp_addr = (acpi_physical_address)table;
+ acpi_20 = false;
+ find_rsdp = true;
+ } else if (!(efi_guidcmp(guid, ACPI_20_TABLE_GUID))) {
+ *rsdp_addr = (acpi_physical_address)table;
+ acpi_20 = true;
+ return true;
+ }
+ }
+ return find_rsdp;
+}
+#else
+static bool efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
+{
+ return false;
+}
+#endif
+
+static u8 checksum(u8 *buffer, u32 length)
+{
+ u8 sum = 0;
+ u8 *end = buffer + length;
+
+ while (buffer < end)
+ sum = (u8)(sum + *(buffer++));
+
+ return sum;
+}
+
+static u8 *scan_memory_for_rsdp(u8 *start_address, u32 length)
+{
+ struct acpi_table_rsdp *rsdp;
+ u8 *end_address;
+ u8 *mem_rover;
+
+ end_address = start_address + length;
+
+ for (mem_rover = start_address; mem_rover < end_address;
+ mem_rover += ACPI_RSDP_SCAN_STEP) {
+ rsdp = (struct acpi_table_rsdp *)mem_rover;
+ if (!ACPI_VALIDATE_RSDP_SIG(rsdp->signature))
+ continue;
+ if (checksum((u8 *) rsdp,
+ ACPI_RSDP_CHECKSUM_LENGTH) != 0)
+ continue;
+ if ((rsdp->revision >= 2) && (checksum((u8 *)
+ rsdp, ACPI_RSDP_XCHECKSUM_LENGTH) != 0))
+ continue;
+ return mem_rover;
+ }
+ return NULL;
+}
+
+static void bios_get_rsdp_addr(acpi_physical_address *rsdp_addr)
+{
+ struct acpi_table_rsdp *rsdp;
+ u32 physical_address;
+ u8 *table_ptr;
+ u8 *mem_rover;
+
+ table_ptr = (u8 *)ACPI_EBDA_PTR_LOCATION;
+ *(u32 *)(void *)&physical_address = *(u16 *)(void *)table_ptr;
+ physical_address <<= 4;
+ table_ptr = (u8 *)(acpi_physical_address)physical_address;
+
+ if (physical_address > 0x400) {
+ mem_rover = scan_memory_for_rsdp(table_ptr,
+ ACPI_EBDA_WINDOW_SIZE);
+
+ if (mem_rover) {
+ physical_address += (u32)ACPI_PTR_DIFF(mem_rover,
+ table_ptr);
+ *rsdp_addr = (acpi_physical_address)physical_address;
+ return;
+ }
+ }
+
+ table_ptr = (u8 *)ACPI_HI_RSDP_WINDOW_BASE;
+ mem_rover = scan_memory_for_rsdp(table_ptr, ACPI_HI_RSDP_WINDOW_SIZE);
+
+ if (mem_rover) {
+ physical_address = (u32)(ACPI_HI_RSDP_WINDOW_BASE +
+ ACPI_PTR_DIFF(mem_rover, table_ptr));
+ *rsdp_addr = (acpi_physical_address)physical_address;
+ return;
+ }
+}
+
+/*
+ * Used to dig rsdp table from efi table or bios.
+ * If rsdp table found in efi table, use it. Or search bios.
+ */
+static acpi_physical_address get_rsdp_addr(void)
+{
+ acpi_physical_address pa = 0;
+ bool status = false;
+
+ status = efi_get_rsdp_addr(&pa);
+
+ if (!status || pa == 0)
+ bios_get_rsdp_addr(&pa);
+
+ return pa;
+}
+
+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;
+ u32 size;
+ u8 *entry;
+ u32 count;
+ 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 *)&address64 = *(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))
+ return header;
+ }
+ entry += size;
+ }
+ return NULL;
+}
--
2.17.1
Hi chao,
On 07/23/18 at 05:29pm, Chao Fan wrote:
> In order to parse ACPI tables, reuse the head file linux/acpi.h,
> so that the files in 'compressed' directory can read ACPI table
> by including this head file.
>
> Signed-off-by: Chao Fan <[email protected]>
> ---
> arch/x86/boot/compressed/acpitb.h | 7 +++++++
> 1 file changed, 7 insertions(+)
> create mode 100644 arch/x86/boot/compressed/acpitb.h
>
> diff --git a/arch/x86/boot/compressed/acpitb.h b/arch/x86/boot/compressed/acpitb.h
> new file mode 100644
> index 000000000000..f8ab6e5b3e06
> --- /dev/null
> +++ b/arch/x86/boot/compressed/acpitb.h
> @@ -0,0 +1,7 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <linux/acpi.h>
> +
> +#define ACPI_MAX_TABLES 128
> +
> +/* Function to get ACPI SRAT table pointer. */
> +struct acpi_table_header *get_acpi_srat_table(void);
Since acpitb.h includes so few lines of code, not sure if we can move
them into .c files directly.
By the way, you might need to rebase this patchset on top of
tip/x86/boot.
Thanks
Baoquan
On Tue, Jul 24, 2018 at 02:02:57PM +0800, Baoquan He wrote:
>Hi chao,
>
>On 07/23/18 at 05:29pm, Chao Fan wrote:
>> In order to parse ACPI tables, reuse the head file linux/acpi.h,
>> so that the files in 'compressed' directory can read ACPI table
>> by including this head file.
>>
>> Signed-off-by: Chao Fan <[email protected]>
>> ---
>> arch/x86/boot/compressed/acpitb.h | 7 +++++++
>> 1 file changed, 7 insertions(+)
>> create mode 100644 arch/x86/boot/compressed/acpitb.h
>>
>> diff --git a/arch/x86/boot/compressed/acpitb.h b/arch/x86/boot/compressed/acpitb.h
>> new file mode 100644
>> index 000000000000..f8ab6e5b3e06
>> --- /dev/null
>> +++ b/arch/x86/boot/compressed/acpitb.h
>> @@ -0,0 +1,7 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#include <linux/acpi.h>
>> +
>> +#define ACPI_MAX_TABLES 128
>> +
>> +/* Function to get ACPI SRAT table pointer. */
>> +struct acpi_table_header *get_acpi_srat_table(void);
>
>Since acpitb.h includes so few lines of code, not sure if we can move
>them into .c files directly.
Both acpitb.c and kaslr.c in my PATCH will use this head file.
And also eboot.h is also simple, so I put this code alone.
>
>By the way, you might need to rebase this patchset on top of
>tip/x86/boot.
OK, now it is based on master of tip.
Will do it in next version.
Thanks,
Chao Fan
>
>Thanks
>Baoquan
>
>
On Tue, Jul 24, 2018 at 02:02:57PM +0800, Baoquan He wrote:
>Hi chao,
>
>On 07/23/18 at 05:29pm, Chao Fan wrote:
>> In order to parse ACPI tables, reuse the head file linux/acpi.h,
>> so that the files in 'compressed' directory can read ACPI table
>> by including this head file.
>>
>> Signed-off-by: Chao Fan <[email protected]>
>> ---
>> arch/x86/boot/compressed/acpitb.h | 7 +++++++
>> 1 file changed, 7 insertions(+)
>> create mode 100644 arch/x86/boot/compressed/acpitb.h
>>
>> diff --git a/arch/x86/boot/compressed/acpitb.h b/arch/x86/boot/compressed/acpitb.h
>> new file mode 100644
>> index 000000000000..f8ab6e5b3e06
>> --- /dev/null
>> +++ b/arch/x86/boot/compressed/acpitb.h
>> @@ -0,0 +1,7 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#include <linux/acpi.h>
>> +
>> +#define ACPI_MAX_TABLES 128
>> +
>> +/* Function to get ACPI SRAT table pointer. */
>> +struct acpi_table_header *get_acpi_srat_table(void);
>
>Since acpitb.h includes so few lines of code, not sure if we can move
>them into .c files directly.
>
>By the way, you might need to rebase this patchset on top of
>tip/x86/boot.
Sorry Baoquan,
I tried to add this patcheset to the tip/x86/boot branch using both 'patch'
command and 'git am'. I found no problem and no offset.
So is there some problems when you use them?
Thanks,
Chao Fan
>
>Thanks
>Baoquan
>
>
On 07/24/18 at 04:36pm, Chao Fan wrote:
> On Tue, Jul 24, 2018 at 02:02:57PM +0800, Baoquan He wrote:
> >Hi chao,
> >
> >On 07/23/18 at 05:29pm, Chao Fan wrote:
> >> In order to parse ACPI tables, reuse the head file linux/acpi.h,
> >> so that the files in 'compressed' directory can read ACPI table
> >> by including this head file.
> >>
> >> Signed-off-by: Chao Fan <[email protected]>
> >> ---
> >> arch/x86/boot/compressed/acpitb.h | 7 +++++++
> >> 1 file changed, 7 insertions(+)
> >> create mode 100644 arch/x86/boot/compressed/acpitb.h
> >>
> >> diff --git a/arch/x86/boot/compressed/acpitb.h b/arch/x86/boot/compressed/acpitb.h
> >> new file mode 100644
> >> index 000000000000..f8ab6e5b3e06
> >> --- /dev/null
> >> +++ b/arch/x86/boot/compressed/acpitb.h
> >> @@ -0,0 +1,7 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 */
> >> +#include <linux/acpi.h>
> >> +
> >> +#define ACPI_MAX_TABLES 128
> >> +
> >> +/* Function to get ACPI SRAT table pointer. */
> >> +struct acpi_table_header *get_acpi_srat_table(void);
> >
> >Since acpitb.h includes so few lines of code, not sure if we can move
> >them into .c files directly.
> >
> >By the way, you might need to rebase this patchset on top of
> >tip/x86/boot.
>
> Sorry Baoquan,
>
> I tried to add this patcheset to the tip/x86/boot branch using both 'patch'
> command and 'git am'. I found no problem and no offset.
> So is there some problems when you use them?
That's great, just to remind.
Hi,
Sorry for disturbance, no reply for a week, any comments?
Thanks,
Chao Fan
On Mon, Jul 23, 2018 at 05:29:04PM +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 memory 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 hotremove, 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 patchset. After several versions, Ingo gave a suggestion:
>https://www.mail-archive.com/[email protected]/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.
>Since I think ACPI code is independent part, so copy the codes
>and functions to 'compressed/' directory, so that kaslr won't
>influence the initialization of ACPI.
>
>PATCH 1/4 Reuse the head file of linux/acpi.h, and copy a fcuntion from
> ACPI code.
>PATCH 2/4 Functions to parse ACPI code.
>PATCH 3/4 If 'CONFIG_MEMORY_HOTREMOVE' specified, walk all nodes and
> store the information of immovable memory regions.
>PATCH 4/4 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.
>
>Any comments will be welcome.
>
>
>Chao Fan (4):
> x86/boot: Add acpitb.h to help parse acpi tables
> 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 | 251 ++++++++++++++++++++++++++++++
> arch/x86/boot/compressed/acpitb.h | 7 +
> arch/x86/boot/compressed/kaslr.c | 121 ++++++++++++--
> 4 files changed, 372 insertions(+), 11 deletions(-)
> create mode 100644 arch/x86/boot/compressed/acpitb.c
> create mode 100644 arch/x86/boot/compressed/acpitb.h
>
>--
>2.17.1
>
Hi Fan,
At 07/23/2018 05:29 PM, Chao Fan wrote:
> If 'CONFIG_MEMORY_HOTREMOVE' specified, walk the acpi srat memory
> tables, store the immovable memory regions, so that kaslr can get
> the information abouth where can be selected or not.
> If 'CONFIG_MEMORY_HOTREMOVE' not specified, go on the old code.
>
> Signed-off-by: Chao Fan <[email protected]>
> ---
> arch/x86/boot/compressed/kaslr.c | 55 ++++++++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 531c9876f573..4705682caf1f 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -31,6 +31,7 @@
>
> #include "misc.h"
> #include "error.h"
> +#include "acpitb.h"
> #include "../string.h"
>
> #include <generated/compile.h>
> @@ -104,6 +105,14 @@ static bool memmap_too_large;
> /* Store memory limit specified by "mem=nn[KMG]" or "memmap=nn[KMG]" */
> unsigned long long mem_limit = ULLONG_MAX;
>
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +/* Store the immovable memory regions */
> +struct mem_vector immovable_mem[];
> +
> +/* Store the amount of immovable memory regions */
> +static int num_immovable_mem;
> +#endif
> +
>
> enum mem_avoid_index {
> MEM_AVOID_ZO_RANGE = 0,
> @@ -298,6 +307,47 @@ static int handle_mem_options(void)
> return 0;
> }
>
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +/*
> + * According to ACPI table, filter the immvoable memory regions
> + * and store them in immovable_mem[].
> + */
> +static void handle_immovable_mem(void)
> +{
> + struct acpi_table_header *table_header;
> + struct acpi_subtable_header *table;
> + struct acpi_srat_mem_affinity *ma;
> + unsigned long table_size;
> + unsigned long table_end;
> + int i = 0;
> +
> + 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));
> +
> + table_size = sizeof(struct acpi_subtable_header);
table_size isn't used, can be remove.
> + while (((unsigned long)table) + table->length < table_end) {
> + if (table->type == 1) {
> + ma = (struct acpi_srat_mem_affinity *)table;
> + if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)) {
> + immovable_mem[i].start = ma->base_address;
> + immovable_mem[i].size = ma->length;
> + i++;
need a check(i < MAX_NUMNODES*2) before doing that in case of
__IndexOutOfBoundsException__ even if it may be impossible in ACPI.
Thanks,
dou
> + }
> + }
> + table = (struct acpi_subtable_header *)
> + ((unsigned long)table + table->length);
> + }
> + num_immovable_mem = i;
> +}
> +#endif
> +
> /*
> * In theory, KASLR can put the kernel anywhere in the range of [16M, 64T).
> * The mem_avoid array is used to store the ranges that need to be avoided
> @@ -421,6 +471,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 */
> + handle_immovable_mem();
> +#endif
> +
> #ifdef CONFIG_X86_VERBOSE_BOOTUP
> /* Make sure video RAM can be used. */
> add_identity_map(0, PMD_SIZE);
>
On Thu, Aug 02, 2018 at 11:47:13AM +0800, Dou Liyang wrote:
>Hi Fan,
>
>At 07/23/2018 05:29 PM, Chao Fan wrote:
>> If 'CONFIG_MEMORY_HOTREMOVE' specified, walk the acpi srat memory
>> tables, store the immovable memory regions, so that kaslr can get
>> the information abouth where can be selected or not.
>> If 'CONFIG_MEMORY_HOTREMOVE' not specified, go on the old code.
>>
>> Signed-off-by: Chao Fan <[email protected]>
>> ---
>> arch/x86/boot/compressed/kaslr.c | 55 ++++++++++++++++++++++++++++++++
>> 1 file changed, 55 insertions(+)
>>
>> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
>> index 531c9876f573..4705682caf1f 100644
>> --- a/arch/x86/boot/compressed/kaslr.c
>> +++ b/arch/x86/boot/compressed/kaslr.c
>> @@ -31,6 +31,7 @@
>> #include "misc.h"
>> #include "error.h"
>> +#include "acpitb.h"
>> #include "../string.h"
>> #include <generated/compile.h>
>> @@ -104,6 +105,14 @@ static bool memmap_too_large;
>> /* Store memory limit specified by "mem=nn[KMG]" or "memmap=nn[KMG]" */
>> unsigned long long mem_limit = ULLONG_MAX;
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +/* Store the immovable memory regions */
>> +struct mem_vector immovable_mem[];
>> +
>> +/* Store the amount of immovable memory regions */
>> +static int num_immovable_mem;
>> +#endif
>> +
>> enum mem_avoid_index {
>> MEM_AVOID_ZO_RANGE = 0,
>> @@ -298,6 +307,47 @@ static int handle_mem_options(void)
>> return 0;
>> }
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +/*
>> + * According to ACPI table, filter the immvoable memory regions
>> + * and store them in immovable_mem[].
>> + */
>> +static void handle_immovable_mem(void)
>> +{
>> + struct acpi_table_header *table_header;
>> + struct acpi_subtable_header *table;
>> + struct acpi_srat_mem_affinity *ma;
>> + unsigned long table_size;
>> + unsigned long table_end;
>> + int i = 0;
>> +
>> + 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));
>> +
>> + table_size = sizeof(struct acpi_subtable_header);
>
>table_size isn't used, can be remove.
>
Thank you very much, will update in next version.
Thanks,
Chao Fan
>> + while (((unsigned long)table) + table->length < table_end) {
>> + if (table->type == 1) {
>> + ma = (struct acpi_srat_mem_affinity *)table;
>> + if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)) {
>> + immovable_mem[i].start = ma->base_address;
>> + immovable_mem[i].size = ma->length;
>> + i++;
>
>need a check(i < MAX_NUMNODES*2) before doing that in case of
>__IndexOutOfBoundsException__ even if it may be impossible in ACPI.
>
>Thanks,
> dou
>> + }
>> + }
>> + table = (struct acpi_subtable_header *)
>> + ((unsigned long)table + table->length);
>> + }
>> + num_immovable_mem = i;
>> +}
>> +#endif
>> +
>> /*
>> * In theory, KASLR can put the kernel anywhere in the range of [16M, 64T).
>> * The mem_avoid array is used to store the ranges that need to be avoided
>> @@ -421,6 +471,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 */
>> + handle_immovable_mem();
>> +#endif
>> +
>> #ifdef CONFIG_X86_VERBOSE_BOOTUP
>> /* Make sure video RAM can be used. */
>> add_identity_map(0, PMD_SIZE);
>>
Hi Fan,
At 07/23/2018 05:29 PM, Chao Fan wrote:
> If 'CONFIG_MEMORY_HOTREMOVE' specified and the account of immovable
> memory regions is not zero. Calculate the intersection between memory
> regions from e820/efi memory table and immovable memory regions.
> Or go on the old code.
>
> Rename process_mem_region to slots_count to match slots_fetch_random,
> and name new function as process_mem_region.
>
> Signed-off-by: Chao Fan <[email protected]>
> ---
> arch/x86/boot/compressed/kaslr.c | 66 ++++++++++++++++++++++++++------
> 1 file changed, 55 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 4705682caf1f..10bda3a1fcaa 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -631,9 +631,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,
^^^^^^^^^^^
is not suitable.
IMO, how about process_mem_slots() or you may have a better name, it's
up to you.
> + unsigned long minimum,
> + unsigned long image_size)
> {
> struct mem_vector region, overlap;
> struct slot_area slot_area;
slot_area is also unused.
Thanks,
dou
On Thu, Aug 02, 2018 at 01:46:29PM +0800, Dou Liyang wrote:
>Hi Fan,
>
>At 07/23/2018 05:29 PM, Chao Fan wrote:
>> If 'CONFIG_MEMORY_HOTREMOVE' specified and the account of immovable
>> memory regions is not zero. Calculate the intersection between memory
>> regions from e820/efi memory table and immovable memory regions.
>> Or go on the old code.
>>
>> Rename process_mem_region to slots_count to match slots_fetch_random,
>> and name new function as process_mem_region.
>>
>> Signed-off-by: Chao Fan <[email protected]>
>> ---
>> arch/x86/boot/compressed/kaslr.c | 66 ++++++++++++++++++++++++++------
>> 1 file changed, 55 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
>> index 4705682caf1f..10bda3a1fcaa 100644
>> --- a/arch/x86/boot/compressed/kaslr.c
>> +++ b/arch/x86/boot/compressed/kaslr.c
>> @@ -631,9 +631,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,
> ^^^^^^^^^^^
> is not suitable.
>IMO, how about process_mem_slots() or you may have a better name, it's
>up to you.
It's from Kees Cook's advise, he wants to ues slots_count() to match
slots_fetch_random() in my old PATCH long long ago.
Since the method of handling this part is not changed a lot, so I keep
this name.
>
>> + unsigned long minimum,
>> + unsigned long image_size)
>> {
>> struct mem_vector region, overlap;
>> struct slot_area slot_area;
>
>slot_area is also unused.
Yes, I will make a PATCH to do the clean jobs.
Thanks,
Chao Fan
>
>Thanks,
> dou
>
At 08/02/2018 02:00 PM, Chao Fan wrote:
> On Thu, Aug 02, 2018 at 01:46:29PM +0800, Dou Liyang wrote:
>> Hi Fan,
>>
>> At 07/23/2018 05:29 PM, Chao Fan wrote:
>>> If 'CONFIG_MEMORY_HOTREMOVE' specified and the account of immovable
>>> memory regions is not zero. Calculate the intersection between memory
>>> regions from e820/efi memory table and immovable memory regions.
>>> Or go on the old code.
>>>
>>> Rename process_mem_region to slots_count to match slots_fetch_random,
>>> and name new function as process_mem_region.
>>>
>>> Signed-off-by: Chao Fan <[email protected]>
>>> ---
>>> arch/x86/boot/compressed/kaslr.c | 66 ++++++++++++++++++++++++++------
>>> 1 file changed, 55 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
>>> index 4705682caf1f..10bda3a1fcaa 100644
>>> --- a/arch/x86/boot/compressed/kaslr.c
>>> +++ b/arch/x86/boot/compressed/kaslr.c
>>> @@ -631,9 +631,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,
>> ^^^^^^^^^^^
>> is not suitable.
>> IMO, how about process_mem_slots() or you may have a better name, it's
>> up to you.
>
> It's from Kees Cook's advise, he wants to ues slots_count() to match
> slots_fetch_random() in my old PATCH long long ago.
> Since the method of handling this part is not changed a lot, so I keep
> this name.
>
Okay! ;-)
dou
On Thu, 2 Aug 2018, Chao Fan wrote:
> On Thu, Aug 02, 2018 at 11:47:13AM +0800, Dou Liyang wrote:
Removed 70 lines of complete useless information....
> >> + table = (struct acpi_subtable_header *)
> >> + ((unsigned long)table_header + sizeof(struct acpi_table_srat));
> >> +
> >> + table_size = sizeof(struct acpi_subtable_header);
> >
> >table_size isn't used, can be remove.
> >
>
> Thank you very much, will update in next version.
And yet another 40 lines.
>
Folks. Can you please both stop this annoying habit of keeping the full
context of the mail and then sprinkling a random sentence into the middle?
Thanks,
tglx
At 08/02/2018 03:05 PM, Thomas Gleixner wrote:
[...]
>
> Folks. Can you please both stop this annoying habit of keeping the full
> context of the mail and then sprinkling a random sentence into the middle?
>
I see. won??t do this stupid thing again.
Thanks,
dou