linux-kernel - Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 7 Jul 2017 11:56:58 +0100
From:   Matt Fleming <matt@...eblueprint.co.uk>
To:     Baoquan He <bhe@...hat.com>
Cc:     Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
        Kees Cook <keescook@...omium.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...nel.org>,
        "izumi.taku@...fujitsu.com" <izumi.taku@...fujitsu.com>,
        Thomas Garnier <thgarnie@...gle.com>,
        "fanc.fnst@...fujitsu.com" <fanc.fnst@...fujitsu.com>,
        Junichi Nomura <j-nomura@...jp.nec.com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>
Subject: Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA}
 from KASLR's choice

On Fri, 07 Jul, at 11:07:59AM, Baoquan He wrote:
> On 07/06/17 at 03:57pm, Matt Fleming wrote:
> > On Thu, 06 Jul, at 08:31:07AM, Naoya Horiguchi wrote:
> > > +	for (i = 0; i < nr_desc; i++) {
> > > +		md = (efi_memory_desc_t *)(pmap + (i * e->efi_memdesc_size));
> > > +
> > > +		/*
> > > +		 * EFI_BOOT_SERVICES_{CODE|DATA} are avoided because boot
> > > +		 * services regions could be accessed after ExitBootServices()
> > > +		 * due to the workaround for buggy firmware.
> > > +		 */
> > > +		if (!(md->type == EFI_LOADER_CODE ||
> > > +		      md->type == EFI_LOADER_DATA ||
> > > +		      md->type == EFI_CONVENTIONAL_MEMORY))
> > > +			continue;
> > 
> > Wouldn't it make more sense to *only* use EFI_CONVENTIONAL_MEMORY?
> > 
> > You can't re-use EFI_LOADER_* regions because the kaslr code is run so
> > early in boot that you've no idea if data the kernel will need is in
> > those EFI_LOADER_* regions.
> > 
> > For example, we pass struct setup_data objects inside of
> > EFI_LOADER_DATA regions.
> 
> It doesn't matter because we have tried to avoid those memory setup_data
> resides in in mem_avoid_overlap(). Here discarding EFI_LOADER_* could
> discard the whole regions while setup_data could occupy small part of
> them.

What about the GDT that we allocate in the x86 EFI boot stub as
EFI_LOADER_DATA? Are there functions to avoid that too?

What about any future uses we add? Who's going to remember to patch
the kaslr code which now duplicates some of the EFI memory map logic?

All of these problems can avoided if you just stick with
EFI_CONVENTIONAL_MEMORY.

Honestly, how much memory do we expect to waste if we ignore
EFI_LOADER_* regions?

Also, the fact that you're referencing EFI-specific boot quirks in the
kaslr code should be a massive red flag that you're playing with the
innards of the EFI subsystem.