linux-kernel - Re: [tip:core/urgent] x86/efi: Fix boot crash by mapping EFI memmap entries bottom-up at runtime, instead of top-down

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] [day] [month] [year] [list]

Message-ID: <20151002094405.GA2846@codeblueprint.co.uk>
Date:	Fri, 2 Oct 2015 10:44:05 +0100
From:	Matt Fleming <matt@...eblueprint.co.uk>
To:	tglx@...utronix.de, hpa@...or.com, leif.lindholm@...aro.org,
	ard.biesheuvel@...aro.org, jlee@...e.com, stable@...r.kernel.org,
	mjg59@...f.ucam.org, linux-kernel@...r.kernel.org,
	mingo@...nel.org, torvalds@...ux-foundation.org, bp@...e.de,
	dyoung@...hat.com, matt.fleming@...el.com, pjones@...hat.com,
	JBottomley@...n.com, peterz@...radead.org, efault@....de
Cc:	linux-tip-commits@...r.kernel.org
Subject: Re: [tip:core/urgent] x86/efi: Fix boot crash by mapping EFI memmap
 entries bottom-up at runtime, instead of top-down

On Thu, 01 Oct, at 05:48:43AM, tip-bot for Matt Fleming wrote:
> Commit-ID:  a5caa209ba9c29c6421292e7879d2387a2ef39c9
> Gitweb:     http://git.kernel.org/tip/a5caa209ba9c29c6421292e7879d2387a2ef39c9
> Author:     Matt Fleming <matt.fleming@...el.com>
> AuthorDate: Fri, 25 Sep 2015 23:02:18 +0100
> Committer:  Ingo Molnar <mingo@...nel.org>
> CommitDate: Thu, 1 Oct 2015 12:51:28 +0200
> 
> x86/efi: Fix boot crash by mapping EFI memmap entries bottom-up at runtime, instead of top-down
> 
> 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 like this may occur,
> 
>   BUG: unable to handle kernel paging request at fffffffefe6086dd
>   IP: [<fffffffefe6086dd>] 0xfffffffefe6086dd
>   Call Trace:
>    [<ffffffff8104c90e>] efi_call+0x7e/0x100
>    [<ffffffff81602091>] ? virt_efi_set_variable+0x61/0x90
>    [<ffffffff8104c583>] efi_delete_dummy_variable+0x63/0x70
>    [<ffffffff81f4e4aa>] efi_enter_virtual_mode+0x383/0x392
>    [<ffffffff81f37e1b>] start_kernel+0x38a/0x417
>    [<ffffffff81f37495>] x86_64_start_reservations+0x2a/0x2c
>    [<ffffffff81f37582>] x86_64_start_kernel+0xeb/0xef
> 
> Here 0xfffffffefe6086dd refers to an address the firmware
> expects to be mapped but which the OS never claimed was mapped.
> The issue is that included in these regions are relative
> addresses to other regions which were emitted by the firmware
> toolchain before the "splitting" of sections occurred at
> runtime.
> 
> Needless to say, we don't satisfy this unwritten requirement on
> x86_64 and instead map the EFI memory map entries in reverse
> order. The above crash is almost certainly triggerable with any
> kernel newer than v3.13 because that's when we rewrote the EFI
> runtime region mapping code, in commit d2f7cbe7b26a ("x86/efi:
> Runtime services virtual mapping"). For kernel versions before
> v3.13 things may work by pure luck depending on the
> fragmentation of the kernel virtual address space at the time we
> map the EFI regions.
> 
> Instead of mapping the EFI memory map entries in reverse order,
> where entry N has a higher virtual address than entry N+1, map
> them in the same order as they appear in the EFI memory map to
> preserve this relative offset between regions.
> 
> This patch has been kept as small as possible with the intention
> that it should be applied aggressively to stable and
> distribution kernels. It is very much a bugfix rather than
> support for a new feature, since when EFI_PROPERTIES_TABLE is
> enabled we must map things as outlined above to even boot - we
> have no way of asking the firmware not to split the code/data
> regions.
> 
> In fact, this patch doesn't even make use of the more strict
> memory protections available in UEFI v2.5. That will come later.
> 
> Suggested-by: Ard Biesheuvel <ard.biesheuvel@...aro.org>
> Reported-by: Ard Biesheuvel <ard.biesheuvel@...aro.org>
> Signed-off-by: Matt Fleming <matt.fleming@...el.com>
> Cc: <stable@...r.kernel.org>
> Cc: Borislav Petkov <bp@...e.de>
> Cc: Chun-Yi <jlee@...e.com>
> Cc: Dave Young <dyoung@...hat.com>
> Cc: H. Peter Anvin <hpa@...or.com>
> Cc: James Bottomley <JBottomley@...n.com>
> Cc: Lee, Chun-Yi <jlee@...e.com>
> Cc: Leif Lindholm <leif.lindholm@...aro.org>
> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> Cc: Matthew Garrett <mjg59@...f.ucam.org>
> Cc: Mike Galbraith <efault@....de>
> Cc: Peter Jones <pjones@...hat.com>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: linux-kernel@...r.kernel.org
> Link: http://lkml.kernel.org/r/1443218539-7610-2-git-send-email-matt@codeblueprint.co.uk
> Signed-off-by: Ingo Molnar <mingo@...nel.org>

It's probably a little late to change this now, but I just realised
that I dropped Joey's Tested-by tags from this patch. Sorry Joey!

-- 
Matt Fleming, Intel Open Source Technology Center
--
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/