From 709709ac6410f4a14ded158a4b23b979e33e10fb Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Mon, 27 Jul 2020 19:07:54 -0400 Subject: x86/kaslr: Make command line handling safer Handle the possibility that the command line is NULL. Replace open-coded strlen with a function call. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Reviewed-by: Kees Cook Link: https://lore.kernel.org/r/20200727230801.3468620-2-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index d7408af55738..e0f69f3625ae 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -268,15 +268,19 @@ static void parse_gb_huge_pages(char *param, char *val) static void handle_mem_options(void) { char *args = (char *)get_cmd_line_ptr(); - size_t len = strlen((char *)args); + size_t len; char *tmp_cmdline; char *param, *val; u64 mem_size; + if (!args) + return; + if (!strstr(args, "memmap=") && !strstr(args, "mem=") && !strstr(args, "hugepages")) return; + len = strlen(args); tmp_cmdline = malloc(len + 1); if (!tmp_cmdline) error("Failed to allocate space for tmp_cmdline"); @@ -399,8 +403,7 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size, { unsigned long init_size = boot_params->hdr.init_size; u64 initrd_start, initrd_size; - u64 cmd_line, cmd_line_size; - char *ptr; + unsigned long cmd_line, cmd_line_size; /* * Avoid the region that is unsafe to overlap during @@ -421,16 +424,15 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size, /* No need to set mapping for initrd, it will be handled in VO. */ /* Avoid kernel command line. */ - cmd_line = (u64)boot_params->ext_cmd_line_ptr << 32; - cmd_line |= boot_params->hdr.cmd_line_ptr; + cmd_line = get_cmd_line_ptr(); /* Calculate size of cmd_line. */ - ptr = (char *)(unsigned long)cmd_line; - for (cmd_line_size = 0; ptr[cmd_line_size++];) - ; - mem_avoid[MEM_AVOID_CMDLINE].start = cmd_line; - mem_avoid[MEM_AVOID_CMDLINE].size = cmd_line_size; - add_identity_map(mem_avoid[MEM_AVOID_CMDLINE].start, - mem_avoid[MEM_AVOID_CMDLINE].size); + if (cmd_line) { + cmd_line_size = strlen((char *)cmd_line) + 1; + mem_avoid[MEM_AVOID_CMDLINE].start = cmd_line; + mem_avoid[MEM_AVOID_CMDLINE].size = cmd_line_size; + add_identity_map(mem_avoid[MEM_AVOID_CMDLINE].start, + mem_avoid[MEM_AVOID_CMDLINE].size); + } /* Avoid boot parameters. */ mem_avoid[MEM_AVOID_BOOTPARAMS].start = (unsigned long)boot_params; -- cgit v1.2.3 From e2ee6173162b28053cb76b25887a0be9331c9e21 Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Mon, 27 Jul 2020 19:07:55 -0400 Subject: x86/kaslr: Remove bogus warning and unnecessary goto Drop the warning on seeing "--" in handle_mem_options(). This will trigger whenever one of the memory options is present in the command line together with "--", but there's no problem if that is the case. Replace goto with break. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Reviewed-by: Kees Cook Link: https://lore.kernel.org/r/20200727230801.3468620-3-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index e0f69f3625ae..c31f3a5ab9e4 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -295,10 +295,8 @@ static void handle_mem_options(void) while (*args) { args = next_arg(args, ¶m, &val); /* Stop at -- */ - if (!val && strcmp(param, "--") == 0) { - warn("Only '--' specified in cmdline"); - goto out; - } + if (!val && strcmp(param, "--") == 0) + break; if (!strcmp(param, "memmap")) { mem_avoid_memmap(PARSE_MEMMAP, val); @@ -311,7 +309,7 @@ static void handle_mem_options(void) continue; mem_size = memparse(p, &p); if (mem_size == 0) - goto out; + break; mem_limit = mem_size; } else if (!strcmp(param, "efi_fake_mem")) { @@ -319,7 +317,6 @@ static void handle_mem_options(void) } } -out: free(tmp_cmdline); return; } -- cgit v1.2.3 From 08705365560a352d3f5b4f1f52270b4d4ff7911e Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Mon, 27 Jul 2020 19:07:56 -0400 Subject: x86/kaslr: Fix process_efi_entries comment Since commit: 0982adc74673 ("x86/boot/KASLR: Work around firmware bugs by excluding EFI_BOOT_SERVICES_* and EFI_LOADER_* from KASLR's choice") process_efi_entries() will return true if we have an EFI memmap, not just if it contained EFI_MEMORY_MORE_RELIABLE regions. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Reviewed-by: Kees Cook Link: https://lore.kernel.org/r/20200727230801.3468620-4-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index c31f3a5ab9e4..1ab67a84a781 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -742,8 +742,8 @@ static bool process_mem_region(struct mem_vector *region, #ifdef CONFIG_EFI /* - * Returns true if mirror region found (and must have been processed - * for slots adding) + * Returns true if we processed the EFI memmap, which we prefer over the E820 + * table if it is available. */ static bool process_efi_entries(unsigned long minimum, unsigned long image_size) -- cgit v1.2.3 From 451286940d95778e83fa7f97006316d995b4c4a8 Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Mon, 27 Jul 2020 19:07:57 -0400 Subject: x86/kaslr: Initialize mem_limit to the real maximum address On 64-bit, the kernel must be placed below MAXMEM (64TiB with 4-level paging or 4PiB with 5-level paging). This is currently not enforced by KASLR, which thus implicitly relies on physical memory being limited to less than 64TiB. On 32-bit, the limit is KERNEL_IMAGE_SIZE (512MiB). This is enforced by special checks in __process_mem_region(). Initialize mem_limit to the maximum (depending on architecture), instead of ULLONG_MAX, and make sure the command-line arguments can only decrease it. This makes the enforcement explicit on 64-bit, and eliminates the 32-bit specific checks to keep the kernel below 512M. Check upfront to make sure the minimum address is below the limit before doing any work. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Acked-by: Kees Cook Link: https://lore.kernel.org/r/20200727230801.3468620-5-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 41 +++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 1ab67a84a781..da45e66cb6e4 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -94,8 +94,11 @@ static unsigned long get_boot_seed(void) 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; +/* + * Store memory limit: MAXMEM on 64-bit and KERNEL_IMAGE_SIZE on 32-bit. + * It may be reduced by "mem=nn[KMG]" or "memmap=nn[KMG]" command line options. + */ +static unsigned long long mem_limit; /* Number of immovable memory regions */ static int num_immovable_mem; @@ -221,7 +224,7 @@ static void mem_avoid_memmap(enum parse_mode mode, char *str) if (start == 0) { /* Store the specified memory limit if size > 0 */ - if (size > 0) + if (size > 0 && size < mem_limit) mem_limit = size; continue; @@ -311,7 +314,8 @@ static void handle_mem_options(void) if (mem_size == 0) break; - mem_limit = mem_size; + if (mem_size < mem_limit) + mem_limit = mem_size; } else if (!strcmp(param, "efi_fake_mem")) { mem_avoid_memmap(PARSE_EFI, val); } @@ -322,7 +326,9 @@ static void handle_mem_options(void) } /* - * In theory, KASLR can put the kernel anywhere in the range of [16M, 64T). + * In theory, KASLR can put the kernel anywhere in the range of [16M, MAXMEM) + * on 64-bit, and [16M, KERNEL_IMAGE_SIZE) on 32-bit. + * * The mem_avoid array is used to store the ranges that need to be avoided * when KASLR searches for an appropriate random address. We must avoid any * regions that are unsafe to overlap with during decompression, and other @@ -620,10 +626,6 @@ static void __process_mem_region(struct mem_vector *entry, unsigned long start_orig, end; struct mem_vector cur_entry; - /* On 32-bit, ignore entries entirely above our maximum. */ - if (IS_ENABLED(CONFIG_X86_32) && entry->start >= KERNEL_IMAGE_SIZE) - return; - /* Ignore entries entirely below our minimum. */ if (entry->start + entry->size < minimum) return; @@ -656,11 +658,6 @@ static void __process_mem_region(struct mem_vector *entry, /* Reduce size by any delta from the original address. */ region.size -= region.start - start_orig; - /* On 32-bit, reduce region size to fit within max size. */ - if (IS_ENABLED(CONFIG_X86_32) && - region.start + region.size > KERNEL_IMAGE_SIZE) - region.size = KERNEL_IMAGE_SIZE - region.start; - /* Return if region can't contain decompressed kernel */ if (region.size < image_size) return; @@ -845,15 +842,16 @@ static void process_e820_entries(unsigned long minimum, static unsigned long find_random_phys_addr(unsigned long minimum, unsigned long image_size) { + /* Bail out early if it's impossible to succeed. */ + if (minimum + image_size > mem_limit) + return 0; + /* Check if we had too many memmaps. */ if (memmap_too_large) { debug_putstr("Aborted memory entries scan (more than 4 memmap= args)!\n"); return 0; } - /* Make sure minimum is aligned. */ - minimum = ALIGN(minimum, CONFIG_PHYSICAL_ALIGN); - if (process_efi_entries(minimum, image_size)) return slots_fetch_random(); @@ -866,8 +864,6 @@ static unsigned long find_random_virt_addr(unsigned long minimum, { unsigned long slots, random_addr; - /* Make sure minimum is aligned. */ - minimum = ALIGN(minimum, CONFIG_PHYSICAL_ALIGN); /* Align image_size for easy slot calculations. */ image_size = ALIGN(image_size, CONFIG_PHYSICAL_ALIGN); @@ -914,6 +910,11 @@ void choose_random_location(unsigned long input, /* Prepare to add new identity pagetables on demand. */ initialize_identity_maps(); + if (IS_ENABLED(CONFIG_X86_32)) + mem_limit = KERNEL_IMAGE_SIZE; + else + mem_limit = MAXMEM; + /* Record the various known unsafe memory ranges. */ mem_avoid_init(input, input_size, *output); @@ -923,6 +924,8 @@ void choose_random_location(unsigned long input, * location: */ min_addr = min(*output, 512UL << 20); + /* Make sure minimum is aligned. */ + min_addr = ALIGN(min_addr, CONFIG_PHYSICAL_ALIGN); /* Walk available memory entries to find a random address. */ random_addr = find_random_phys_addr(min_addr, output_size); -- cgit v1.2.3 From 8d1cf8595860f4807f4ff1f8f1fc53e7576e0d71 Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:06 -0400 Subject: x86/kaslr: Fix off-by-one error in __process_mem_region() In case of an overlap, the beginning of the region should be used even if it is exactly image_size, not just strictly larger. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-6-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index da45e66cb6e4..848346fc0dbb 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -669,7 +669,7 @@ static void __process_mem_region(struct mem_vector *entry, } /* Store beginning of region if holds at least image_size. */ - if (overlap.start > region.start + image_size) { + if (overlap.start >= region.start + image_size) { struct mem_vector beginning; beginning.start = region.start; -- cgit v1.2.3 From 3f9412c73053a5be311607e42560c1303a873be7 Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:07 -0400 Subject: x86/kaslr: Drop redundant cur_entry from __process_mem_region() cur_entry is only used as cur_entry.start + cur_entry.size, which is always equal to end. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-7-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 848346fc0dbb..f2454eef5790 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -624,7 +624,6 @@ static void __process_mem_region(struct mem_vector *entry, { struct mem_vector region, overlap; unsigned long start_orig, end; - struct mem_vector cur_entry; /* Ignore entries entirely below our minimum. */ if (entry->start + entry->size < minimum) @@ -634,11 +633,9 @@ static void __process_mem_region(struct mem_vector *entry, end = min(entry->size + entry->start, mem_limit); if (entry->start >= end) return; - cur_entry.start = entry->start; - cur_entry.size = end - entry->start; - region.start = cur_entry.start; - region.size = cur_entry.size; + region.start = entry->start; + region.size = end - entry->start; /* Give up if slot area array is full. */ while (slot_area_index < MAX_SLOT_AREA) { @@ -652,7 +649,7 @@ static void __process_mem_region(struct mem_vector *entry, region.start = ALIGN(region.start, CONFIG_PHYSICAL_ALIGN); /* Did we raise the address above the passed in memory entry? */ - if (region.start > cur_entry.start + cur_entry.size) + if (region.start > end) return; /* Reduce size by any delta from the original address. */ -- cgit v1.2.3 From ee435ee6490d147c1b9963cc8b331665e4cea634 Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:08 -0400 Subject: x86/kaslr: Eliminate 'start_orig' local variable from __process_mem_region() Set the region.size within the loop, which removes the need for start_orig. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-8-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index f2454eef5790..e978c3508814 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -623,7 +623,7 @@ static void __process_mem_region(struct mem_vector *entry, unsigned long image_size) { struct mem_vector region, overlap; - unsigned long start_orig, end; + unsigned long end; /* Ignore entries entirely below our minimum. */ if (entry->start + entry->size < minimum) @@ -635,12 +635,9 @@ static void __process_mem_region(struct mem_vector *entry, return; region.start = entry->start; - region.size = end - entry->start; /* Give up if slot area array is full. */ while (slot_area_index < MAX_SLOT_AREA) { - start_orig = region.start; - /* Potentially raise address to minimum location. */ if (region.start < minimum) region.start = minimum; @@ -653,7 +650,7 @@ static void __process_mem_region(struct mem_vector *entry, return; /* Reduce size by any delta from the original address. */ - region.size -= region.start - start_orig; + region.size = end - region.start; /* Return if region can't contain decompressed kernel */ if (region.size < image_size) @@ -679,7 +676,6 @@ static void __process_mem_region(struct mem_vector *entry, return; /* Clip off the overlapping region and start over. */ - region.size -= overlap.start - region.start + overlap.size; region.start = overlap.start + overlap.size; } } -- cgit v1.2.3 From ef7b07d59e2f18042622cecde0c7a89b60f33a89 Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:09 -0400 Subject: x86/kaslr: Drop redundant variable in __process_mem_region() region.size can be trimmed to store the portion of the region before the overlap, instead of a separate mem_vector variable. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-9-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index e978c3508814..8cc47faea56d 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -664,11 +664,8 @@ static void __process_mem_region(struct mem_vector *entry, /* Store beginning of region if holds at least image_size. */ if (overlap.start >= region.start + image_size) { - struct mem_vector beginning; - - beginning.start = region.start; - beginning.size = overlap.start - region.start; - process_gb_huge_pages(&beginning, image_size); + region.size = overlap.start - region.start; + process_gb_huge_pages(®ion, image_size); } /* Return if overlap extends to or past end of region. */ -- cgit v1.2.3 From bf457be1548eee6d106daf9604e029b36fed2b11 Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:10 -0400 Subject: x86/kaslr: Drop some redundant checks from __process_mem_region() Clip the start and end of the region to minimum and mem_limit prior to the loop. region.start can only increase during the loop, so raising it to minimum before the loop is enough. A region that becomes empty due to this will get checked in the first iteration of the loop. Drop the check for overlap extending beyond the end of the region. This will get checked in the next loop iteration anyway. Rename end to region_end for symmetry with region.start. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-10-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 27 ++++++--------------------- 1 file changed, 6 insertions(+), 21 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 8cc47faea56d..d074986e8061 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -623,34 +623,23 @@ static void __process_mem_region(struct mem_vector *entry, unsigned long image_size) { struct mem_vector region, overlap; - unsigned long end; + unsigned long region_end; - /* Ignore entries entirely below our minimum. */ - if (entry->start + entry->size < minimum) - return; - - /* Ignore entries above memory limit */ - end = min(entry->size + entry->start, mem_limit); - if (entry->start >= end) - return; - - region.start = entry->start; + /* Enforce minimum and memory limit. */ + region.start = max_t(unsigned long long, entry->start, minimum); + region_end = min(entry->start + entry->size, mem_limit); /* Give up if slot area array is full. */ while (slot_area_index < MAX_SLOT_AREA) { - /* Potentially raise address to minimum location. */ - if (region.start < minimum) - region.start = minimum; - /* Potentially raise address to meet alignment needs. */ region.start = ALIGN(region.start, CONFIG_PHYSICAL_ALIGN); /* Did we raise the address above the passed in memory entry? */ - if (region.start > end) + if (region.start > region_end) return; /* Reduce size by any delta from the original address. */ - region.size = end - region.start; + region.size = region_end - region.start; /* Return if region can't contain decompressed kernel */ if (region.size < image_size) @@ -668,10 +657,6 @@ static void __process_mem_region(struct mem_vector *entry, process_gb_huge_pages(®ion, image_size); } - /* Return if overlap extends to or past end of region. */ - if (overlap.start + overlap.size >= region.start + region.size) - return; - /* Clip off the overlapping region and start over. */ region.start = overlap.start + overlap.size; } -- cgit v1.2.3 From 79c2fd2afe55944098047721c33e06fd48654e57 Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:11 -0400 Subject: x86/kaslr: Fix off-by-one error in process_gb_huge_pages() If the remaining size of the region is exactly 1Gb, there is still one hugepage that can be reserved. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-11-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index d074986e8061..0df513e3e2ce 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -562,7 +562,7 @@ process_gb_huge_pages(struct mem_vector *region, unsigned long image_size) size = region->size - (addr - region->start); /* Check how many 1GB huge pages can be filtered out: */ - while (size > PUD_SIZE && max_gb_huge_pages) { + while (size >= PUD_SIZE && max_gb_huge_pages) { size -= PUD_SIZE; max_gb_huge_pages--; i++; -- cgit v1.2.3 From 50def2693a900dfb1d91872056dc8164245820fc Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:12 -0400 Subject: x86/kaslr: Short-circuit gb_huge_pages on x86-32 32-bit does not have GB pages, so don't bother checking for them. Using the IS_ENABLED() macro allows the compiler to completely remove the gb_huge_pages code. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-12-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 0df513e3e2ce..3727e9708690 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -303,7 +303,7 @@ static void handle_mem_options(void) if (!strcmp(param, "memmap")) { mem_avoid_memmap(PARSE_MEMMAP, val); - } else if (strstr(param, "hugepages")) { + } else if (IS_ENABLED(CONFIG_X86_64) && strstr(param, "hugepages")) { parse_gb_huge_pages(param, val); } else if (!strcmp(param, "mem")) { char *p = val; @@ -551,7 +551,7 @@ process_gb_huge_pages(struct mem_vector *region, unsigned long image_size) struct mem_vector tmp; int i = 0; - if (!max_gb_huge_pages) { + if (!IS_ENABLED(CONFIG_X86_64) || !max_gb_huge_pages) { store_slot_info(region, image_size); return; } -- cgit v1.2.3 From be9e8d9541a95bdfac1c13d112cc032ea7fc745f Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:13 -0400 Subject: x86/kaslr: Simplify process_gb_huge_pages() Replace the loop to determine the number of 1Gb pages with arithmetic. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-13-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 47 ++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 26 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 3727e9708690..00ef84b689f6 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -547,49 +547,44 @@ static void store_slot_info(struct mem_vector *region, unsigned long image_size) static void process_gb_huge_pages(struct mem_vector *region, unsigned long image_size) { - unsigned long addr, size = 0; + unsigned long pud_start, pud_end, gb_huge_pages; struct mem_vector tmp; - int i = 0; if (!IS_ENABLED(CONFIG_X86_64) || !max_gb_huge_pages) { store_slot_info(region, image_size); return; } - addr = ALIGN(region->start, PUD_SIZE); - /* Did we raise the address above the passed in memory entry? */ - if (addr < region->start + region->size) - size = region->size - (addr - region->start); - - /* Check how many 1GB huge pages can be filtered out: */ - while (size >= PUD_SIZE && max_gb_huge_pages) { - size -= PUD_SIZE; - max_gb_huge_pages--; - i++; - } + /* Are there any 1GB pages in the region? */ + pud_start = ALIGN(region->start, PUD_SIZE); + pud_end = ALIGN_DOWN(region->start + region->size, PUD_SIZE); /* No good 1GB huge pages found: */ - if (!i) { + if (pud_start >= pud_end) { store_slot_info(region, image_size); return; } - /* - * Skip those 'i'*1GB good huge pages, and continue checking and - * processing the remaining head or tail part of the passed region - * if available. - */ - - if (addr >= region->start + image_size) { + /* Check if the head part of the region is usable. */ + if (pud_start >= region->start + image_size) { tmp.start = region->start; - tmp.size = addr - region->start; + tmp.size = pud_start - region->start; store_slot_info(&tmp, image_size); } - size = region->size - (addr - region->start) - i * PUD_SIZE; - if (size >= image_size) { - tmp.start = addr + i * PUD_SIZE; - tmp.size = size; + /* Skip the good 1GB pages. */ + gb_huge_pages = (pud_end - pud_start) >> PUD_SHIFT; + if (gb_huge_pages > max_gb_huge_pages) { + pud_end = pud_start + (max_gb_huge_pages << PUD_SHIFT); + max_gb_huge_pages = 0; + } else { + max_gb_huge_pages -= gb_huge_pages; + } + + /* Check if the tail part of the region is usable. */ + if (region->start + region->size >= pud_end + image_size) { + tmp.start = pud_end; + tmp.size = region->start + region->size - pud_end; store_slot_info(&tmp, image_size); } } -- cgit v1.2.3 From 3870d971791f13df88a7a656e3fd6e2df8686097 Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:14 -0400 Subject: x86/kaslr: Drop test for command-line parameters before parsing This check doesn't save anything. In the case when none of the parameters are present, each strstr will scan args twice (once to find the length and then for searching), six scans in total. Just going ahead and parsing the arguments only requires three scans: strlen, memcpy, and parsing. This will be the first malloc, so free will actually free up the memory, so the check doesn't save heap space either. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-14-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 00ef84b689f6..bd13dc5e64b7 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -279,10 +279,6 @@ static void handle_mem_options(void) if (!args) return; - if (!strstr(args, "memmap=") && !strstr(args, "mem=") && - !strstr(args, "hugepages")) - return; - len = strlen(args); tmp_cmdline = malloc(len + 1); if (!tmp_cmdline) -- cgit v1.2.3 From d6d0f36c735367ed7cf42b5ba454ba5858e17816 Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:15 -0400 Subject: x86/kaslr: Make the type of number of slots/slot areas consistent The number of slots can be 'unsigned int', since on 64-bit, the maximum amount of memory is 2^52, the minimum alignment is 2^21, so the slot number cannot be greater than 2^31. But in case future processors have more than 52 physical address bits, make it 'unsigned long'. The slot areas are limited by MAX_SLOT_AREA, currently 100. It is indexed by an int, but the number of areas is stored as 'unsigned long'. Change both to 'unsigned int' for consistency. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-15-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index bd13dc5e64b7..5c7457cc58f6 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -508,17 +508,15 @@ static bool mem_avoid_overlap(struct mem_vector *img, struct slot_area { unsigned long addr; - int num; + unsigned long num; }; #define MAX_SLOT_AREA 100 static struct slot_area slot_areas[MAX_SLOT_AREA]; - +static unsigned int slot_area_index; static unsigned long slot_max; -static unsigned long slot_area_index; - static void store_slot_info(struct mem_vector *region, unsigned long image_size) { struct slot_area slot_area; @@ -588,7 +586,7 @@ process_gb_huge_pages(struct mem_vector *region, unsigned long image_size) static unsigned long slots_fetch_random(void) { unsigned long slot; - int i; + unsigned int i; /* Handle case of no slots stored. */ if (slot_max == 0) -- cgit v1.2.3 From 46a5b29a4a63a3ba987cbb5467774a4b5787618e Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:16 -0400 Subject: x86/kaslr: Drop redundant check in store_slot_info() Drop unnecessary check that number of slots is not zero in store_slot_info, it's guaranteed to be at least 1 by the calculation. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-16-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 5c7457cc58f6..0c64026a0951 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -525,13 +525,10 @@ static void store_slot_info(struct mem_vector *region, unsigned long image_size) return; slot_area.addr = region->start; - slot_area.num = (region->size - image_size) / - CONFIG_PHYSICAL_ALIGN + 1; + slot_area.num = 1 + (region->size - image_size) / CONFIG_PHYSICAL_ALIGN; - if (slot_area.num > 0) { - slot_areas[slot_area_index++] = slot_area; - slot_max += slot_area.num; - } + slot_areas[slot_area_index++] = slot_area; + slot_max += slot_area.num; } /* -- cgit v1.2.3 From eb38be6db516fb72ccf7282628b545a185b3bc7a Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:17 -0400 Subject: x86/kaslr: Drop unnecessary alignment in find_random_virt_addr() Drop unnecessary alignment of image_size to CONFIG_PHYSICAL_ALIGN in find_random_virt_addr, it cannot change the result: the largest valid slot is the largest n that satisfies minimum + n * CONFIG_PHYSICAL_ALIGN + image_size <= KERNEL_IMAGE_SIZE (since minimum is already aligned) and so n is equal to (KERNEL_IMAGE_SIZE - minimum - image_size) / CONFIG_PHYSICAL_ALIGN even if image_size is not aligned to CONFIG_PHYSICAL_ALIGN. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-17-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 0c64026a0951..ce34a05ccdc4 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -825,16 +825,12 @@ static unsigned long find_random_virt_addr(unsigned long minimum, { unsigned long slots, random_addr; - /* Align image_size for easy slot calculations. */ - image_size = ALIGN(image_size, CONFIG_PHYSICAL_ALIGN); - /* * There are how many CONFIG_PHYSICAL_ALIGN-sized slots * that can hold image_size within the range of minimum to * KERNEL_IMAGE_SIZE? */ - slots = (KERNEL_IMAGE_SIZE - minimum - image_size) / - CONFIG_PHYSICAL_ALIGN + 1; + slots = 1 + (KERNEL_IMAGE_SIZE - minimum - image_size) / CONFIG_PHYSICAL_ALIGN; random_addr = kaslr_get_random_long("Virtual") % slots; -- cgit v1.2.3 From 4268b4da572f8bde8bc2f3243927ff5795687a6f Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:18 -0400 Subject: x86/kaslr: Small cleanup of find_random_phys_addr() Just a trivial rearrangement to do all the processing together, and only have one call to slots_fetch_random() in the source. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-18-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index ce34a05ccdc4..ecdf33da2a97 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -813,10 +813,9 @@ static unsigned long find_random_phys_addr(unsigned long minimum, return 0; } - if (process_efi_entries(minimum, image_size)) - return slots_fetch_random(); + if (!process_efi_entries(minimum, image_size)) + process_e820_entries(minimum, image_size); - process_e820_entries(minimum, image_size); return slots_fetch_random(); } -- cgit v1.2.3 From e4cb955bf173474a61f56200610004aacc7a62ff Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:19 -0400 Subject: x86/kaslr: Make minimum/image_size 'unsigned long' Change type of minimum/image_size arguments in process_mem_region to 'unsigned long'. These actually can never be above 4G (even on x86_64), and they're 'unsigned long' in every other function except this one. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-19-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index ecdf33da2a97..3244f5b865e0 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -649,8 +649,8 @@ 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) + unsigned long minimum, + unsigned long image_size) { int i; /* -- cgit v1.2.3 From 3a066990a35eb289d54036637d2793d4743b8f07 Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:20 -0400 Subject: x86/kaslr: Replace 'unsigned long long' with 'u64' No functional change. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-20-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 13 ++++++------- arch/x86/boot/compressed/misc.h | 4 ++-- 2 files changed, 8 insertions(+), 9 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 3244f5b865e0..db8589c2b548 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -98,7 +98,7 @@ static bool memmap_too_large; * Store memory limit: MAXMEM on 64-bit and KERNEL_IMAGE_SIZE on 32-bit. * It may be reduced by "mem=nn[KMG]" or "memmap=nn[KMG]" command line options. */ -static unsigned long long mem_limit; +static u64 mem_limit; /* Number of immovable memory regions */ static int num_immovable_mem; @@ -141,8 +141,7 @@ enum parse_mode { }; static int -parse_memmap(char *p, unsigned long long *start, unsigned long long *size, - enum parse_mode mode) +parse_memmap(char *p, u64 *start, u64 *size, enum parse_mode mode) { char *oldp; @@ -172,7 +171,7 @@ parse_memmap(char *p, unsigned long long *start, unsigned long long *size, */ *size = 0; } else { - unsigned long long flags; + u64 flags; /* * efi_fake_mem=nn@ss:attr the attr specifies @@ -211,7 +210,7 @@ static void mem_avoid_memmap(enum parse_mode mode, char *str) while (str && (i < MAX_MEMMAP_REGIONS)) { int rc; - unsigned long long start, size; + u64 start, size; char *k = strchr(str, ','); if (k) @@ -612,7 +611,7 @@ static void __process_mem_region(struct mem_vector *entry, unsigned long region_end; /* Enforce minimum and memory limit. */ - region.start = max_t(unsigned long long, entry->start, minimum); + region.start = max_t(u64, entry->start, minimum); region_end = min(entry->start + entry->size, mem_limit); /* Give up if slot area array is full. */ @@ -673,7 +672,7 @@ static bool process_mem_region(struct mem_vector *region, * immovable memory and @region. */ for (i = 0; i < num_immovable_mem; i++) { - unsigned long long start, end, entry_end, region_end; + u64 start, end, entry_end, region_end; struct mem_vector entry; if (!mem_overlaps(region, &immovable_mem[i])) diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h index 726e264410ff..3efce27ba35c 100644 --- a/arch/x86/boot/compressed/misc.h +++ b/arch/x86/boot/compressed/misc.h @@ -70,8 +70,8 @@ int cmdline_find_option(const char *option, char *buffer, int bufsize); int cmdline_find_option_bool(const char *option); struct mem_vector { - unsigned long long start; - unsigned long long size; + u64 start; + u64 size; }; #if CONFIG_RANDOMIZE_BASE -- cgit v1.2.3 From 0eb1a8af01d6264cf948d67c8bff15e2eb859355 Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:21 -0400 Subject: x86/kaslr: Make local variables 64-bit Change the type of local variables/fields that store mem_vector addresses to u64 to make it less likely that 32-bit overflow will cause issues on 32-bit. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-21-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index db8589c2b548..80cdd2071305 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -461,7 +461,7 @@ static bool mem_avoid_overlap(struct mem_vector *img, { int i; struct setup_data *ptr; - unsigned long earliest = img->start + img->size; + u64 earliest = img->start + img->size; bool is_overlapping = false; for (i = 0; i < MEM_AVOID_MAX; i++) { @@ -506,7 +506,7 @@ static bool mem_avoid_overlap(struct mem_vector *img, } struct slot_area { - unsigned long addr; + u64 addr; unsigned long num; }; @@ -537,7 +537,8 @@ static void store_slot_info(struct mem_vector *region, unsigned long image_size) static void process_gb_huge_pages(struct mem_vector *region, unsigned long image_size) { - unsigned long pud_start, pud_end, gb_huge_pages; + u64 pud_start, pud_end; + unsigned long gb_huge_pages; struct mem_vector tmp; if (!IS_ENABLED(CONFIG_X86_64) || !max_gb_huge_pages) { @@ -579,7 +580,7 @@ process_gb_huge_pages(struct mem_vector *region, unsigned long image_size) } } -static unsigned long slots_fetch_random(void) +static u64 slots_fetch_random(void) { unsigned long slot; unsigned int i; @@ -595,7 +596,7 @@ static unsigned long slots_fetch_random(void) slot -= slot_areas[i].num; continue; } - return slot_areas[i].addr + slot * CONFIG_PHYSICAL_ALIGN; + return slot_areas[i].addr + ((u64)slot * CONFIG_PHYSICAL_ALIGN); } if (i == slot_area_index) @@ -608,7 +609,7 @@ static void __process_mem_region(struct mem_vector *entry, unsigned long image_size) { struct mem_vector region, overlap; - unsigned long region_end; + u64 region_end; /* Enforce minimum and memory limit. */ region.start = max_t(u64, entry->start, minimum); -- cgit v1.2.3 From f49236ae424d499d02ee3ce35fb9130ddf95b03f Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Tue, 28 Jul 2020 18:57:22 -0400 Subject: x86/kaslr: Add a check that the random address is in range Check in find_random_phys_addr() that the chosen address is inside the range that was required. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200728225722.67457-22-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 80cdd2071305..735fcb2a8b7b 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -803,6 +803,8 @@ static void process_e820_entries(unsigned long minimum, static unsigned long find_random_phys_addr(unsigned long minimum, unsigned long image_size) { + u64 phys_addr; + /* Bail out early if it's impossible to succeed. */ if (minimum + image_size > mem_limit) return 0; @@ -816,7 +818,15 @@ static unsigned long find_random_phys_addr(unsigned long minimum, if (!process_efi_entries(minimum, image_size)) process_e820_entries(minimum, image_size); - return slots_fetch_random(); + phys_addr = slots_fetch_random(); + + /* Perform a final check to make sure the address is in range. */ + if (phys_addr < minimum || phys_addr + image_size > mem_limit) { + warn("Invalid physical address chosen!\n"); + return 0; + } + + return (unsigned long)phys_addr; } static unsigned long find_random_virt_addr(unsigned long minimum, -- cgit v1.2.3 From 76167e5c5457aee8fba3edc5b8554183696fc94d Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Sun, 2 Aug 2020 21:15:34 -0400 Subject: x86/kaslr: Replace strlen() with strnlen() strnlen is safer in case the command line is not NUL-terminated. Signed-off-by: Arvind Sankar Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/20200803011534.730645-2-nivedita@alum.mit.edu --- arch/x86/boot/compressed/kaslr.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'arch') diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 735fcb2a8b7b..6d397435ccad 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -43,6 +43,10 @@ #define STATIC #include +#define _SETUP +#include /* For COMMAND_LINE_SIZE */ +#undef _SETUP + #ifdef CONFIG_X86_5LEVEL unsigned int __pgtable_l5_enabled; unsigned int pgdir_shift __ro_after_init = 39; @@ -278,7 +282,7 @@ static void handle_mem_options(void) if (!args) return; - len = strlen(args); + len = strnlen(args, COMMAND_LINE_SIZE-1); tmp_cmdline = malloc(len + 1); if (!tmp_cmdline) error("Failed to allocate space for tmp_cmdline"); @@ -425,7 +429,7 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size, cmd_line = get_cmd_line_ptr(); /* Calculate size of cmd_line. */ if (cmd_line) { - cmd_line_size = strlen((char *)cmd_line) + 1; + cmd_line_size = strnlen((char *)cmd_line, COMMAND_LINE_SIZE-1) + 1; mem_avoid[MEM_AVOID_CMDLINE].start = cmd_line; mem_avoid[MEM_AVOID_CMDLINE].size = cmd_line_size; add_identity_map(mem_avoid[MEM_AVOID_CMDLINE].start, -- cgit v1.2.3