Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1164266AbdDXClh (ORCPT ); Sun, 23 Apr 2017 22:41:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43972 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1164223AbdDXClQ (ORCPT ); Sun, 23 Apr 2017 22:41:16 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com BE23F8124F Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=bhe@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com BE23F8124F From: Baoquan He To: linux-kernel@vger.kernel.org, keescook@chromium.org, mingo@kernel.org Cc: dave.jiang@intel.com, dan.j.williams@intel.com, hpa@zytor.com, tglx@linutronix.de, dyoung@redhat.com, Baoquan He , Ingo Molnar , x86@kernel.org, Yinghai Lu , Borislav Petkov Subject: [PATCH v2 1/3] KASLR: Parse all memmap entries in cmdline Date: Mon, 24 Apr 2017 10:40:48 +0800 Message-Id: <1493001650-5793-2-git-send-email-bhe@redhat.com> In-Reply-To: <1493001650-5793-1-git-send-email-bhe@redhat.com> References: <1493001650-5793-1-git-send-email-bhe@redhat.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Mon, 24 Apr 2017 02:41:16 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6938 Lines: 267 In commit: f28442497b5c ("x86/boot: Fix KASLR and memmap= collision") ... the memmap= option is parsed so that KASLR can avoid those reserved regions. It uses cmdline_find_option() to get the value if memmap= is specified, however the problem is that cmdline_find_option() can only find the last entry if multiple memmap entries are provided. This is not correct. In this patch, the whole cmdline will be scanned to search each memmap, all of them will be parsed and handled. Signed-off-by: Baoquan He Cc: "H. Peter Anvin" Cc: Thomas Gleixner Cc: Ingo Molnar Cc: x86@kernel.org Cc: Kees Cook Cc: Yinghai Lu Cc: Borislav Petkov --- arch/x86/boot/compressed/cmdline.c | 2 +- arch/x86/boot/compressed/kaslr.c | 127 +++++++++++++++++++++---------------- arch/x86/boot/string.c | 8 +++ 3 files changed, 82 insertions(+), 55 deletions(-) diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c index 73ccf63..9dc1ce6 100644 --- a/arch/x86/boot/compressed/cmdline.c +++ b/arch/x86/boot/compressed/cmdline.c @@ -13,7 +13,7 @@ static inline char rdfs8(addr_t addr) return *((char *)(fs + addr)); } #include "../cmdline.c" -static unsigned long get_cmd_line_ptr(void) +unsigned long get_cmd_line_ptr(void) { unsigned long cmd_line_ptr = boot_params->hdr.cmd_line_ptr; diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 6d9a546..6649ecd 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -9,14 +9,28 @@ * contain the entire properly aligned running kernel image. * */ + +/* + * linux/ctype.h is expected, while conflicts with boot/ctype.h + * which is useless here. Do not include it. + */ +#define BOOT_CTYPE_H + +/* + * Exporting symbol in boot stage is meaningless, and will trigger + * compiling error in some cases. Disable it here. + */ +#define _LINUX_EXPORT_H +#define EXPORT_SYMBOL(sym) + #include "misc.h" #include "error.h" -#include "../boot.h" #include #include #include #include +#include #include /* Simplified build-specific string for starting entropy. */ @@ -61,6 +75,9 @@ struct mem_vector { #define MAX_MEMMAP_REGIONS 4 static bool memmap_too_large; +int mem_avoid_memmap_index; +extern unsigned long get_cmd_line_ptr(void); + enum mem_avoid_index { MEM_AVOID_ZO_RANGE = 0, @@ -85,49 +102,14 @@ static bool mem_overlaps(struct mem_vector *one, struct mem_vector *two) return true; } -/** - * _memparse - Parse a string with mem suffixes into a number - * @ptr: Where parse begins - * @retptr: (output) Optional pointer to next char after parse completes - * - * Parses a string into a number. The number stored at @ptr is - * potentially suffixed with K, M, G, T, P, E. - */ -static unsigned long long _memparse(const char *ptr, char **retptr) +char *skip_spaces(const char *str) { - char *endptr; /* Local pointer to end of parsed string */ - - unsigned long long ret = simple_strtoull(ptr, &endptr, 0); - - switch (*endptr) { - case 'E': - case 'e': - ret <<= 10; - case 'P': - case 'p': - ret <<= 10; - case 'T': - case 't': - ret <<= 10; - case 'G': - case 'g': - ret <<= 10; - case 'M': - case 'm': - ret <<= 10; - case 'K': - case 'k': - ret <<= 10; - endptr++; - default: - break; - } - - if (retptr) - *retptr = endptr; - - return ret; + while (isspace(*str)) + ++str; + return (char *)str; } +#include "../../../../lib/ctype.c" +#include "../../../../lib/cmdline.c" static int parse_memmap(char *p, unsigned long long *start, unsigned long long *size) @@ -142,7 +124,7 @@ parse_memmap(char *p, unsigned long long *start, unsigned long long *size) return -EINVAL; oldp = p; - *size = _memparse(p, &p); + *size = memparse(p, &p); if (p == oldp) return -EINVAL; @@ -155,27 +137,21 @@ parse_memmap(char *p, unsigned long long *start, unsigned long long *size) case '#': case '$': case '!': - *start = _memparse(p + 1, &p); + *start = memparse(p + 1, &p); return 0; } return -EINVAL; } -static void mem_avoid_memmap(void) +static void mem_avoid_memmap(char *str) { - char arg[128]; int rc; - int i; - char *str; + int i = mem_avoid_memmap_index; - /* See if we have any memmap areas */ - rc = cmdline_find_option("memmap", arg, sizeof(arg)); - if (rc <= 0) + if (i >= MAX_MEMMAP_REGIONS) return; - i = 0; - str = arg; while (str && (i < MAX_MEMMAP_REGIONS)) { int rc; unsigned long long start, size; @@ -196,12 +172,55 @@ static void mem_avoid_memmap(void) mem_avoid[MEM_AVOID_MEMMAP_BEGIN + i].size = size; i++; } + mem_avoid_memmap_index = i; /* More than 4 memmaps, fail kaslr */ if ((i >= MAX_MEMMAP_REGIONS) && str) memmap_too_large = true; } + +/* Macros used by the included decompressor code below. */ +#define STATIC +#include + +#define COMMAND_LINE_SIZE 256 +static int handle_mem_memmap(void) +{ + char *args = (char *)get_cmd_line_ptr(); + size_t len = strlen((char *)args); + char *tmp_cmdline; + char *param, *val; + + tmp_cmdline = malloc(COMMAND_LINE_SIZE); + if (!tmp_cmdline ) + error("Failed to allocate space for tmp_cmdline"); + + len = (len >= COMMAND_LINE_SIZE) ? COMMAND_LINE_SIZE - 1 : len; + memcpy(tmp_cmdline, args, len); + tmp_cmdline[len] = 0; + args = tmp_cmdline; + + /* Chew leading spaces */ + args = skip_spaces(args); + + while (*args) { + args = next_arg(args, ¶m, &val); + /* Stop at -- */ + if (!val && strcmp(param, "--") == 0) { + warn("Only '--' specified in cmdline"); + free(tmp_cmdline); + return -1; + } + + if (!strcmp(param, "memmap")) + mem_avoid_memmap(val); + } + + free(tmp_cmdline); + return 0; +} + /* * 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 @@ -323,7 +342,7 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size, /* We don't need to set a mapping for setup_data. */ /* Mark the memmap regions we need to avoid */ - mem_avoid_memmap(); + handle_mem_memmap(); #ifdef CONFIG_X86_VERBOSE_BOOTUP /* Make sure video RAM can be used. */ diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c index 5457b02..630e366 100644 --- a/arch/x86/boot/string.c +++ b/arch/x86/boot/string.c @@ -122,6 +122,14 @@ unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int bas return result; } +long simple_strtol(const char *cp, char **endp, unsigned int base) +{ + if (*cp == '-') + return -simple_strtoull(cp + 1, endp, base); + + return simple_strtoull(cp, endp, base); +} + /** * strlen - Find the length of a string * @s: The string to be sized -- 2.5.5