From: Kees Cook <keescook@chromium.org>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Ingo Molnar <mingo@kernel.org>, Dave Jiang <dave.jiang@intel.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
X86 ML <x86@kernel.org>, david <david@fromorbit.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-nvdimm@lists.01.org" <linux-nvdimm@ml01.01.org>
Subject: Re: [PATCH] x86: fix kaslr and memmap collision
Date: Tue, 22 Nov 2016 14:37:51 -0800 [thread overview]
Message-ID: <CAGXu5jJukgbW+drE6w4=0iqaKpmXyCxmRjVPcLQ8Ypq2e4a=jg@mail.gmail.com> (raw)
In-Reply-To: <CAPcyv4jR8cVG4y90aCNycc8-PY29Jm3J89-w87dgE4hBpfVxWw@mail.gmail.com>
On Tue, Nov 22, 2016 at 11:01 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Tue, Nov 22, 2016 at 10:54 AM, Kees Cook <keescook@chromium.org> wrote:
>> On Tue, Nov 22, 2016 at 9:26 AM, Dan Williams <dan.j.williams@intel.com> wrote:
>>> [ replying for Dave since he's offline today and tomorrow ]
>>>
>>> On Tue, Nov 22, 2016 at 12:47 AM, Ingo Molnar <mingo@kernel.org> wrote:
>>>>
>>>> * Dave Jiang <dave.jiang@intel.com> wrote:
>>>>
>>>>> CONFIG_RANDOMIZE_BASE relocates the kernel to a random base address.
>>>>> However it does not take into account the memmap= parameter passed in from
>>>>> the kernel commandline.
>>>>
>>>> memmap= parameters are often used as a list.
>>>>
>>>>> [...] This results in the kernel sometimes being put in the middle of the user
>>>>> memmap. [...]
>>>>
>>>> What does this mean? If memmap= is used to re-define the memory map then the
>>>> kernel getting in the middle of a RAM area is what we want, isn't it? What we
>>>> don't want is for the kernel to get into reserved areas, right?
>>>
>>> Right, this is about teaching kaslr to not land the kernel in newly
>>> defined reserved regions that were not marked reserved in the initial
>>> e820 map from platform firmware.
>>>
>>>>> [...] Check has been added in the kaslr in order to avoid the region marked by
>>>>> memmap.
>>>>
>>>> What does this mean?
>>>
>>> Is this clearer? "Update the set of 'mem_avoid' entries to exclude
>>> 'memmap=' defined reserved regions from the set of valid address range
>>> to land the kernel image."
>>>
>>>>
>>>>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>>>>> ---
>>>>> arch/x86/boot/boot.h | 2 ++
>>>>> arch/x86/boot/compressed/kaslr.c | 45 ++++++++++++++++++++++++++++++++++++++
>>>>> arch/x86/boot/string.c | 25 +++++++++++++++++++++
>>>>> 3 files changed, 72 insertions(+)
>>>>>
>>>>> diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
>>>>> index e5612f3..0d5fe5b 100644
>>>>> --- a/arch/x86/boot/boot.h
>>>>> +++ b/arch/x86/boot/boot.h
>>>>> @@ -332,6 +332,8 @@ int strncmp(const char *cs, const char *ct, size_t count);
>>>>> size_t strnlen(const char *s, size_t maxlen);
>>>>> unsigned int atou(const char *s);
>>>>> unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int base);
>>>>> +unsigned long simple_strtoul(const char *cp, char **endp, unsigned int base);
>>>>> +long simple_strtol(const char *cp, char **endp, unsigned int base);
>>>>> size_t strlen(const char *s);
>>>>>
>>>>> /* tty.c */
>>>>> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
>>>>> index a66854d..6fb8f1ec 100644
>>>>> --- a/arch/x86/boot/compressed/kaslr.c
>>>>> +++ b/arch/x86/boot/compressed/kaslr.c
>>>>> @@ -11,6 +11,7 @@
>>>>> */
>>>>> #include "misc.h"
>>>>> #include "error.h"
>>>>> +#include "../boot.h"
>>>>>
>>>>> #include <generated/compile.h>
>>>>> #include <linux/module.h>
>>>>> @@ -61,6 +62,7 @@ enum mem_avoid_index {
>>>>> MEM_AVOID_INITRD,
>>>>> MEM_AVOID_CMDLINE,
>>>>> MEM_AVOID_BOOTPARAMS,
>>>>> + MEM_AVOID_MEMMAP,
>>>>> MEM_AVOID_MAX,
>>>>> };
>>>>>
>>>>> @@ -77,6 +79,37 @@ static bool mem_overlaps(struct mem_vector *one, struct mem_vector *two)
>>>>> return true;
>>>>> }
>>>>>
>>>>> +#include "../../../../lib/cmdline.c"
>>>>> +
>>>>> +static int
>>>>> +parse_memmap(char *p, unsigned long long *start, unsigned long long *size)
>>>>> +{
>>>>> + char *oldp;
>>>>> +
>>>>> + if (!p)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + /* we don't care about this option here */
>>>>> + if (!strncmp(p, "exactmap", 8))
>>>>> + return -EINVAL;
>>>>> +
>>>>> + oldp = p;
>>>>> + *size = memparse(p, &p);
>>>>> + if (p == oldp)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + switch (*p) {
>>>>> + case '@':
>>>>> + case '#':
>>>>> + case '$':
>>>>> + case '!':
>>>>> + *start = memparse(p+1, &p);
>>>>> + return 0;
>>>>> + }
>>>>> +
>>>>> + return -EINVAL;
>>>>> +}
>>>>> +
>>>>> /*
>>>>> * 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
>>>>> @@ -158,6 +191,8 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
>>>>> u64 initrd_start, initrd_size;
>>>>> u64 cmd_line, cmd_line_size;
>>>>> char *ptr;
>>>>> + char arg[38];
>>>>
>>>> Where does the magic '38' come from?
>>>>
>>>>> + unsigned long long memmap_start, memmap_size;
>>>>>
>>>>> /*
>>>>> * Avoid the region that is unsafe to overlap during
>>>>> @@ -195,6 +230,16 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
>>>>> add_identity_map(mem_avoid[MEM_AVOID_BOOTPARAMS].start,
>>>>> mem_avoid[MEM_AVOID_BOOTPARAMS].size);
>>>>>
>>>>> + /* see if we have any memmap areas */
>>>>> + if (cmdline_find_option("memmap", arg, sizeof(arg)) > 0) {
>>>>> + int rc = parse_memmap(arg, &memmap_start, &memmap_size);
>>>>> +
>>>>> + if (!rc) {
>>>>> + mem_avoid[MEM_AVOID_MEMMAP].start = memmap_start;
>>>>> + mem_avoid[MEM_AVOID_MEMMAP].size = memmap_size;
>>>>> + }
>>>>> + }
>>>>> +
>>>>
>>>> This only handles a single (first) memmap argument, is that sufficient?
>>>
>>> No, you're right, we need to handle multiple ranges. Since the
>>> mem_avoid array is statically allocated perhaps we can handle up to 4
>>> memmap= entries, but past that point disable kaslr for that boot?
>>
>> Yeah, that seems fine to me. I assume it's rare to have 4?
>>
>
> It should be rare to have *one* since ACPI 6.0 added support for
> communicating persistent memory ranges. However there are legacy
> nvdimm users that I know are doing at least 2, but I have hard time
> imagining they would ever do more than 4.
Cool. As long as it announces KASLR being disabled (as in some of the
other conditions) that should be fine.
-Kees
--
Kees Cook
Nexus Security
next prev parent reply other threads:[~2016-11-22 22:37 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-22 0:22 [PATCH] x86: fix kaslr and memmap collision Dave Jiang
2016-11-22 8:47 ` Ingo Molnar
2016-11-22 17:26 ` Dan Williams
2016-11-22 18:54 ` Kees Cook
2016-11-22 19:01 ` Dan Williams
2016-11-22 22:37 ` Kees Cook [this message]
2016-11-24 0:04 ` Dave Chinner
2016-11-24 19:30 ` Dan Williams
2017-01-03 8:31 ` Baoquan He
2017-01-03 16:27 ` Ross Zwisler
2017-01-03 18:24 ` Dan Williams
2017-01-03 20:15 ` Dave Jiang
2017-01-04 1:57 ` Baoquan He
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAGXu5jJukgbW+drE6w4=0iqaKpmXyCxmRjVPcLQ8Ypq2e4a=jg@mail.gmail.com' \
--to=keescook@chromium.org \
--cc=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=david@fromorbit.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvdimm@ml01.01.org \
--cc=mingo@kernel.org \
--cc=mingo@redhat.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).