Skip to content
  • Matt Fleming's avatar
    x86/efi: Fix boot crash by always mapping boot service regions into new EFI page tables · 452308de
    Matt Fleming authored
    Some machines have EFI regions in page zero (physical address
    0x00000000) and historically that region has been added to the e820
    map via trim_bios_range(), and ultimately mapped into the kernel page
    tables. It was not mapped via efi_map_regions() as one would expect.
    
    Alexis reports that with the new separate EFI page tables some boot
    services regions, such as page zero, are not mapped. This triggers an
    oops during the SetVirtualAddressMap() runtime call.
    
    For the EFI boot services quirk on x86 we need to memblock_reserve()
    boot services regions until after SetVirtualAddressMap(). Doing that
    while respecting the ownership of regions that may have already been
    reserved by the kernel was the motivation behind this commit:
    
      7d68dc3f
    
     ("x86, efi: Do not reserve boot services regions within reserved areas")
    
    That patch was merged at a time when the EFI runtime virtual mappings
    were inserted into the kernel page tables as described above, and the
    trick of setting ->numpages (and hence the region size) to zero to
    track regions that should not be freed in efi_free_boot_services()
    meant that we never mapped those regions in efi_map_regions(). Instead
    we were relying solely on the existing kernel mappings.
    
    Now that we have separate page tables we need to make sure the EFI
    boot services regions are mapped correctly, even if someone else has
    already called memblock_reserve(). Instead of stashing a tag in
    ->numpages, set the EFI_MEMORY_RUNTIME bit of ->attribute. Since it
    generally makes no sense to mark a boot services region as required at
    runtime, it's pretty much guaranteed the firmware will not have
    already set this bit.
    
    For the record, the specific circumstances under which Alexis
    triggered this bug was that an EFI runtime driver on his machine was
    responding to the EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE event during
    SetVirtualAddressMap().
    
    The event handler for this driver looks like this,
    
      sub rsp,0x28
      lea rdx,[rip+0x2445] # 0xaa948720
      mov ecx,0x4
      call func_aa9447c0  ; call to ConvertPointer(4, & 0xaa948720)
      mov r11,QWORD PTR [rip+0x2434] # 0xaa948720
      xor eax,eax
      mov BYTE PTR [r11+0x1],0x1
      add rsp,0x28
      ret
    
    Which is pretty typical code for an EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE
    handler. The "mov r11, QWORD PTR [rip+0x2424]" was the faulting
    instruction because ConvertPointer() was being called to convert the
    address 0x0000000000000000, which when converted is left unchanged and
    remains 0x0000000000000000.
    
    The output of the oops trace gave the impression of a standard NULL
    pointer dereference bug, but because we're accessing physical
    addresses during ConvertPointer(), it wasn't. EFI boot services code
    is stored at that address on Alexis' machine.
    
    Reported-by: default avatarAlexis Murzeau <amurzeau@gmail.com>
    Signed-off-by: default avatarMatt Fleming <matt@codeblueprint.co.uk>
    Cc: Andy Lutomirski <luto@amacapital.net>
    Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
    Cc: Ben Hutchings <ben@decadent.org.uk>
    Cc: Borislav Petkov <bp@alien8.de>
    Cc: Brian Gerst <brgerst@gmail.com>
    Cc: Denys Vlasenko <dvlasenk@redhat.com>
    Cc: H. Peter Anvin <hpa@zytor.com>
    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    Cc: Maarten Lankhorst <maarten.lankhorst@canonical.com>
    Cc: Matthew Garrett <mjg59@srcf.ucam.org>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Raphael Hertzog <hertzog@debian.org>
    Cc: Roger Shimizu <rogershimizu@gmail.com>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    Cc: linux-efi@vger.kernel.org
    Link: http://lkml.kernel.org/r/1457695163-29632-2-git-send-email-matt@codeblueprint.co.uk
    Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=815125
    
    
    Signed-off-by: default avatarIngo Molnar <mingo@kernel.org>
    452308de