linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ardb@kernel.org>
To: linux-efi@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>
Cc: Ard Biesheuvel <ardb@kernel.org>,
	linux-kernel@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>,
	Arvind Sankar <nivedita@alum.mit.edu>,
	Borislav Petkov <bp@suse.de>,
	Colin Ian King <colin.king@canonical.com>,
	Gary Lin <glin@suse.com>, Jiri Slaby <jslaby@suse.cz>,
	Sergey Shatunov <me@prok.pw>, Takashi Iwai <tiwai@suse.de>
Subject: [PATCH 4/9] efi/x86: Always relocate the kernel for EFI handover entry
Date: Thu,  9 Apr 2020 15:04:29 +0200	[thread overview]
Message-ID: <20200409130434.6736-5-ardb@kernel.org> (raw)
In-Reply-To: <20200409130434.6736-1-ardb@kernel.org>

From: Arvind Sankar <nivedita@alum.mit.edu>

Commit

  d5cdf4cfeac9 ("efi/x86: Don't relocate the kernel unless necessary")

tries to avoid relocating the kernel in the EFI stub as far as possible.

However, when systemd-boot is used to boot a unified kernel image [1],
the image is constructed by embedding the bzImage as a .linux section in
a PE executable that contains a small stub loader from systemd that will
call the EFI stub handover entry, together with additional sections and
potentially an initrd. When this image is constructed, by for example
dracut, the initrd is placed after the bzImage without ensuring that at
least init_size bytes are available for the bzImage. If the kernel is
not relocated by the EFI stub, this could result in the compressed
kernel's startup code in head_{32,64}.S overwriting the initrd.

To prevent this, unconditionally relocate the kernel if the EFI stub was
entered via the handover entry point.

[1] https://systemd.io/BOOT_LOADER_SPECIFICATION/#type-2-efi-unified-kernel-images

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
Reported-by: Sergey Shatunov <me@prok.pw>
Fixes: d5cdf4cfeac9 ("efi/x86: Don't relocate the kernel unless necessary")
Link: https://lore.kernel.org/r/20200406180614.429454-2-nivedita@alum.mit.edu
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/x86-stub.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 867a57e28980..05ccb229fb45 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -740,8 +740,15 @@ unsigned long efi_main(efi_handle_t handle,
 	 * now use KERNEL_IMAGE_SIZE, which will be 512MiB, the same as what
 	 * KASLR uses.
 	 *
-	 * Also relocate it if image_offset is zero, i.e. we weren't loaded by
-	 * LoadImage, but we are not aligned correctly.
+	 * Also relocate it if image_offset is zero, i.e. the kernel wasn't
+	 * loaded by LoadImage, but rather by a bootloader that called the
+	 * handover entry. The reason we must always relocate in this case is
+	 * to handle the case of systemd-boot booting a unified kernel image,
+	 * which is a PE executable that contains the bzImage and an initrd as
+	 * COFF sections. The initrd section is placed after the bzImage
+	 * without ensuring that there are at least init_size bytes available
+	 * for the bzImage, and thus the compressed kernel's startup code may
+	 * overwrite the initrd unless it is moved out of the way.
 	 */
 
 	buffer_start = ALIGN(bzimage_addr - image_offset,
@@ -751,8 +758,7 @@ unsigned long efi_main(efi_handle_t handle,
 	if ((buffer_start < LOAD_PHYSICAL_ADDR)				     ||
 	    (IS_ENABLED(CONFIG_X86_32) && buffer_end > KERNEL_IMAGE_SIZE)    ||
 	    (IS_ENABLED(CONFIG_X86_64) && buffer_end > MAXMEM_X86_64_4LEVEL) ||
-	    (image_offset == 0 && !IS_ALIGNED(bzimage_addr,
-					      hdr->kernel_alignment))) {
+	    (image_offset == 0)) {
 		status = efi_relocate_kernel(&bzimage_addr,
 					     hdr->init_size, hdr->init_size,
 					     hdr->pref_address,
-- 
2.17.1


  parent reply	other threads:[~2020-04-09 13:05 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-09 13:04 [GIT PULL 0/9] EFI fixes for v5.7-rc Ard Biesheuvel
2020-04-09 13:04 ` [PATCH 1/9] efi/cper: Use scnprintf() for avoiding potential buffer overflow Ard Biesheuvel
2020-04-09 13:04 ` [PATCH 2/9] efi/libstub/x86: remove redundant assignment to pointer hdr Ard Biesheuvel
2020-04-09 13:04 ` [PATCH 3/9] efi/x86: Move efi stub globals from .bss to .data Ard Biesheuvel
2020-04-09 20:05   ` Brian Gerst
2020-04-09 20:53     ` Brian Gerst
2020-04-09 21:08       ` Arvind Sankar
2020-04-10  8:20         ` Ard Biesheuvel
2020-04-10 15:16           ` Arvind Sankar
2020-04-10 16:03             ` Ard Biesheuvel
2020-04-10 18:01               ` Arvind Sankar
2020-04-10 18:03                 ` Ard Biesheuvel
2020-04-10 19:03                   ` Arvind Sankar
2020-04-11  1:03                   ` Arvind Sankar
2020-04-09 13:04 ` Ard Biesheuvel [this message]
2020-04-09 13:04 ` [PATCH 5/9] efi/arm: Deal with ADR going out of range in efi_enter_kernel() Ard Biesheuvel
2020-04-14  8:20   ` [tip: efi/urgent] " tip-bot2 for Ard Biesheuvel
2020-04-09 13:04 ` [PATCH 6/9] Documentation: efi/x86: clarify EFI handover protocol and its requirements Ard Biesheuvel
2020-04-14  8:20   ` [tip: efi/urgent] Documentation/x86, efi/x86: Clarify " tip-bot2 for Ard Biesheuvel
2020-04-09 13:04 ` [PATCH 7/9] efi/libstub/file: merge filename buffers to reduce stack usage Ard Biesheuvel
2020-04-14  8:20   ` [tip: efi/urgent] efi/libstub/file: Merge file name " tip-bot2 for Ard Biesheuvel
2020-04-09 13:04 ` [PATCH 8/9] efi/x86: Fix the deletion of variables in mixed mode Ard Biesheuvel
2020-04-09 13:04 ` [PATCH 9/9] efi/x86: Don't remap text<->rodata gap read-only for " Ard Biesheuvel
2020-04-14  8:20   ` [tip: efi/urgent] " tip-bot2 for Ard Biesheuvel
2020-04-09 19:01 ` [GIT PULL 0/9] EFI fixes for v5.7-rc Theodore Y. Ts'o
2020-04-09 19:04   ` Ard Biesheuvel
2020-04-09 20:16     ` Theodore Y. Ts'o
2020-04-09 21:29       ` Ard Biesheuvel
2020-04-09 23:57         ` Theodore Y. Ts'o
2020-04-10  7:08           ` Ard Biesheuvel
2020-04-10 13:54             ` Dave Young
2020-04-11 19:43               ` Theodore Y. Ts'o
2020-04-12  3:51                 ` Dave Young
2020-04-13 14:07 ` David Howells

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=20200409130434.6736-5-ardb@kernel.org \
    --to=ardb@kernel.org \
    --cc=arnd@arndb.de \
    --cc=bp@suse.de \
    --cc=colin.king@canonical.com \
    --cc=glin@suse.com \
    --cc=jslaby@suse.cz \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=me@prok.pw \
    --cc=mingo@kernel.org \
    --cc=nivedita@alum.mit.edu \
    --cc=tglx@linutronix.de \
    --cc=tiwai@suse.de \
    /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;
as well as URLs for NNTP newsgroup(s).