| Message ID | 20221003112625.972646-7-ardb@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | efi/x86: Avoid corrupted config tables under Xen | expand |
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
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 */