All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ardb@kernel.org>
To: linux-efi@vger.kernel.org
Cc: xen-devel@lists.xenproject.org,
	"Ard Biesheuvel" <ardb@kernel.org>,
	"Demi Marie Obenour" <demi@invisiblethingslab.com>,
	"Peter Jones" <pjones@redhat.com>,
	"Juergen Gross" <jgross@suse.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Oleksandr Tyshchenko" <oleksandr_tyshchenko@epam.com>,
	"Kees Cook" <keescook@chromium.org>,
	"Anton Vorontsov" <anton@enomsg.org>,
	"Colin Cross" <ccross@android.com>,
	"Tony Luck" <tony.luck@intel.com>,
	"Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
Subject: [PATCH v2 6/6] efi: Apply allowlist to EFI configuration tables when running under Xen
Date: Mon,  3 Oct 2022 13:26:25 +0200	[thread overview]
Message-ID: <20221003112625.972646-7-ardb@kernel.org> (raw)
In-Reply-To: <20221003112625.972646-1-ardb@kernel.org>

As it turns out, Xen does not guarantee that EFI bootservices data
regions in memory are preserved, which means that EFI configuration
tables pointing into such memory regions may be corrupted before the
dom0 OS has had a chance to inspect them.

Demi Marie reports that this is causing problems for Qubes OS when it
attempts to perform system firmware updates, which requires that the
contents of the ESRT configuration table are valid when the fwupd user
space program runs.

However, other configuration tables such as the memory attributes
table or the runtime properties table are equally affected, and so we
need a comprehensive workaround that works for any table type.

So when running under Xen, check the EFI memory descriptor covering the
start of the table, and disregard the table if it does not reside in
memory that is preserved by Xen.

