linux-kernel - Re: [PATCH 1/2] x86/efi: Map EFI memmap entries in-order at runtime

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:	Mon, 28 Sep 2015 10:22:45 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Ard Biesheuvel <ard.biesheuvel@...aro.org>
Cc:	Matt Fleming <matt@...eblueprint.co.uk>,
	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	Matt Fleming <matt.fleming@...el.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>,
	"Lee, Chun-Yi" <jlee@...e.com>, Borislav Petkov <bp@...e.de>,
	Leif Lindholm <leif.lindholm@...aro.org>,
	Peter Jones <pjones@...hat.com>,
	James Bottomley <JBottomley@...n.com>,
	Matthew Garrett <mjg59@...f.ucam.org>,
	Dave Young <dyoung@...hat.com>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Borislav Petkov <bp@...en8.de>,
	Andy Lutomirski <luto@...nel.org>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	Brian Gerst <brgerst@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 1/2] x86/efi: Map EFI memmap entries in-order at runtime


* Ard Biesheuvel <ard.biesheuvel@...aro.org> wrote:

> On 27 September 2015 at 08:03, Ingo Molnar <mingo@...nel.org> wrote:
> >
> > * Matt Fleming <matt@...eblueprint.co.uk> wrote:
> >
> [...]
> >> [...] The actual virtual addresses we pick are exactly the same with the two
> >> patches.
> >
> > So I'm NAK-ing this for now:
> >
> >  - The code is it reads today pretends to be an 'allocator'. It is _NOT_ an
> >    allocator, because all the sections have already been determined by the
> >    firmware, and, as we just learned the hard way, we do not want to deviate from
> >    that! There's nothing to 'allocate'!
> >
> >    What these patches seem to implement is an elaborate 'allocator' that ends up
> >    doing nothing on 'new 64-bit' ...
> >
> >  - The 32-bit and 64-bit and 'old_mmap' asymmetries:
> >
> >         if (!efi_enabled(EFI_OLD_MEMMAP) && efi_enabled(EFI_64BIT)) {
> >
> >    seem fragile and nonsensical. The question is: is it possible for the whole EFI
> >    image to be larger than a couple of megabytes? If not then 32-bit should just
> >    mirror the firmware layout as well, and if EFI_OLD_MEMMAP does anything
> >    differently from this _obvious_ 1:1 mapping of the EFI memory offsets then it's
> >    not worth keeping as a legacy, because there's just nothing better than
> >    mirroring the firmware layout.
> >
> > My suggestion would be to just 1:1 map what the EFI tables describe, modulo the
> > single absolute offset by which we shift the whole thing to a single base.
> >
> > Is there any technical reason why we'd want to deviate from that? Gigabytes of
> > tables or gigabytes of holes that 32-bit cannot handle? Firmware that wants an OS
> > layout that differs from the firmware layout?
> >
> 
> The combined EFI_MEMORY_RUNTIME regions could span the entire 1:1 addressable PA 
> space. They usually don't but it is a possibility, which means 32-bit will not 
> generally be able to support this approach. [...]

Ok, that's a good argument which invalidates my NAK.

> [...] For 64-bit ARM, there are some minor complications when the base of RAM is 
> up very high in physical memory, but we already fixed that for the boot time ID 
> map and for KVM.
> 
> > Also, nobody seems to be asking the obvious hardware compatibility question 
> > when trying to implement a standard influenced in great part by an entity that 
> > is partly ignorant of and partly hostile to Linux: how does Windows map the 
> > EFI sections, under what OSs are these firmware versions tested? I suspect no 
> > firmware is released that crashes on bootup on all OSs that can run on that 
> > hardware, right?
> 
> Interestingly, it was the other way around this time. The engineers that 
> implemented this feature for EDK2 could not boot Windows 8 anymore, because it 
> supposedly maps the regions in reverse order as well (and MS too will need to 
> backport a fix that inverts the mapping order). The engineers also tested 
> Linux/x86, by means of a SUSE installer image, which booted fine, most likely 
> due to the fact that it is an older version which still uses the old memmap 
> layout.

That's nice to hear!

> My concern with all of this is that this security feature will become an obscure 
> opt-in feature rather than something UEFIv2.5 firmware implementations can 
> enable by default.

Ok, so I think the patches are mostly fine after all, except that I don't think 
the condition on 64-bit makes any sense:

+       if (!efi_enabled(EFI_OLD_MEMMAP) && efi_enabled(EFI_64BIT)) {

I can see us being nervous wrt. backported patches, but is there any strong reason 
to not follow this up with a third (non-backported) patch that changes this to:

+       if (!efi_enabled(EFI_OLD_MEMMAP)) {

for v4.4?

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/