From: Demi Marie Obenour <demi@invisiblethingslab.com>
To: Ard Biesheuvel <ardb@kernel.org>, linux-efi@vger.kernel.org
Cc: xen-devel@lists.xenproject.org, "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: Re: [PATCH v2 6/6] efi: Apply allowlist to EFI configuration tables when running under Xen
Date: Tue, 6 Dec 2022 18:19:15 -0500 [thread overview]
Message-ID: <Y4/N9N8CrNmZYb/M@itl-email> (raw)
In-Reply-To: <20221003112625.972646-7-ardb@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 3723 bytes --]
On Mon, Oct 03, 2022 at 01:26:25PM +0200, Ard Biesheuvel wrote:
> 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:
Some firmware uses EFI_ACPI_MEMORY_NVS to store ACPI tables, so this
needs to be added to the allowlist. Otherwise affected systems will not
boot. Xen treats EFI_ACPI_MEMORY_NVS the way it treats
EFI_ACPI_RECLAIM_MEMORY, so this is safe.
> + 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
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2022-12-06 23:19 UTC|newest]
Thread overview: 27+ 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: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 ` [PATCH v2 6/6] efi: Apply allowlist to EFI configuration tables when running under Xen Ard Biesheuvel
2022-12-06 23:19 ` Demi Marie Obenour [this message]
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=Y4/N9N8CrNmZYb/M@itl-email \
--to=demi@invisiblethingslab.com \
--cc=anton@enomsg.org \
--cc=ardb@kernel.org \
--cc=ccross@android.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 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).