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

[James Bottomley <jbottomley@odin.com>]

On Wed, 2015-09-16 at 13:25 +0200, Ard Biesheuvel wrote:
> On 16 September 2015 at 12:08, Borislav Petkov <bp@suse.de> wrote:
> > On Wed, Sep 09, 2015 at 12:21:23PM +0100, Matt Fleming wrote:
> >> On Wed, 09 Sep, at 08:33:07AM, joeyli wrote:
> >> >
> >> > Yes, the machine on my hand has EFI_PROPERTIES_TABLE enabled, and it doesn't
> >> > boot without your patch.
> >>
> >> Awesome. Could you test the following patch instead?
> >>
> >> ---
> >>
> >> From 24d324b781a3b688dcc265995949a9cf4e8af687 Mon Sep 17 00:00:00 2001
> >> From: Matt Fleming <matt.fleming@intel.com>
> >> Date: Thu, 3 Sep 2015 15:56:25 +0100
> >> Subject: [PATCH v2] x86/efi: Map EFI memmap entries in-order at runtime
> >>
> >> Beginning with UEFI v2.5 EFI_PROPERTIES_TABLE was introduced that
> >> signals that the firmware PE/COFF loader supports splitting code and
> >> data sections of PE/COFF images into separate EFI memory map entries.
> >> This allows the kernel to map those regions with strict memory
> >> protections, e.g. EFI_MEMORY_RO for code, EFI_MEMORY_XP for data, etc.
> >>
> >> Unfortunately, an unwritten requirement of this new feature is that
> >> the regions need to be mapped with the same offsets relative to each
> >> other as observed in the EFI memory map. If this is not done crashes
> >
> > Let me get this straight: this looks like the next EFI screwup which
> > practically requires specific mapping placement in VA space just
> > because it uses relative addresses?
> 
> Both relative and absolute references, currently. The latter are also
> affected since the relocation offset that is applied to all PE/COFF
> relocation entries is based on the displacement of ImageBase, and
> absolute references to symbols in .data need to be treated specially
> (since it may be shifted relative to the .text section containing
> ImageBase). This could be worked around by converting each absolute
> reference individually using ConvertPointer () [and I have a proof of
> concept that actually makes the problem go away on x86] but it would
> still be only a partial solution, since relative references are not
> tracked in the PE/COFF metadata, so even if we wanted to, it would be
> intractible to find each cross-section relative reference and do the
> fixup.
> 
> > And since you say "unwritten" this
> > practically a requirement is not even in the spec?
> >
> 
> No, it seems nobody thought of this when designing the feature.

To add colour: our problem is section relative references (either loads
or jumps).  The PE/COFF linker is allowed not to emit relocations for
section relative references because it expects that the sections will
always be loaded at the same relative offset.  It looks like it is
possible to force them to have relocation entries, so it would be
possible to add to the standard language requiring this for UEFI
compatible binaries, but that won't help with any of the existing
PE/COFF stuff in the field.

The problem is that to apply the various protections UEFI is
introducing, we're trying to relocate the sections and that's what's
causing the issue.  Before this, no-one really thought of mapping the
sections at different relative addresses.  It's really an unexpected
weakness in the PE/COFF spec that we can't fix, so we have to work
around it.

James

> > Can we state explicitly in the spec NOT to rely on mapping VA placement?
> > I mean, this "unwritten" requirement is seriously screwed on soo many
> > levels...
> >
> 
> Several solutions and/or work arounds are currently under discussion
>