Re: [PATCH v4 1/3] KASLR: Parse all memmap entries in cmdline
On 05/10/17 at 02:29pm, Thomas Gleixner wrote:
> On Tue, 9 May 2017, Baoquan He wrote:
>
> > 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
>
> Can you please finally stop using this 'This patch does foo', 'In this
> patch' phrases. They are bogus. We already know that this is a patch
> otherwise you wouldn't have sent it.
>
> See Documentation/process/SubmittingPatches.txt
>
> Aside of that can you please use properly written out words instead of
> using random abbreviations in the changelog, e.g. command line instead of
> cmdline?
>
> > memmap, all of them will be parsed and handled.
>
> A proper example would be:
>
>Address this by checking each command line token for a "memmap=" match
>and parse each instance instead of using cmdline_find_option().
Sorry for those mistakes. Will change accordingly when repost.
Thanks
Baoquan
Re: [PATCH v4 1/3] KASLR: Parse all memmap entries in cmdline
On 05/10/17 at 02:29pm, Thomas Gleixner wrote:
> On Tue, 9 May 2017, Baoquan He wrote:
>
> > 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
>
> Can you please finally stop using this 'This patch does foo', 'In this
> patch' phrases. They are bogus. We already know that this is a patch
> otherwise you wouldn't have sent it.
>
> See Documentation/process/SubmittingPatches.txt
>
> Aside of that can you please use properly written out words instead of
> using random abbreviations in the changelog, e.g. command line instead of
> cmdline?
>
> > memmap, all of them will be parsed and handled.
>
> A proper example would be:
>
>Address this by checking each command line token for a "memmap=" match
>and parse each instance instead of using cmdline_find_option().
Sorry for those mistakes. Will change accordingly when repost.
Thanks
Baoquan
Re: [PATCH v4 1/3] KASLR: Parse all memmap entries in cmdline
On Tue, 9 May 2017, Baoquan He wrote:
> 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
Can you please finally stop using this 'This patch does foo', 'In this
patch' phrases. They are bogus. We already know that this is a patch
otherwise you wouldn't have sent it.
See Documentation/process/SubmittingPatches.txt
Aside of that can you please use properly written out words instead of
using random abbreviations in the changelog, e.g. command line instead of
cmdline?
> memmap, all of them will be parsed and handled.
A proper example would be:
Address this by checking each command line token for a "memmap=" match
and parse each instance instead of using cmdline_find_option().
Thanks,
tglx
Re: [PATCH v4 1/3] KASLR: Parse all memmap entries in cmdline
On Tue, 9 May 2017, Baoquan He wrote:
> 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
Can you please finally stop using this 'This patch does foo', 'In this
patch' phrases. They are bogus. We already know that this is a patch
otherwise you wouldn't have sent it.
See Documentation/process/SubmittingPatches.txt
Aside of that can you please use properly written out words instead of
using random abbreviations in the changelog, e.g. command line instead of
cmdline?
> memmap, all of them will be parsed and handled.
A proper example would be:
Address this by checking each command line token for a "memmap=" match
and parse each instance instead of using cmdline_find_option().
Thanks,
tglx
[PATCH v4 1/3] KASLR: Parse all memmap entries in cmdline
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
---
arch/x86/boot/compressed/cmdline.c | 2 +-
arch/x86/boot/compressed/kaslr.c | 136 ++---
arch/x86/boot/string.c | 8 +++
3 files changed, 91 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 54c24f0..106e13b 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -9,16 +9,41 @@
* contain the entire properly aligned running kernel image.
*
*/
+
+/*
+ * isspace() in linux/ctype.h is expected by next_args() to filter
+ * out "space/lf/tab". While boot/ctype.h conflicts with linux/ctype.h,
+ * since isdigit() is implemented in both of them. Hence disable it
+ * here.
+ */
+#define BOOT_CTYPE_H
+
+/*
+ * _ctype[] in lib/ctype.c is needed by isspace() of linux/ctype.h.
+ * While both lib/ctype.c and lib/cmdline.c will bring EXPORT_SYMBOL
+ * which is meaningless and will cause compiling error in some cases.
+ * So do not include linux/export.h and define EXPORT_SYMBOL(sym)
+ * as empty.
+ */
+#define _LINUX_EXPORT_H
+#define EXPORT_SYMBOL(sym)
+
#include "misc.h"
#include "error.h"
-#include "../boot.h"
#include
#include
#include
#include
+#include
#include
+/* Macros used by the included decompressor code below. */
+#define STATIC
+#include
+
+extern unsigned long get_cmd_line_ptr(void);
+
/* Simplified build-specific string for starting entropy. */
static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
@@ -62,6 +87,7 @@ struct mem_vector {
static bool memmap_too_large;
+
enum mem_avoid_index {
MEM_AVOID_ZO_RANGE = 0,
MEM_AVOID_INITRD,
@@ -85,49 +111,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, , 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 +133,7 @@ parse_memmap(char *p, unsigned long long *start, unsigned
long long *size)
return -EINVAL;
oldp = p;
- *size = _memparse(p, );
+ *size = memparse(p, );
if (p == oldp)
return -EINVAL;
@@ -155,27 +146,21 @@ parse_memmap(char *p, unsigned long long *start, unsigned
long long *size)
case '#':
case '$':
case '!':
- *start = _memparse(p + 1, );
+ *start = memparse(p + 1, );
return 0;
}
return -EINVAL;
}
-static void mem_avoid_memmap(void)
+static void mem_avoid_memmap(char *str)
{
- char arg[128];
+ static int i;
int rc;
- int i;
- char *str;
-
[PATCH v4 1/3] KASLR: Parse all memmap entries in cmdline
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
---
arch/x86/boot/compressed/cmdline.c | 2 +-
arch/x86/boot/compressed/kaslr.c | 136 ++---
arch/x86/boot/string.c | 8 +++
3 files changed, 91 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 54c24f0..106e13b 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -9,16 +9,41 @@
* contain the entire properly aligned running kernel image.
*
*/
+
+/*
+ * isspace() in linux/ctype.h is expected by next_args() to filter
+ * out "space/lf/tab". While boot/ctype.h conflicts with linux/ctype.h,
+ * since isdigit() is implemented in both of them. Hence disable it
+ * here.
+ */
+#define BOOT_CTYPE_H
+
+/*
+ * _ctype[] in lib/ctype.c is needed by isspace() of linux/ctype.h.
+ * While both lib/ctype.c and lib/cmdline.c will bring EXPORT_SYMBOL
+ * which is meaningless and will cause compiling error in some cases.
+ * So do not include linux/export.h and define EXPORT_SYMBOL(sym)
+ * as empty.
+ */
+#define _LINUX_EXPORT_H
+#define EXPORT_SYMBOL(sym)
+
#include "misc.h"
#include "error.h"
-#include "../boot.h"
#include
#include
#include
#include
+#include
#include
+/* Macros used by the included decompressor code below. */
+#define STATIC
+#include
+
+extern unsigned long get_cmd_line_ptr(void);
+
/* Simplified build-specific string for starting entropy. */
static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
@@ -62,6 +87,7 @@ struct mem_vector {
static bool memmap_too_large;
+
enum mem_avoid_index {
MEM_AVOID_ZO_RANGE = 0,
MEM_AVOID_INITRD,
@@ -85,49 +111,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, , 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 +133,7 @@ parse_memmap(char *p, unsigned long long *start, unsigned
long long *size)
return -EINVAL;
oldp = p;
- *size = _memparse(p, );
+ *size = memparse(p, );
if (p == oldp)
return -EINVAL;
@@ -155,27 +146,21 @@ parse_memmap(char *p, unsigned long long *start, unsigned
long long *size)
case '#':
case '$':
case '!':
- *start = _memparse(p + 1, );
+ *start = memparse(p + 1, );
return 0;
}
return -EINVAL;
}
-static void mem_avoid_memmap(void)
+static void mem_avoid_memmap(char *str)
{
- char arg[128];
+ static int i;
int rc;
- int i;
- char *str;
- /* See if we
