public inbox for linux-efi@vger.kernel.org
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ardb@kernel.org>
To: linux-efi@vger.kernel.org
Cc: Ard Biesheuvel <ardb@kernel.org>,
	tglx@linutronix.de, torvalds@linux-foundation.org,
	Thadeu Lima de Souza Cascardo <cascardo@canonical.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Borislav Petkov <bp@suse.de>,
	Josh Poimboeuf <jpoimboe@kernel.org>
Subject: [PATCH] efi/x86-mixed: leave RET unmitigated but move it into .rodata
Date: Fri, 22 Jul 2022 18:06:12 +0200	[thread overview]
Message-ID: <20220722160612.2976-1-ardb@kernel.org> (raw)

Thadeu reports that the retbleed mitigations have broken EFI runtime
services in mixed mode, as the RET macro now expands to a relative
branch that jumps to nowhere when executed from the 1:1 mapping of the
kernel text that the EFI mixed mode thunk uses on its return back to the
caller.

So as Thadeu suggested in [1], we should switch to a bare 'ret' opcode
followed by 'int3' (to limit straight line speculation). However, doing
so leaves an unmitigated RET in the kernel text that is always present,
even on non-EFI or non-mixed mode systems (which are quite rare these
days to begin with)

So let's take Thadeu's fix a bit further, and move the EFI mixed mode
return trampoline that contains the RET into .rodata, so it is normally
mapped without executable permissions. And given that this snippet of
code is really the only kernel code that we ever execute via this 1:1
mapping, let's make the 1:1 mapping of the kernel .text non-executable
as well, and only map the page that covers the return trampoline with
executable permissions.

Note that mapping .text and .rodata is still necessary, as otherwise,
they will be covered by the default 1:1 mapping of the RAM below 4 GB,
which uses read-write permissions. Also note that merging the mappings
of .text and .rodata is not possible, even if they now use the same
permissions, due to the fact that the hole in the middle may contain
read-write data (such as the mixed mode stack)

[1] https://lore.kernel.org/linux-efi/20220715194550.793957-1-cascardo@canonical.com/

Cc: tglx@linutronix.de
Cc: torvalds@linux-foundation.org
Cc: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/platform/efi/efi_64.c       | 15 ++++++++++++---
 arch/x86/platform/efi/efi_thunk_64.S |  9 +++++++--
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index 1f3675453a57..d8661fb31c76 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -176,7 +176,8 @@ virt_to_phys_or_null_size(void *va, unsigned long size)
 
 int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 {
-	unsigned long pfn, text, pf, rodata;
+	extern const u8 __efi64_thunk_ret_tramp[];
+	unsigned long pfn, text, pf, rodata, tramp;
 	struct page *page;
 	unsigned npages;
 	pgd_t *pgd = efi_mm.pgd;
@@ -240,7 +241,7 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 	text = __pa(_text);
 	pfn = text >> PAGE_SHIFT;
 
-	pf = _PAGE_ENC;
+	pf = _PAGE_NX | _PAGE_ENC;
 	if (kernel_map_pages_in_pgd(pgd, pfn, text, npages, pf)) {
 		pr_err("Failed to map kernel text 1:1\n");
 		return 1;
@@ -250,12 +251,20 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 	rodata = __pa(__start_rodata);
 	pfn = rodata >> PAGE_SHIFT;
 
-	pf = _PAGE_NX | _PAGE_ENC;
 	if (kernel_map_pages_in_pgd(pgd, pfn, rodata, npages, pf)) {
 		pr_err("Failed to map kernel rodata 1:1\n");
 		return 1;
 	}
 
+	tramp = __pa(__efi64_thunk_ret_tramp);
+	pfn = tramp >> PAGE_SHIFT;
+
+	pf = _PAGE_ENC;
+	if (kernel_map_pages_in_pgd(pgd, pfn, tramp, 1, pf)) {
+		pr_err("Failed to map kernel rodata 1:1\n");
+		return 1;
+	}
+
 	return 0;
 }
 
diff --git a/arch/x86/platform/efi/efi_thunk_64.S b/arch/x86/platform/efi/efi_thunk_64.S
index 9ffe2bad27d5..e436ce03741e 100644
--- a/arch/x86/platform/efi/efi_thunk_64.S
+++ b/arch/x86/platform/efi/efi_thunk_64.S
@@ -71,17 +71,22 @@ STACK_FRAME_NON_STANDARD __efi64_thunk
 	pushq	$__KERNEL32_CS
 	pushq	%rdi			/* EFI runtime service address */
 	lretq
+SYM_FUNC_END(__efi64_thunk)
 
+	.section ".rodata", "a", @progbits
+	.balign	16
+SYM_DATA_START(__efi64_thunk_ret_tramp)
 1:	movq	0x20(%rsp), %rsp
 	pop	%rbx
 	pop	%rbp
-	RET
+	ret
+	int3
 
 	.code32
 2:	pushl	$__KERNEL_CS
 	pushl	%ebp
 	lret
-SYM_FUNC_END(__efi64_thunk)
+SYM_DATA_END(__efi64_thunk_ret_tramp)
 
 	.bss
 	.balign 8
-- 
2.35.1


             reply	other threads:[~2022-07-22 16:07 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-22 16:06 Ard Biesheuvel [this message]
2022-07-23 17:20 ` [PATCH] efi/x86-mixed: leave RET unmitigated but move it into .rodata Linus Torvalds
2022-07-24  8:39   ` Ard Biesheuvel
2022-07-24 17:27     ` Borislav Petkov
2022-07-24 18:34       ` Ard Biesheuvel
2022-07-24 19:17         ` Borislav Petkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220722160612.2976-1-ardb@kernel.org \
    --to=ardb@kernel.org \
    --cc=bp@suse.de \
    --cc=cascardo@canonical.com \
    --cc=jpoimboe@kernel.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox