Re: [PATCH v2] efi: take size of partition entry from GPT header
> I suspect you also need a sanity check that the value isn't too small > or stupidly large. What would be the criterion for too large entries? -- Eugene
RE: [PATCH v3] efi: take size of partition entry from GPT header
From: Davidlohr Bueso
> Sent: 12 September 2018 19:55
>
> On Wed, 12 Sep 2018, Eugene Korenevsky wrote:
> > /**
> > * efi_partition(struct parsed_partitions *state)
> > * @state: disk parsed partitions
> >@@ -704,32 +710,36 @@ int efi_partition(struct parsed_partitions *state)
> >
> > pr_debug("GUID Partition Table is valid! Yea!\n");
> >
> >-for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) && i <
> >state->limit-1; i++) {
> >+for (i = 0;
> >+ i < le32_to_cpu(gpt->num_partition_entries) && i < state->limit-1;
> >+ i++) {
>
> Please rearrange this like:
>
>for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) &&
> i < state->limit-1; i++)
Better still cache le32_to_cpu(gpt->num_partition_entries) to a local
so that it only has to be calculated once (even if it gets spilled
to the stack).
It can be bounded by state->limit-1 at the same time.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
RE: [PATCH v2] efi: take size of partition entry from GPT header
From: Karel Zak
> Sent: 12 September 2018 09:39
> On Tue, Sep 11, 2018 at 07:15:27PM +0300, Eugene Korenevsky wrote:
> > +static gpt_entry *get_gpt_entry(gpt_header *gpt_hdr, gpt_entry *ptes, u32
> > index)
> > +{
> > + return (gpt_entry *)((u8 *)ptes + gpt->sizeof_partition_entry * index);
>
> I guess header use LE, so you need:
>
>le32_to_cpu(gpt_hdr->sizeof_partition_entry)
I suspect you also need a sanity check that the value isn't too small
or stupidly large.
In principle slightly short lengths presumably imply that the disk
was formatted with an older standard - so the last fields should be
ignored.
They may not be any such disks - until the on-disk structure is extended
and the kernel structure updated to match.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
[PATCH v7 0/3] x86/boot/KASLR: Parse ACPI table and limit kaslr in immovable memory
***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
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
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
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,
+* use the ACPI table and return true.
+* If neither ACPI table nor ACPI20 table found,
+*
