* [PATCH v4 0/2] x86/boot/KASLR: skip the specified crashkernel region @ 2019-04-08 5:58 Pingfan Liu 2019-04-08 5:58 ` [PATCH v4 1/2] kernel/crash_core: separate the parsing routines to lib/parse_crashkernel.c Pingfan Liu 2019-04-08 5:58 ` [PATCH v4 2/2] x86/boot/KASLR: skip the specified crashkernel region Pingfan Liu 0 siblings, 2 replies; 11+ messages in thread From: Pingfan Liu @ 2019-04-08 5:58 UTC (permalink / raw) To: x86 Cc: Pingfan Liu, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, Baoquan He, Will Deacon, Nicolas Pitre, Vivek Goyal, Chao Fan, Kirill A. Shutemov, Ard Biesheuvel, Hari Bathini, linux-kernel crashkernel=x@y or or =range1:size1[,range2:size2,...]@offset option may fail to reserve the required memory region if KASLR puts kernel into the region. To avoid this uncertainty, asking KASLR to skip the required region. And the parsing routine can be re-used at this early boot stage. Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: Borislav Petkov <bp@alien8.de> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Baoquan He <bhe@redhat.com> Cc: Will Deacon <will.deacon@arm.com> Cc: Nicolas Pitre <nico@linaro.org> Cc: Vivek Goyal <vgoyal@redhat.com> Cc: Chao Fan <fanc.fnst@cn.fujitsu.com> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> CC: Hari Bathini <hbathini@linux.vnet.ibm.com> Cc: linux-kernel@vger.kernel.org --- v3 -> v4: reuse the parse_crashkernel_xx routines Pingfan Liu (2): kernel/crash_core: separate the parsing routines to lib/parse_crashkernel.c x86/boot/KASLR: skip the specified crashkernel region arch/x86/boot/compressed/kaslr.c | 40 ++++++ kernel/crash_core.c | 273 ------------------------------------ lib/Makefile | 2 + lib/parse_crashkernel.c | 289 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 331 insertions(+), 273 deletions(-) create mode 100644 lib/parse_crashkernel.c -- 2.7.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 1/2] kernel/crash_core: separate the parsing routines to lib/parse_crashkernel.c 2019-04-08 5:58 [PATCH v4 0/2] x86/boot/KASLR: skip the specified crashkernel region Pingfan Liu @ 2019-04-08 5:58 ` Pingfan Liu 2019-04-16 19:01 ` Borislav Petkov 2019-04-08 5:58 ` [PATCH v4 2/2] x86/boot/KASLR: skip the specified crashkernel region Pingfan Liu 1 sibling, 1 reply; 11+ messages in thread From: Pingfan Liu @ 2019-04-08 5:58 UTC (permalink / raw) To: x86 Cc: Pingfan Liu, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, Baoquan He, Will Deacon, Nicolas Pitre, Chao Fan, Kirill A. Shutemov, Ard Biesheuvel, Vivek Goyal, Hari Bathini, linux-kernel Beside kernel, at early boot stage, the KASLR code also needs to parse the crashkernel=x@y or crashkernel=ramsize-range:size[,...][@offset] option, and avoid to put randomized kernel in the region. Extracting the parsing related routines to lib/parse_crashkernel.c, so it will be handy included by other files. Signed-off-by: Pingfan Liu <kernelfans@gmail.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: Borislav Petkov <bp@alien8.de> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Baoquan He <bhe@redhat.com> Cc: Will Deacon <will.deacon@arm.com> Cc: Nicolas Pitre <nico@linaro.org> Cc: Chao Fan <fanc.fnst@cn.fujitsu.com> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Vivek Goyal <vgoyal@redhat.com> CC: Hari Bathini <hbathini@linux.vnet.ibm.com> Cc: linux-kernel@vger.kernel.org --- kernel/crash_core.c | 273 --------------------------------------------- lib/Makefile | 2 + lib/parse_crashkernel.c | 289 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 291 insertions(+), 273 deletions(-) create mode 100644 lib/parse_crashkernel.c diff --git a/kernel/crash_core.c b/kernel/crash_core.c index 093c9f9..37c4d6f 100644 --- a/kernel/crash_core.c +++ b/kernel/crash_core.c @@ -21,279 +21,6 @@ u32 *vmcoreinfo_note; /* trusted vmcoreinfo, e.g. we can make a copy in the crash memory */ static unsigned char *vmcoreinfo_data_safecopy; -/* - * parsing the "crashkernel" commandline - * - * this code is intended to be called from architecture specific code - */ - - -/* - * This function parses command lines in the format - * - * crashkernel=ramsize-range:size[,...][@offset] - * - * The function returns 0 on success and -EINVAL on failure. - */ -static int __init parse_crashkernel_mem(char *cmdline, - unsigned long long system_ram, - unsigned long long *crash_size, - unsigned long long *crash_base) -{ - char *cur = cmdline, *tmp; - - /* for each entry of the comma-separated list */ - do { - unsigned long long start, end = ULLONG_MAX, size; - - /* get the start of the range */ - start = memparse(cur, &tmp); - if (cur == tmp) { - pr_warn("crashkernel: Memory value expected\n"); - return -EINVAL; - } - cur = tmp; - if (*cur != '-') { - pr_warn("crashkernel: '-' expected\n"); - return -EINVAL; - } - cur++; - - /* if no ':' is here, than we read the end */ - if (*cur != ':') { - end = memparse(cur, &tmp); - if (cur == tmp) { - pr_warn("crashkernel: Memory value expected\n"); - return -EINVAL; - } - cur = tmp; - if (end <= start) { - pr_warn("crashkernel: end <= start\n"); - return -EINVAL; - } - } - - if (*cur != ':') { - pr_warn("crashkernel: ':' expected\n"); - return -EINVAL; - } - cur++; - - size = memparse(cur, &tmp); - if (cur == tmp) { - pr_warn("Memory value expected\n"); - return -EINVAL; - } - cur = tmp; - if (size >= system_ram) { - pr_warn("crashkernel: invalid size\n"); - return -EINVAL; - } - - /* match ? */ - if (system_ram >= start && system_ram < end) { - *crash_size = size; - break; - } - } while (*cur++ == ','); - - if (*crash_size > 0) { - while (*cur && *cur != ' ' && *cur != '@') - cur++; - if (*cur == '@') { - cur++; - *crash_base = memparse(cur, &tmp); - if (cur == tmp) { - pr_warn("Memory value expected after '@'\n"); - return -EINVAL; - } - } - } else - pr_info("crashkernel size resulted in zero bytes\n"); - - return 0; -} - -/* - * That function parses "simple" (old) crashkernel command lines like - * - * crashkernel=size[@offset] - * - * It returns 0 on success and -EINVAL on failure. - */ -static int __init parse_crashkernel_simple(char *cmdline, - unsigned long long *crash_size, - unsigned long long *crash_base) -{ - char *cur = cmdline; - - *crash_size = memparse(cmdline, &cur); - if (cmdline == cur) { - pr_warn("crashkernel: memory value expected\n"); - return -EINVAL; - } - - if (*cur == '@') - *crash_base = memparse(cur+1, &cur); - else if (*cur != ' ' && *cur != '\0') { - pr_warn("crashkernel: unrecognized char: %c\n", *cur); - return -EINVAL; - } - - return 0; -} - -#define SUFFIX_HIGH 0 -#define SUFFIX_LOW 1 -#define SUFFIX_NULL 2 -static __initdata char *suffix_tbl[] = { - [SUFFIX_HIGH] = ",high", - [SUFFIX_LOW] = ",low", - [SUFFIX_NULL] = NULL, -}; - -/* - * That function parses "suffix" crashkernel command lines like - * - * crashkernel=size,[high|low] - * - * It returns 0 on success and -EINVAL on failure. - */ -static int __init parse_crashkernel_suffix(char *cmdline, - unsigned long long *crash_size, - const char *suffix) -{ - char *cur = cmdline; - - *crash_size = memparse(cmdline, &cur); - if (cmdline == cur) { - pr_warn("crashkernel: memory value expected\n"); - return -EINVAL; - } - - /* check with suffix */ - if (strncmp(cur, suffix, strlen(suffix))) { - pr_warn("crashkernel: unrecognized char: %c\n", *cur); - return -EINVAL; - } - cur += strlen(suffix); - if (*cur != ' ' && *cur != '\0') { - pr_warn("crashkernel: unrecognized char: %c\n", *cur); - return -EINVAL; - } - - return 0; -} - -static __init char *get_last_crashkernel(char *cmdline, - const char *name, - const char *suffix) -{ - char *p = cmdline, *ck_cmdline = NULL; - - /* find crashkernel and use the last one if there are more */ - p = strstr(p, name); - while (p) { - char *end_p = strchr(p, ' '); - char *q; - - if (!end_p) - end_p = p + strlen(p); - - if (!suffix) { - int i; - - /* skip the one with any known suffix */ - for (i = 0; suffix_tbl[i]; i++) { - q = end_p - strlen(suffix_tbl[i]); - if (!strncmp(q, suffix_tbl[i], - strlen(suffix_tbl[i]))) - goto next; - } - ck_cmdline = p; - } else { - q = end_p - strlen(suffix); - if (!strncmp(q, suffix, strlen(suffix))) - ck_cmdline = p; - } -next: - p = strstr(p+1, name); - } - - if (!ck_cmdline) - return NULL; - - return ck_cmdline; -} - -static int __init __parse_crashkernel(char *cmdline, - unsigned long long system_ram, - unsigned long long *crash_size, - unsigned long long *crash_base, - const char *name, - const char *suffix) -{ - char *first_colon, *first_space; - char *ck_cmdline; - - BUG_ON(!crash_size || !crash_base); - *crash_size = 0; - *crash_base = 0; - - ck_cmdline = get_last_crashkernel(cmdline, name, suffix); - - if (!ck_cmdline) - return -EINVAL; - - ck_cmdline += strlen(name); - - if (suffix) - return parse_crashkernel_suffix(ck_cmdline, crash_size, - suffix); - /* - * if the commandline contains a ':', then that's the extended - * syntax -- if not, it must be the classic syntax - */ - first_colon = strchr(ck_cmdline, ':'); - first_space = strchr(ck_cmdline, ' '); - if (first_colon && (!first_space || first_colon < first_space)) - return parse_crashkernel_mem(ck_cmdline, system_ram, - crash_size, crash_base); - - return parse_crashkernel_simple(ck_cmdline, crash_size, crash_base); -} - -/* - * That function is the entry point for command line parsing and should be - * called from the arch-specific code. - */ -int __init parse_crashkernel(char *cmdline, - unsigned long long system_ram, - unsigned long long *crash_size, - unsigned long long *crash_base) -{ - return __parse_crashkernel(cmdline, system_ram, crash_size, crash_base, - "crashkernel=", NULL); -} - -int __init parse_crashkernel_high(char *cmdline, - unsigned long long system_ram, - unsigned long long *crash_size, - unsigned long long *crash_base) -{ - return __parse_crashkernel(cmdline, system_ram, crash_size, crash_base, - "crashkernel=", suffix_tbl[SUFFIX_HIGH]); -} - -int __init parse_crashkernel_low(char *cmdline, - unsigned long long system_ram, - unsigned long long *crash_size, - unsigned long long *crash_base) -{ - return __parse_crashkernel(cmdline, system_ram, crash_size, crash_base, - "crashkernel=", suffix_tbl[SUFFIX_LOW]); -} - Elf_Word *append_elf_note(Elf_Word *buf, char *name, unsigned int type, void *data, size_t data_len) { diff --git a/lib/Makefile b/lib/Makefile index 3b08673..34ccb39 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -164,6 +164,8 @@ obj-$(CONFIG_OF_RECONFIG_NOTIFIER_ERROR_INJECT) += \ of-reconfig-notifier-error-inject.o obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o +obj-$(CONFIG_CRASH_CORE) += parse_crashkernel.o + lib-$(CONFIG_GENERIC_BUG) += bug.o obj-$(CONFIG_HAVE_ARCH_TRACEHOOK) += syscall.o diff --git a/lib/parse_crashkernel.c b/lib/parse_crashkernel.c new file mode 100644 index 0000000..f953930 --- /dev/null +++ b/lib/parse_crashkernel.c @@ -0,0 +1,289 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * parsing the "crashkernel" commandline + * + * this code is intended to be called from architecture specific code + */ +#include <linux/crash_core.h> + +/* + * This function parses command lines in the format + * + * crashkernel=ramsize-range:size[,...][@offset] + * + * The function returns 0 on success and -EINVAL on failure. + */ +static int __init parse_crashkernel_mem(char *cmdline, + unsigned long long system_ram, + unsigned long long *crash_size, + unsigned long long *crash_base) +{ + char *cur = cmdline, *tmp; + + /* for each entry of the comma-separated list */ + do { + unsigned long long start, end = ULLONG_MAX, size; + + /* get the start of the range */ + start = memparse(cur, &tmp); + if (cur == tmp) { + pr_warn("crashkernel: Memory value expected\n"); + return -EINVAL; + } + cur = tmp; + if (*cur != '-') { + pr_warn("crashkernel: '-' expected\n"); + return -EINVAL; + } + cur++; + + /* if no ':' is here, than we read the end */ + if (*cur != ':') { + end = memparse(cur, &tmp); + if (cur == tmp) { + pr_warn("crashkernel: Memory value expected\n"); + return -EINVAL; + } + cur = tmp; + if (end <= start) { + pr_warn("crashkernel: end <= start\n"); + return -EINVAL; + } + } + + if (*cur != ':') { + pr_warn("crashkernel: ':' expected\n"); + return -EINVAL; + } + cur++; + + size = memparse(cur, &tmp); + if (cur == tmp) { + pr_warn("Memory value expected\n"); + return -EINVAL; + } + cur = tmp; + if (size >= system_ram) { + pr_warn("crashkernel: invalid size\n"); + return -EINVAL; + } + + /* match ? */ + if (system_ram >= start && system_ram < end) { + *crash_size = size; + break; + } + } while (*cur++ == ','); + + if (*crash_size > 0) { + while (*cur && *cur != ' ' && *cur != '@') + cur++; + if (*cur == '@') { + cur++; + *crash_base = memparse(cur, &tmp); + if (cur == tmp) { + pr_warn("Memory value expected after '@'\n"); + return -EINVAL; + } + } + } else + pr_info("crashkernel size resulted in zero bytes\n"); + + return 0; +} + +/* + * That function parses "simple" (old) crashkernel command lines like + * + * crashkernel=size[@offset] + * + * It returns 0 on success and -EINVAL on failure. + */ +static int __init parse_crashkernel_simple(char *cmdline, + unsigned long long *crash_size, + unsigned long long *crash_base) +{ + char *cur = cmdline; + + *crash_size = memparse(cmdline, &cur); + if (cmdline == cur) { + pr_warn("crashkernel: memory value expected\n"); + return -EINVAL; + } + + if (*cur == '@') + *crash_base = memparse(cur+1, &cur); + else if (*cur != ' ' && *cur != '\0') { + pr_warn("crashkernel: unrecognized char: %c\n", *cur); + return -EINVAL; + } + + return 0; +} + + + +#define SUFFIX_HIGH 0 +#define SUFFIX_LOW 1 +#define SUFFIX_NULL 2 +static __initdata char *suffix_tbl[] = { + [SUFFIX_HIGH] = ",high", + [SUFFIX_LOW] = ",low", + [SUFFIX_NULL] = NULL, +}; + +/* At boot stage, KASLR does not care about crashkernel=size,[high|low], which + * never specifies the offset of region. + */ +#ifndef _BOOT_KASLR +/* + * That function parses "suffix" crashkernel command lines like + * + * crashkernel=size,[high|low] + * + * It returns 0 on success and -EINVAL on failure. + */ +static int __init parse_crashkernel_suffix(char *cmdline, + unsigned long long *crash_size, + const char *suffix) +{ + char *cur = cmdline; + + *crash_size = memparse(cmdline, &cur); + if (cmdline == cur) { + pr_warn("crashkernel: memory value expected\n"); + return -EINVAL; + } + + /* check with suffix */ + if (strncmp(cur, suffix, strlen(suffix))) { + pr_warn("crashkernel: unrecognized char: %c\n", *cur); + return -EINVAL; + } + cur += strlen(suffix); + if (*cur != ' ' && *cur != '\0') { + pr_warn("crashkernel: unrecognized char: %c\n", *cur); + return -EINVAL; + } + + return 0; +} + +static int __init __parse_crashkernel(char *cmdline, + unsigned long long system_ram, + unsigned long long *crash_size, + unsigned long long *crash_base, + const char *name, + const char *suffix); + +int __init parse_crashkernel_high(char *cmdline, + unsigned long long system_ram, + unsigned long long *crash_size, + unsigned long long *crash_base) +{ + return __parse_crashkernel(cmdline, system_ram, crash_size, crash_base, + "crashkernel=", suffix_tbl[SUFFIX_HIGH]); +} + +int __init parse_crashkernel_low(char *cmdline, + unsigned long long system_ram, + unsigned long long *crash_size, + unsigned long long *crash_base) +{ + return __parse_crashkernel(cmdline, system_ram, crash_size, crash_base, + "crashkernel=", suffix_tbl[SUFFIX_LOW]); +} +#endif + +static __init char *get_last_crashkernel(char *cmdline, + const char *name, + const char *suffix) +{ + char *p = cmdline, *ck_cmdline = NULL; + + /* find crashkernel and use the last one if there are more */ + p = strstr(p, name); + while (p) { + char *end_p = strchr(p, ' '); + char *q; + + if (!end_p) + end_p = p + strlen(p); + + if (!suffix) { + int i; + + /* skip the one with any known suffix */ + for (i = 0; suffix_tbl[i]; i++) { + q = end_p - strlen(suffix_tbl[i]); + if (!strncmp(q, suffix_tbl[i], + strlen(suffix_tbl[i]))) + goto next; + } + ck_cmdline = p; + } else { + q = end_p - strlen(suffix); + if (!strncmp(q, suffix, strlen(suffix))) + ck_cmdline = p; + } +next: + p = strstr(p+1, name); + } + + if (!ck_cmdline) + return NULL; + + return ck_cmdline; +} + +static int __init __parse_crashkernel(char *cmdline, + unsigned long long system_ram, + unsigned long long *crash_size, + unsigned long long *crash_base, + const char *name, + const char *suffix) +{ + char *first_colon, *first_space; + char *ck_cmdline; + + BUG_ON(!crash_size || !crash_base); + *crash_size = 0; + *crash_base = 0; + + ck_cmdline = get_last_crashkernel(cmdline, name, suffix); + + if (!ck_cmdline) + return -EINVAL; + + ck_cmdline += strlen(name); + +#ifndef _BOOT_KASLR + if (suffix) + return parse_crashkernel_suffix(ck_cmdline, crash_size, + suffix); +#endif + /* + * if the commandline contains a ':', then that's the extended + * syntax -- if not, it must be the classic syntax + */ + first_colon = strchr(ck_cmdline, ':'); + first_space = strchr(ck_cmdline, ' '); + if (first_colon && (!first_space || first_colon < first_space)) + return parse_crashkernel_mem(ck_cmdline, system_ram, + crash_size, crash_base); + + return parse_crashkernel_simple(ck_cmdline, crash_size, crash_base); +} + +/* + * That function is the entry point for command line parsing and should be + * called from the arch-specific code. + */ +int __init parse_crashkernel(char *cmdline, + unsigned long long system_ram, + unsigned long long *crash_size, + unsigned long long *crash_base) +{ + return __parse_crashkernel(cmdline, system_ram, crash_size, crash_base, + "crashkernel=", NULL); +} -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/2] kernel/crash_core: separate the parsing routines to lib/parse_crashkernel.c 2019-04-08 5:58 ` [PATCH v4 1/2] kernel/crash_core: separate the parsing routines to lib/parse_crashkernel.c Pingfan Liu @ 2019-04-16 19:01 ` Borislav Petkov 2019-04-17 5:48 ` Pingfan Liu 0 siblings, 1 reply; 11+ messages in thread From: Borislav Petkov @ 2019-04-16 19:01 UTC (permalink / raw) To: Pingfan Liu Cc: x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Baoquan He, Will Deacon, Nicolas Pitre, Chao Fan, Kirill A. Shutemov, Ard Biesheuvel, Vivek Goyal, Hari Bathini, linux-kernel On Mon, Apr 08, 2019 at 01:58:34PM +0800, Pingfan Liu wrote: > Beside kernel, at early boot stage, the KASLR code also needs to parse the > crashkernel=x@y or crashkernel=ramsize-range:size[,...][@offset] option, > and avoid to put randomized kernel in the region. > > Extracting the parsing related routines to lib/parse_crashkernel.c, so it > will be handy included by other > files. Use this commit message for your next submission: crash: Carve out crashkernel= cmdline parsing Make the "crashkernel=" parsing functionality available to the early KASLR code. Will be used by a later patch to parse crashkernel regions which KASLR should aviod. > Signed-off-by: Pingfan Liu <kernelfans@gmail.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Baoquan He <bhe@redhat.com> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Nicolas Pitre <nico@linaro.org> > Cc: Chao Fan <fanc.fnst@cn.fujitsu.com> > Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Vivek Goyal <vgoyal@redhat.com> > CC: Hari Bathini <hbathini@linux.vnet.ibm.com> > Cc: linux-kernel@vger.kernel.org > --- > kernel/crash_core.c | 273 --------------------------------------------- > lib/Makefile | 2 + > lib/parse_crashkernel.c | 289 ++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 291 insertions(+), 273 deletions(-) And this is not how you carve out code. First, you do a patch which does only code move. Nothing more. In a follow on patch, you make the changes to the moved code so that it is immediately visible what you're changing. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/2] kernel/crash_core: separate the parsing routines to lib/parse_crashkernel.c 2019-04-16 19:01 ` Borislav Petkov @ 2019-04-17 5:48 ` Pingfan Liu 0 siblings, 0 replies; 11+ messages in thread From: Pingfan Liu @ 2019-04-17 5:48 UTC (permalink / raw) To: Borislav Petkov Cc: x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Baoquan He, Will Deacon, Nicolas Pitre, Chao Fan, Kirill A. Shutemov, Ard Biesheuvel, Vivek Goyal, Hari Bathini, LKML On Wed, Apr 17, 2019 at 3:01 AM Borislav Petkov <bp@alien8.de> wrote: > > On Mon, Apr 08, 2019 at 01:58:34PM +0800, Pingfan Liu wrote: > > Beside kernel, at early boot stage, the KASLR code also needs to parse the > > crashkernel=x@y or crashkernel=ramsize-range:size[,...][@offset] option, > > and avoid to put randomized kernel in the region. > > > > Extracting the parsing related routines to lib/parse_crashkernel.c, so it > > will be handy included by other > > files. > > Use this commit message for your next submission: > > crash: Carve out crashkernel= cmdline parsing > > Make the "crashkernel=" parsing functionality available to the early > KASLR code. Will be used by a later patch to parse crashkernel regions > which KASLR should aviod. > OK. > > Signed-off-by: Pingfan Liu <kernelfans@gmail.com> > > Cc: Thomas Gleixner <tglx@linutronix.de> > > Cc: Ingo Molnar <mingo@redhat.com> > > Cc: Borislav Petkov <bp@alien8.de> > > Cc: "H. Peter Anvin" <hpa@zytor.com> > > Cc: Baoquan He <bhe@redhat.com> > > Cc: Will Deacon <will.deacon@arm.com> > > Cc: Nicolas Pitre <nico@linaro.org> > > Cc: Chao Fan <fanc.fnst@cn.fujitsu.com> > > Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Cc: Vivek Goyal <vgoyal@redhat.com> > > CC: Hari Bathini <hbathini@linux.vnet.ibm.com> > > Cc: linux-kernel@vger.kernel.org > > --- > > kernel/crash_core.c | 273 --------------------------------------------- > > lib/Makefile | 2 + > > lib/parse_crashkernel.c | 289 ++++++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 291 insertions(+), 273 deletions(-) > > And this is not how you carve out code. > > First, you do a patch which does only code move. Nothing more. > > In a follow on patch, you make the changes to the moved code so that it > is immediately visible what you're changing. > Will fix it. Thanks for your review. Regards, Pingfan ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 2/2] x86/boot/KASLR: skip the specified crashkernel region 2019-04-08 5:58 [PATCH v4 0/2] x86/boot/KASLR: skip the specified crashkernel region Pingfan Liu 2019-04-08 5:58 ` [PATCH v4 1/2] kernel/crash_core: separate the parsing routines to lib/parse_crashkernel.c Pingfan Liu @ 2019-04-08 5:58 ` Pingfan Liu 2019-04-16 19:01 ` Borislav Petkov 1 sibling, 1 reply; 11+ messages in thread From: Pingfan Liu @ 2019-04-08 5:58 UTC (permalink / raw) To: x86 Cc: Pingfan Liu, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, Baoquan He, Will Deacon, Nicolas Pitre, Vivek Goyal, Chao Fan, Kirill A. Shutemov, Ard Biesheuvel, Hari Bathini, linux-kernel crashkernel=x@y or or =range1:size1[,range2:size2,...]@offset option may fail to reserve the required memory region if KASLR puts kernel into the region. To avoid this uncertainty, asking KASLR to skip the required region. Signed-off-by: Pingfan Liu <kernelfans@gmail.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: Borislav Petkov <bp@alien8.de> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Baoquan He <bhe@redhat.com> Cc: Will Deacon <will.deacon@arm.com> Cc: Nicolas Pitre <nico@linaro.org> Cc: Vivek Goyal <vgoyal@redhat.com> Cc: Chao Fan <fanc.fnst@cn.fujitsu.com> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> CC: Hari Bathini <hbathini@linux.vnet.ibm.com> Cc: linux-kernel@vger.kernel.org --- arch/x86/boot/compressed/kaslr.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 2e53c05..765a593 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -107,6 +107,7 @@ enum mem_avoid_index { MEM_AVOID_BOOTPARAMS, MEM_AVOID_MEMMAP_BEGIN, MEM_AVOID_MEMMAP_END = MEM_AVOID_MEMMAP_BEGIN + MAX_MEMMAP_REGIONS - 1, + MEM_AVOID_CRASHKERNEL, MEM_AVOID_MAX, }; @@ -131,6 +132,11 @@ char *skip_spaces(const char *str) } #include "../../../../lib/ctype.c" #include "../../../../lib/cmdline.c" +#ifdef CONFIG_CRASH_CORE +#define printk +#define _BOOT_KASLR +#include "../../../../lib/parse_crashkernel.c" +#endif static int parse_memmap(char *p, unsigned long long *start, unsigned long long *size) @@ -292,6 +298,39 @@ static void handle_mem_options(void) return; } +static u64 mem_ram_size(void) +{ + struct boot_e820_entry *entry; + u64 total_sz = 0; + int i; + + for (i = 0; i < boot_params->e820_entries; i++) { + entry = &boot_params->e820_table[i]; + /* Skip non-RAM entries. */ + if (entry->type != E820_TYPE_RAM) + continue; + total_sz += entry->size; + } + return total_sz; +} + +/* + * For crashkernel=size@offset or =range1:size1[,range2:size2,...]@offset + * options, recording mem_avoid for them. + */ +static void handle_crashkernel_options(void) +{ + unsigned long long crash_size, crash_base = 0; + char *cmdline = (char *)get_cmd_line_ptr(); + u64 total_sz = mem_ram_size(); + + parse_crashkernel(cmdline, total_sz, &crash_size, &crash_base); + if (crash_base) { + mem_avoid[MEM_AVOID_CRASHKERNEL].start = crash_base; + mem_avoid[MEM_AVOID_CRASHKERNEL].size = crash_size; + } +} + /* * 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 @@ -414,6 +453,7 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size, /* Mark the memmap regions we need to avoid */ handle_mem_options(); + handle_crashkernel_options(); /* Enumerate the immovable memory regions */ num_immovable_mem = count_immovable_mem_regions(); -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/2] x86/boot/KASLR: skip the specified crashkernel region 2019-04-08 5:58 ` [PATCH v4 2/2] x86/boot/KASLR: skip the specified crashkernel region Pingfan Liu @ 2019-04-16 19:01 ` Borislav Petkov 2019-04-17 5:53 ` Pingfan Liu 0 siblings, 1 reply; 11+ messages in thread From: Borislav Petkov @ 2019-04-16 19:01 UTC (permalink / raw) To: Pingfan Liu Cc: x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Baoquan He, Will Deacon, Nicolas Pitre, Vivek Goyal, Chao Fan, Kirill A. Shutemov, Ard Biesheuvel, Hari Bathini, linux-kernel On Mon, Apr 08, 2019 at 01:58:35PM +0800, Pingfan Liu wrote: > crashkernel=x@y or or =range1:size1[,range2:size2,...]@offset option may > fail to reserve the required memory region if KASLR puts kernel into the > region. To avoid this uncertainty, asking KASLR to skip the required > region. > > Signed-off-by: Pingfan Liu <kernelfans@gmail.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Baoquan He <bhe@redhat.com> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Nicolas Pitre <nico@linaro.org> > Cc: Vivek Goyal <vgoyal@redhat.com> > Cc: Chao Fan <fanc.fnst@cn.fujitsu.com> > Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > CC: Hari Bathini <hbathini@linux.vnet.ibm.com> > Cc: linux-kernel@vger.kernel.org > --- > arch/x86/boot/compressed/kaslr.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c > index 2e53c05..765a593 100644 > --- a/arch/x86/boot/compressed/kaslr.c > +++ b/arch/x86/boot/compressed/kaslr.c > @@ -107,6 +107,7 @@ enum mem_avoid_index { > MEM_AVOID_BOOTPARAMS, > MEM_AVOID_MEMMAP_BEGIN, > MEM_AVOID_MEMMAP_END = MEM_AVOID_MEMMAP_BEGIN + MAX_MEMMAP_REGIONS - 1, > + MEM_AVOID_CRASHKERNEL, > MEM_AVOID_MAX, > }; > > @@ -131,6 +132,11 @@ char *skip_spaces(const char *str) > } > #include "../../../../lib/ctype.c" > #include "../../../../lib/cmdline.c" > +#ifdef CONFIG_CRASH_CORE > +#define printk > +#define _BOOT_KASLR > +#include "../../../../lib/parse_crashkernel.c" > +#endif > > static int > parse_memmap(char *p, unsigned long long *start, unsigned long long *size) > @@ -292,6 +298,39 @@ static void handle_mem_options(void) > return; > } > > +static u64 mem_ram_size(void) > +{ > + struct boot_e820_entry *entry; > + u64 total_sz = 0; > + int i; > + > + for (i = 0; i < boot_params->e820_entries; i++) { > + entry = &boot_params->e820_table[i]; > + /* Skip non-RAM entries. */ > + if (entry->type != E820_TYPE_RAM) > + continue; > + total_sz += entry->size; > + } > + return total_sz; > +} > + > +/* > + * For crashkernel=size@offset or =range1:size1[,range2:size2,...]@offset > + * options, recording mem_avoid for them. > + */ > +static void handle_crashkernel_options(void) > +{ > + unsigned long long crash_size, crash_base = 0; > + char *cmdline = (char *)get_cmd_line_ptr(); > + u64 total_sz = mem_ram_size(); > + > + parse_crashkernel(cmdline, total_sz, &crash_size, &crash_base); That function has a return value which you could test. And then you don't need to set crash_base to 0 above. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/2] x86/boot/KASLR: skip the specified crashkernel region 2019-04-16 19:01 ` Borislav Petkov @ 2019-04-17 5:53 ` Pingfan Liu 2019-04-17 16:06 ` Borislav Petkov 0 siblings, 1 reply; 11+ messages in thread From: Pingfan Liu @ 2019-04-17 5:53 UTC (permalink / raw) To: Borislav Petkov Cc: x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Baoquan He, Will Deacon, Nicolas Pitre, Vivek Goyal, Chao Fan, Kirill A. Shutemov, Ard Biesheuvel, Hari Bathini, LKML On Wed, Apr 17, 2019 at 3:01 AM Borislav Petkov <bp@alien8.de> wrote: > > On Mon, Apr 08, 2019 at 01:58:35PM +0800, Pingfan Liu wrote: > > crashkernel=x@y or or =range1:size1[,range2:size2,...]@offset option may > > fail to reserve the required memory region if KASLR puts kernel into the > > region. To avoid this uncertainty, asking KASLR to skip the required > > region. > > > > Signed-off-by: Pingfan Liu <kernelfans@gmail.com> > > Cc: Thomas Gleixner <tglx@linutronix.de> > > Cc: Ingo Molnar <mingo@redhat.com> > > Cc: Borislav Petkov <bp@alien8.de> > > Cc: "H. Peter Anvin" <hpa@zytor.com> > > Cc: Baoquan He <bhe@redhat.com> > > Cc: Will Deacon <will.deacon@arm.com> > > Cc: Nicolas Pitre <nico@linaro.org> > > Cc: Vivek Goyal <vgoyal@redhat.com> > > Cc: Chao Fan <fanc.fnst@cn.fujitsu.com> > > Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > CC: Hari Bathini <hbathini@linux.vnet.ibm.com> > > Cc: linux-kernel@vger.kernel.org > > --- > > arch/x86/boot/compressed/kaslr.c | 40 ++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 40 insertions(+) > > > > diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c > > index 2e53c05..765a593 100644 > > --- a/arch/x86/boot/compressed/kaslr.c > > +++ b/arch/x86/boot/compressed/kaslr.c > > @@ -107,6 +107,7 @@ enum mem_avoid_index { > > MEM_AVOID_BOOTPARAMS, > > MEM_AVOID_MEMMAP_BEGIN, > > MEM_AVOID_MEMMAP_END = MEM_AVOID_MEMMAP_BEGIN + MAX_MEMMAP_REGIONS - 1, > > + MEM_AVOID_CRASHKERNEL, > > MEM_AVOID_MAX, > > }; > > > > @@ -131,6 +132,11 @@ char *skip_spaces(const char *str) > > } > > #include "../../../../lib/ctype.c" > > #include "../../../../lib/cmdline.c" > > +#ifdef CONFIG_CRASH_CORE > > +#define printk > > +#define _BOOT_KASLR > > +#include "../../../../lib/parse_crashkernel.c" > > +#endif > > > > static int > > parse_memmap(char *p, unsigned long long *start, unsigned long long *size) > > @@ -292,6 +298,39 @@ static void handle_mem_options(void) > > return; > > } > > > > +static u64 mem_ram_size(void) > > +{ > > + struct boot_e820_entry *entry; > > + u64 total_sz = 0; > > + int i; > > + > > + for (i = 0; i < boot_params->e820_entries; i++) { > > + entry = &boot_params->e820_table[i]; > > + /* Skip non-RAM entries. */ > > + if (entry->type != E820_TYPE_RAM) > > + continue; > > + total_sz += entry->size; > > + } > > + return total_sz; > > +} > > + > > +/* > > + * For crashkernel=size@offset or =range1:size1[,range2:size2,...]@offset > > + * options, recording mem_avoid for them. > > + */ > > +static void handle_crashkernel_options(void) > > +{ > > + unsigned long long crash_size, crash_base = 0; > > + char *cmdline = (char *)get_cmd_line_ptr(); > > + u64 total_sz = mem_ram_size(); > > + > > + parse_crashkernel(cmdline, total_sz, &crash_size, &crash_base); > > That function has a return value which you could test. And then you > don't need to set crash_base to 0 above. > Take __parse_crashkernel()->parse_crashkernel_simple() for example. If no offset given, then it still return 0, but crash_base is dangling. Regards, Pingfan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/2] x86/boot/KASLR: skip the specified crashkernel region 2019-04-17 5:53 ` Pingfan Liu @ 2019-04-17 16:06 ` Borislav Petkov 2019-04-18 7:56 ` Pingfan Liu 0 siblings, 1 reply; 11+ messages in thread From: Borislav Petkov @ 2019-04-17 16:06 UTC (permalink / raw) To: Pingfan Liu Cc: x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Baoquan He, Will Deacon, Nicolas Pitre, Vivek Goyal, Chao Fan, Kirill A. Shutemov, Ard Biesheuvel, Hari Bathini, LKML On Wed, Apr 17, 2019 at 01:53:37PM +0800, Pingfan Liu wrote: > Take __parse_crashkernel()->parse_crashkernel_simple() for example. If > no offset given, then it still return 0, but crash_base is dangling. Well, that is bad design. parse_crashkernel_simple() should return a *separate* distinct value which denotes that @offset hasn't been passed. Please fix that by having it return 1 or something else positive to denote that there wasn't an [@offset] given. And then correct that crap here: static void __init reserve_crashkernel(void) { ... ret = parse_crashkernel(boot_command_line, total_mem, &crash_size, &crash_base); if (ret != 0 || crash_size <= 0) { where *two*! variables are used as return values from a single function. That's just sloppy. Thx. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/2] x86/boot/KASLR: skip the specified crashkernel region 2019-04-17 16:06 ` Borislav Petkov @ 2019-04-18 7:56 ` Pingfan Liu 2019-04-18 12:32 ` Borislav Petkov 0 siblings, 1 reply; 11+ messages in thread From: Pingfan Liu @ 2019-04-18 7:56 UTC (permalink / raw) To: Borislav Petkov Cc: x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Baoquan He, Will Deacon, Nicolas Pitre, Vivek Goyal, Chao Fan, Kirill A. Shutemov, Ard Biesheuvel, Hari Bathini, LKML On Thu, Apr 18, 2019 at 12:06 AM Borislav Petkov <bp@alien8.de> wrote: > > On Wed, Apr 17, 2019 at 01:53:37PM +0800, Pingfan Liu wrote: > > Take __parse_crashkernel()->parse_crashkernel_simple() for example. If > > no offset given, then it still return 0, but crash_base is dangling. Sorry for misleading, I made a mistake. In parse_crashkernel()->__parse_crashkernel(), { *crash_size = 0; *crash_base = 0;}. Hence no need to initialize crash_base in handle_crashkernel_options(). > > Well, that is bad design. parse_crashkernel_simple() should return a > *separate* distinct value which denotes that @offset hasn't been passed. Then in my case, either no @offset or invalid argument will keep "*crash_base = 0", and KASLR does not care about either of them. > > Please fix that by having it return 1 or something else positive to > denote that there wasn't an [@offset] given. > > And then correct that crap here: > > static void __init reserve_crashkernel(void) > { > ... > > ret = parse_crashkernel(boot_command_line, total_mem, &crash_size, &crash_base); > if (ret != 0 || crash_size <= 0) { It is not elegant. Will try a separate patch to fix it firstly. Thanks, Pingfan > > where *two*! variables are used as return values from a single function. > That's just sloppy. > > Thx. > > -- > Regards/Gruss, > Boris. > > Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/2] x86/boot/KASLR: skip the specified crashkernel region 2019-04-18 7:56 ` Pingfan Liu @ 2019-04-18 12:32 ` Borislav Petkov 2019-05-06 10:08 ` Pingfan Liu 0 siblings, 1 reply; 11+ messages in thread From: Borislav Petkov @ 2019-04-18 12:32 UTC (permalink / raw) To: Pingfan Liu Cc: x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Baoquan He, Will Deacon, Nicolas Pitre, Vivek Goyal, Chao Fan, Kirill A. Shutemov, Ard Biesheuvel, Hari Bathini, LKML On Thu, Apr 18, 2019 at 03:56:09PM +0800, Pingfan Liu wrote: > Then in my case, either no @offset or invalid argument will keep > "*crash_base = 0", and KASLR does not care about either of them. Ok. > It is not elegant. Will try a separate patch to fix it firstly. That's appreciated, thanks. It is about time that whole kexec/kaslr/... code gets some much needed cleaning up and streamlining. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/2] x86/boot/KASLR: skip the specified crashkernel region 2019-04-18 12:32 ` Borislav Petkov @ 2019-05-06 10:08 ` Pingfan Liu 0 siblings, 0 replies; 11+ messages in thread From: Pingfan Liu @ 2019-05-06 10:08 UTC (permalink / raw) To: Borislav Petkov Cc: x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Baoquan He, Will Deacon, Nicolas Pitre, Vivek Goyal, Chao Fan, Kirill A. Shutemov, Ard Biesheuvel, Hari Bathini, LKML On Thu, Apr 18, 2019 at 8:32 PM Borislav Petkov <bp@alien8.de> wrote: > > On Thu, Apr 18, 2019 at 03:56:09PM +0800, Pingfan Liu wrote: > > Then in my case, either no @offset or invalid argument will keep > > "*crash_base = 0", and KASLR does not care about either of them. > > Ok. > > > It is not elegant. Will try a separate patch to fix it firstly. > > That's appreciated, thanks. It is about time that whole kexec/kaslr/... > code gets some much needed cleaning up and streamlining. > I had tried it v1 on https://patchwork.kernel.org/patch/10909627/ and v2 on https://patchwork.kernel.org/patch/10914169/. It seems no more feed back and hard to push forward. Since "x86/boot/KASLR: skip the specified crashkernel region" has no dependency on the above patch, I would like to send the next version for it. Regards, Pingfan ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-05-06 10:08 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-08 5:58 [PATCH v4 0/2] x86/boot/KASLR: skip the specified crashkernel region Pingfan Liu 2019-04-08 5:58 ` [PATCH v4 1/2] kernel/crash_core: separate the parsing routines to lib/parse_crashkernel.c Pingfan Liu 2019-04-16 19:01 ` Borislav Petkov 2019-04-17 5:48 ` Pingfan Liu 2019-04-08 5:58 ` [PATCH v4 2/2] x86/boot/KASLR: skip the specified crashkernel region Pingfan Liu 2019-04-16 19:01 ` Borislav Petkov 2019-04-17 5:53 ` Pingfan Liu 2019-04-17 16:06 ` Borislav Petkov 2019-04-18 7:56 ` Pingfan Liu 2019-04-18 12:32 ` Borislav Petkov 2019-05-06 10:08 ` Pingfan Liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).