mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ardb@kernel.org>
To: linux-efi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Ard Biesheuvel <ardb@kernel.org>,
	Evgeniy Baskov <baskov@ispras.ru>, Borislav Petkov <bp@alien8.de>,
	Andy Lutomirski <luto@kernel.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Alexey Khoroshilov <khoroshilov@ispras.ru>,
	Peter Jones <pjones@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>, Dave Young <dyoung@redhat.com>,
	Mario Limonciello <mario.limonciello@amd.com>,
	Kees Cook <keescook@chromium.org>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	"Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: [RFC PATCH 3/3] efi/zboot: x86: Clear NX restrictions on populated code regions
Date: Sun, 16 Apr 2023 14:07:29 +0200	[thread overview]
Message-ID: <20230416120729.2470762-4-ardb@kernel.org> (raw)
In-Reply-To: <20230416120729.2470762-1-ardb@kernel.org>

Future EFI firmware will require the PE/COFF NX_COMPAT header flag to be
set in order to retain access to all system facilities while features
such as UEFI secure boot or TCG measured boot are enabled.

The consequence of setting this flag is that the EFI firmware image
loader may configure the page allocator to set the NX attribute on all
allocations requested by the image. This means we should clear this
attribute on all regions we allocate and expect to be able to execute
from.

In the x86 EFI zboot case, the only code we execute under EFI's 1:1
mapping that was not loaded by the image loader itself is the trampoline
that effectuates the switch between 4 and 5 level paging, and the part
of the loaded kernel image that runs before switching to its own page
tables.  So let's use the EFI memory attributes protocol to clear the NX
attribute on these regions.

Whether or not setting the read-only attribute first is required is
unclear at this point. Given that the kernel startup code uses two
different executable sections before switching to its own page tables
(normal text and inittext, with a writable data section in between),
this would require some minor reorganization of the kernel memory map.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/kernel/head_64.S                |  4 +++
 drivers/firmware/efi/libstub/x86-zboot.c | 27 ++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 4ae067852fb28663..38897ac51f13bb55 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -74,6 +74,10 @@ SYM_CODE_START_NOALIGN(startup_64)
 	 */
 	.org	startup_64 + 0x10 - 3, BYTES_NOP1
 	nopl	(_end - startup_64)(%rax)
+
+	/* put the size of the initial executable mapping at offset 0x20 */
+	.org	startup_64 + 0x20 - 3, BYTES_NOP1
+	nopl	(_einittext - startup_64)(%rax)
 #endif
 	leaq	_text(%rip), %rdi
 
diff --git a/drivers/firmware/efi/libstub/x86-zboot.c b/drivers/firmware/efi/libstub/x86-zboot.c
index 16e8b315892dedda..70668104804fb050 100644
--- a/drivers/firmware/efi/libstub/x86-zboot.c
+++ b/drivers/firmware/efi/libstub/x86-zboot.c
@@ -60,10 +60,33 @@ efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char **cmdline_ptr)
 	return status;
 }
 
+static void efi_remap_exec(unsigned long base, unsigned long size)
+{
+	static efi_memory_attribute_protocol_t *memattr = (void *)ULONG_MAX;
+	efi_guid_t guid = EFI_MEMORY_ATTRIBUTE_PROTOCOL_GUID;
+	efi_status_t status;
+
+	if (memattr == (void *)ULONG_MAX) {
+		memattr = NULL;
+		status = efi_bs_call(locate_protocol, &guid, NULL,
+				     (void **)&memattr);
+		if (status != EFI_SUCCESS)
+			return;
+	} else if (!memattr) {
+		return;
+	}
+
+	status = memattr->clear_memory_attributes(memattr, base, size,
+						  EFI_MEMORY_XP);
+	if (status != EFI_SUCCESS)
+		efi_warn("Failed to clear NX attribute on code region\n");
+}
+
 void efi_cache_sync_image(unsigned long image_base, unsigned long alloc_size)
 {
 	const u32 payload_size = *(u32 *)(_gzdata_end - 4);
 	const u32 image_size = *(u32 *)(image_base + 0x10);
+	const u32 code_size = *(u32 *)(image_base + 0x20);
 	const s32 *reloc = (s32 *)(image_base + payload_size);
 	u64 va_offset = __START_KERNEL - image_base;
 	u64 range, delta;
@@ -107,6 +130,8 @@ void efi_cache_sync_image(unsigned long image_base, unsigned long alloc_size)
 		*(u64 *)((s64)*reloc - va_offset) += delta;
 
 	efi_free(alloc_size - image_size, image_base + image_size);
+
+	efi_remap_exec(image_base, PAGE_ALIGN(code_size));
 }
 
 static void __naked tmpl_toggle(void *cr3, void *gdt)
@@ -197,6 +222,8 @@ static efi_status_t efi_setup_5level_paging(void)
 	 */
 	*(u32 *)&la57_code[tmpl_size - 6] += (u64)la57_code;
 
+	efi_remap_exec((unsigned long)la57_code, PAGE_SIZE);
+
 	return EFI_SUCCESS;
 }
 
-- 
2.39.2


  parent reply	other threads:[~2023-04-16 12:08 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-16 12:07 [RFC PATCH 0/3] efi: Implement generic zboot support Ard Biesheuvel
2023-04-16 12:07 ` [RFC PATCH 1/3] efi/libstub: x86: Split off pieces shared with zboot Ard Biesheuvel
2023-04-16 12:07 ` [RFC PATCH 2/3] efi/zboot: x86: Implement EFI zboot support Ard Biesheuvel
2023-04-16 12:07 ` Ard Biesheuvel [this message]
2023-04-18 14:10 ` [RFC PATCH 0/3] efi: Implement generic " Evgeniy Baskov
2023-04-19  2:56 ` Dave Young
2023-04-19  5:54 ` Gerd Hoffmann
2023-04-19 14:44   ` Ard Biesheuvel
2023-04-20  6:07     ` Gerd Hoffmann
2023-04-20  7:54       ` Ard Biesheuvel
2023-04-20 12:29         ` Mario Limonciello
2023-04-21 13:29 ` Andy Lutomirski
2023-04-21 13:41   ` Ard Biesheuvel
2023-05-03 17:55     ` Andy Lutomirski
2023-05-03 18:13       ` Ard Biesheuvel

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=20230416120729.2470762-4-ardb@kernel.org \
    --to=ardb@kernel.org \
    --cc=baskov@ispras.ru \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=dyoung@redhat.com \
    --cc=keescook@chromium.org \
    --cc=khoroshilov@ispras.ru \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=kraxel@redhat.com \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mario.limonciello@amd.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=pjones@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=thomas.lendacky@amd.com \
    --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

all inboxes | Powered by JetHome®