From: Matt Fleming <matt@codeblueprint.co.uk>
To: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: Baoquan He <bhe@redhat.com>, Kees Cook <keescook@chromium.org>,
linux-kernel@vger.kernel.org, x86@kernel.org,
Thomas Gleixner <tglx@linutronix.de>,
"H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@kernel.org>,
izumi.taku@jp.fujitsu.com, Thomas Garnier <thgarnie@google.com>,
fanc.fnst@cn.fujitsu.com,
"Jun'ichi Nomura" <j-nomura@ce.jp.nec.com>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: [PATCH v3 1/2] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice
Date: Mon, 24 Jul 2017 14:17:07 +0100 [thread overview]
Message-ID: <20170724131707.GA11076@codeblueprint.co.uk> (raw)
In-Reply-To: <1499665896-23731-1-git-send-email-n-horiguchi@ah.jp.nec.com>
On Mon, 10 Jul, at 02:51:35PM, Naoya Horiguchi wrote:
> KASLR chooses kernel location from E820_TYPE_RAM regions by walking over
> e820 entries now. E820_TYPE_RAM includes EFI_BOOT_SERVICES_CODE and
> EFI_BOOT_SERVICES_DATA, so those regions can be the target. According to
> UEFI spec, all memory regions marked as EfiBootServicesCode and
> EfiBootServicesData are available for free memory after the first call
> of ExitBootServices(). So such regions should be usable for kernel on
> spec basis.
>
> In x86, however, we have some workaround for broken firmware, where we
> keep such regions reserved until SetVirtualAddressMap() is done.
> See the following code in should_map_region():
>
> static bool should_map_region(efi_memory_desc_t *md)
> {
> ...
> /*
> * Map boot services regions as a workaround for buggy
> * firmware that accesses them even when they shouldn't.
> *
> * See efi_{reserve,free}_boot_services().
> */
> if (md->type == EFI_BOOT_SERVICES_CODE ||
> md->type == EFI_BOOT_SERVICES_DATA)
> return false;
>
> This workaround suppressed a boot crash, but potential issues still
> remain because no one prevents the regions from overlapping with kernel
> image by KASLR.
>
> So let's make sure that EFI_BOOT_SERVICES_{CODE|DATA} regions are never
> chosen as kernel memory for the workaround to work fine.
>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> ---
> v2 -> v3:
> - skip EFI_LOADER_CODE and EFI_LOADER_DATA in region scan
>
> v1 -> v2:
> - switch efi_mirror_found to local variable
> - insert break when EFI_MEMORY_MORE_RELIABLE found
> ---
> arch/x86/boot/compressed/kaslr.c | 44 +++++++++++++++++++++++++++++-----------
> 1 file changed, 32 insertions(+), 12 deletions(-)
I think the patch content is fine, but please update the comments and
the changelog to say why EFI_CONVENTIONAL_MEMORY was chosen, i.e. that
it's the only memory type that we know to be free.
next prev parent reply other threads:[~2017-07-24 13:17 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-06 8:31 [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Naoya Horiguchi
2017-07-06 9:13 ` Chao Fan
2017-07-06 9:22 ` Naoya Horiguchi
2017-07-06 9:36 ` Chao Fan
2017-07-06 9:18 ` Baoquan He
2017-07-06 9:36 ` Naoya Horiguchi
2017-07-06 10:04 ` Chao Fan
2017-07-06 10:20 ` Chao Fan
2017-07-06 14:57 ` Matt Fleming
2017-07-07 3:07 ` Baoquan He
2017-07-07 6:11 ` Naoya Horiguchi
2017-07-07 10:58 ` Matt Fleming
2017-07-10 5:47 ` Naoya Horiguchi
2017-07-10 5:51 ` [PATCH v3 1/2] " Naoya Horiguchi
2017-07-24 13:17 ` Matt Fleming [this message]
2017-07-25 6:17 ` Naoya Horiguchi
2017-07-10 5:51 ` [PATCH v3 2/2] x86/efi: clean up dead code around efi_reserve_boot_services() Naoya Horiguchi
2017-07-24 13:20 ` Matt Fleming
2017-07-26 0:12 ` Naoya Horiguchi
2017-07-26 1:13 ` Baoquan He
2017-07-26 1:34 ` Baoquan He
2017-07-28 6:48 ` [PATCH] x86/boot: check overlap between kernel and EFI_BOOT_SERVICES_* Naoya Horiguchi
2017-07-29 10:04 ` kbuild test robot
2017-07-29 13:01 ` [RFC PATCH] x86/boot: efi_kernel_boot_services_overlap can be static kbuild test robot
2017-07-29 13:01 ` [PATCH] x86/boot: check overlap between kernel and EFI_BOOT_SERVICES_* kbuild test robot
2017-08-23 8:24 ` Baoquan He
2017-07-07 10:56 ` [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Matt Fleming
2017-07-09 10:44 ` Baoquan He
2017-07-09 14:27 ` Baoquan He
2017-07-07 7:22 ` [PATCH v2 1/2] " Naoya Horiguchi
2017-07-07 7:22 ` [PATCH v2 2/2] x86/efi: clean up dead code around efi_reserve_boot_services() Naoya Horiguchi
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=20170724131707.GA11076@codeblueprint.co.uk \
--to=matt@codeblueprint.co.uk \
--cc=ard.biesheuvel@linaro.org \
--cc=bhe@redhat.com \
--cc=fanc.fnst@cn.fujitsu.com \
--cc=hpa@zytor.com \
--cc=izumi.taku@jp.fujitsu.com \
--cc=j-nomura@ce.jp.nec.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=n-horiguchi@ah.jp.nec.com \
--cc=tglx@linutronix.de \
--cc=thgarnie@google.com \
--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