Co-developed-by: Demi Marie Obenour <demi@invisiblethingslab.com>
Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/efi.c |  7 ++++++
 drivers/xen/efi.c          | 24 ++++++++++++++++++++
 include/linux/efi.h        |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 2c12b1a06481..0a4583c13a40 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -560,6 +560,13 @@ static __init int match_config_table(const efi_guid_t *guid,
 
 	for (i = 0; efi_guidcmp(table_types[i].guid, NULL_GUID); i++) {
 		if (!efi_guidcmp(*guid, table_types[i].guid)) {
+			if (IS_ENABLED(CONFIG_XEN_EFI) &&
+			    !xen_efi_config_table_is_usable(guid, table)) {
+				if (table_types[i].name[0])
+					pr_cont("(%s=0x%lx) ",
+						table_types[i].name, table);
+				return 1;
+			}
 			*(table_types[i].ptr) = table;
 			if (table_types[i].name[0])
 				pr_cont("%s=0x%lx ",
diff --git a/drivers/xen/efi.c b/drivers/xen/efi.c
index 74f3f6d8cdc8..c275a9c377fe 100644
--- a/drivers/xen/efi.c
+++ b/drivers/xen/efi.c
@@ -326,3 +326,27 @@ int efi_mem_desc_lookup(u64 phys_addr, efi_memory_desc_t *out_md)
 
         return 0;
 }
+
+bool __init xen_efi_config_table_is_usable(const efi_guid_t *guid,
+					   unsigned long table)
+{
+	efi_memory_desc_t md;
+	int rc;
+
+	if (!efi_enabled(EFI_PARAVIRT))
+		return true;
+
+	rc = efi_mem_desc_lookup(table, &md);
+	if (rc)
+		return false;
+
+	switch (md.type) {
+	case EFI_RUNTIME_SERVICES_CODE:
+	case EFI_RUNTIME_SERVICES_DATA:
+	case EFI_ACPI_RECLAIM_MEMORY:
+	case EFI_RESERVED_TYPE:
+		return true;
+	}
+
+	return false;
+}
diff --git a/include/linux/efi.h b/include/linux/efi.h
index e0ee6f6da4b4..b0cba86352ce 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1352,4 +1352,6 @@ struct linux_efi_initrd {
 /* Header of a populated EFI secret area */
 #define EFI_SECRET_TABLE_HEADER_GUID	EFI_GUID(0x1e74f542, 0x71dd, 0x4d66,  0x96, 0x3e, 0xef, 0x42, 0x87, 0xff, 0x17, 0x3b)
 
+bool xen_efi_config_table_is_usable(const efi_guid_t *, unsigned long table);
+
 #endif /* _LINUX_EFI_H */
-- 
2.35.1


  parent reply	other threads:[~2022-10-03 11:26 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-03 11:26 [PATCH v2 0/6] efi/x86: Avoid corrupted config tables under Xen Ard Biesheuvel
2022-10-03 11:26 ` [PATCH v2 1/6] efi: Move EFI fake memmap support into x86 arch tree Ard Biesheuvel
2022-10-03 11:26 ` [PATCH v2 2/6] efi: memmap: Move manipulation routines " Ard Biesheuvel
2022-10-03 11:26 ` [PATCH v2 3/6] efi: xen: Set EFI_PARAVIRT for Xen dom0 boot on all architectures Ard Biesheuvel
2022-10-03 11:26 ` [PATCH v2 4/6] efi: memmap: Disregard bogus entries instead of returning them Ard Biesheuvel
2022-10-03 15:18   ` Demi Marie Obenour
2022-10-03 15:57     ` Ard Biesheuvel
2022-10-03 11:26 ` [PATCH v2 5/6] efi: xen: Implement memory descriptor lookup based on hypercall Ard Biesheuvel
2022-10-03 15:29   ` Demi Marie Obenour
2022-10-03 15:59     ` Ard Biesheuvel
2022-10-03 16:04       ` Marek Marczykowski-Górecki
2022-10-03 16:22       ` Demi Marie Obenour
2022-10-03 16:37         ` Ard Biesheuvel
2022-10-03 17:04           ` Marek Marczykowski-Górecki
2022-10-03 17:04             ` Marek Marczykowski-Górecki
2022-10-03 17:57             ` Demi Marie Obenour
2022-10-03 18:01               ` Marek Marczykowski-Górecki
2023-01-15 13:31                 ` Marek Marczykowski-Górecki
2022-11-19  1:10             ` Demi Marie Obenour
2022-10-03 11:26 ` Ard Biesheuvel [this message]
2022-12-06 23:19   ` [PATCH v2 6/6] efi: Apply allowlist to EFI configuration tables when running under Xen Demi Marie Obenour
2023-01-19 19:03 ` [PATCH v3 0/5] efi: Support ESRT " Demi Marie Obenour
2023-01-19 19:03   ` [PATCH v3 1/5] efi: memmap: Disregard bogus entries instead of returning them Demi Marie Obenour
2023-01-19 19:03   ` [PATCH v3 2/5] efi: xen: Implement memory descriptor lookup based on hypercall Demi Marie Obenour
2023-01-19 19:03   ` [PATCH v3 3/5] efi: Apply allowlist to EFI configuration tables when running under Xen Demi Marie Obenour
2023-01-19 19:03   ` [PATCH v3 4/5] efi: Actually enable the ESRT " Demi Marie Obenour
2023-01-19 19:04   ` [PATCH v3 5/5] efi: Warn if trying to reserve memory " Demi Marie Obenour
2023-01-23  7:30   ` [PATCH v3 0/5] efi: Support ESRT " 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=20221003112625.972646-7-ardb@kernel.org \
    --to=ardb@kernel.org \
    --cc=anton@enomsg.org \
    --cc=ccross@android.com \
    --cc=demi@invisiblethingslab.com \
    --cc=jgross@suse.com \
    --cc=keescook@chromium.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=marmarek@invisiblethingslab.com \
    --cc=oleksandr_tyshchenko@epam.com \
    --cc=pjones@redhat.com \
    --cc=sstabellini@kernel.org \
    --cc=tony.luck@intel.com \
    --cc=xen-devel@lists.xenproject.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 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.