All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ardb@kernel.org>
To: linux-efi@vger.kernel.org
Cc: hdegoede@redhat.com, matthewgarrett@google.com,
	Ard Biesheuvel <ardb@kernel.org>
Subject: [PATCH] efi/libstub: disable file loading and page deallocation in mixed mode
Date: Fri, 13 Dec 2019 10:11:15 +0100	[thread overview]
Message-ID: <20191213091115.567-1-ardb@kernel.org> (raw)

EFI mixed mode is a nice hack, since it allows us to run 64-bit Linux
on low end x86_64 machines that shipped with 32-bit UEFI as they were
built to run 32-bit Windows only.

Mixed mode relies on the ability to convert calls made using the
64-bit calling convention into calls using the 32-bit one. This
involves pushing a 32-bit word onto the stack for each argument
passed in a 64-bit register, relying on the fact that all quantities
that are the native size or smaller (including pointers) can be safely
truncated to 32 bits. (In the pointer case, we rely on the fact that
we are still executing in the firmware context, which uses a 1:1
mapping that can only access the lower 4 GB of the address space)

For types that are explicitly 64 bits wide, such as EFI_PHYSICAL_ADDRESS
or UINT64, this assumption doesn't hold. The correct way to marshall
such a call would be to push two consecutive 32-bit words onto the
stack, but given that the naive thunking code has no knowledge
whatsoever of the prototype of the function it is invoking, all we can
do is avoid calling such functions altogether.

The FreePages() boot service is affected by this, so we should not call
that at all in mixed mode. In practice, this doesn't change much, since
in the past, these calls would have been made with a bogus address, and
so we were leaking this memory already. Note that the scope of this leak
is the EFI execution context only, so it makes no difference for Linux.

The other piece of functionality that we need to disable is loading files
passed via file=xxxx on the command line, given that the Open() method
takes two UINT64s as well.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 .../firmware/efi/libstub/efi-stub-helper.c    | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 0f3dbfed6306..f1f316e96819 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -353,7 +353,12 @@ void efi_free(efi_system_table_t *sys_table_arg, unsigned long size,
 {
 	unsigned long nr_pages;
 
-	if (!size)
+	/*
+	 * Mixed mode does not support calling firmware routines that take
+	 * explicit 64-bit wide arguments. So all we can do is leak the
+	 * allocation.
+	 */
+	if (!size || (IS_ENABLED(CONFIG_EFI_MIXED) && !efi_is_64bit()))
 		return;
 
 	nr_pages = round_up(size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
@@ -536,6 +541,18 @@ efi_status_t handle_cmdline_files(efi_system_table_t *sys_table_arg,
 	char *str;
 	int i, j, k;
 
+	/*
+	 * Using firmware services to load files is not supported in mixed mode
+	 * systems, because it involves calling functions that have 64-bit wide
+	 * parameters in their prototypes, which are not marshalled correctly
+	 * by the thunking code.
+	 */
+	if (IS_ENABLED(CONFIG_EFI_MIXED) && !efi_is_64bit()) {
+		pr_efi(sys_table_arg,
+		       "Ignoring file= arguments on mixed mode system\n");
+		return EFI_SUCCESS;
+	}
+
 	file_addr = 0;
 	file_size_total = 0;
 
-- 
2.17.1


             reply	other threads:[~2019-12-13  9:11 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-13  9:11 Ard Biesheuvel [this message]
2019-12-13 12:29 ` [PATCH] efi/libstub: disable file loading and page deallocation in mixed mode Hans de Goede
2019-12-13 18:49   ` Ard Biesheuvel
2019-12-13 19:56     ` Hans de Goede
2019-12-13 20:08       ` Ard Biesheuvel
     [not found]         ` <f276df9f-83b4-e404-bcfc-91f0212a5fc0@redhat.com>
2019-12-13 20:16           ` Ard Biesheuvel
2019-12-14 15:21             ` Hans de Goede
2019-12-14 17:07               ` 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=20191213091115.567-1-ardb@kernel.org \
    --to=ardb@kernel.org \
    --cc=hdegoede@redhat.com \
    --cc=linux-efi@vger.kernel.org \
    --cc=matthewgarrett@google.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.