efi: get rid of EFI_SYSTEM_TABLES status bit
PATCH 1/2 efi: get rid of EFI_SYSTEM_TABLES status bit
The EFI_SYSTEM_TABLES status bit is set by all EFI supporting architectures upon discovery of the EFI system table, but the bit is never tested in any code we have in the tree. So remove it.
--- arch/ia64/kernel/efi.c | 2 -- arch/x86/platform/efi/efi.c | 2 -- drivers/firmware/efi/arm-runtime.c | 1 - include/linux/efi.h | 1 - 4 files changed, 6 deletions(-)
arch/ia64/kernel/efi.c +0 -2
@@ -531,8 +531,6 @@ efi_init (void) efi.systab->hdr.revision >>16 , efi.systab->hdr.revision & 0xffff, vendor); -set_bit (EFI_SYSTEM_TABLES , &efi.flags); - palo_phys =EFI_INVALID_TABLE_ADDR ;if (efi_config_init (arch_tables) !=0 )
arch/x86/platform/efi/efi.c +0 -2
@@ -352,8 +352,6 @@ static int __init efi_systab_init(void *phys) efi.systab->hdr.revision >>16 , efi.systab->hdr.revision & 0xffff); -set_bit (EFI_SYSTEM_TABLES , &efi.flags); -return 0 ; }
drivers/firmware/efi/arm-runtime.c +0 -1
@@ -105,7 +105,6 @@ static int __init arm_enable_runtime_services(void)pr_err ("Failed to remap EFI System Table\n" );return -ENOMEM ; } -set_bit (EFI_SYSTEM_TABLES , &efi.flags);if (!efi_virtmap_init ()) {pr_err ("No UEFI virtual mapping was installed -- runtime services will not be available\n" );
include/linux/efi.h +0 -1
@@ -975,7 +975,6 @@ extern int __init efi_setup_pcdp_console(char *);* possible, remove EFI-related code altogether. */ #define EFI_BOOT 0/* Were we booted from EFI? */ -#define EFI_SYSTEM_TABLES 1/* Can we use EFI system tables? */ #define EFI_CONFIG_TABLES 2/* Can we use EFI config tables? */ #define EFI_RUNTIME_SERVICES 3/* Can we use runtime services? */ #define EFI_MEMMAP 4/* Can we use EFI memory map? */
Signature · 1 line
2.5.0
PATCH 2/2 arm*: efi: drop writable mapping of the UEFI System table
Commit 2eec5dedf770 ("efi/arm-init: Use read-only early mappings") updated the early ARM UEFI init code to create the temporary, early mapping of the UEFI System table using read-only attributes, as a hardening measure against inadvertent modification.
However, this still leaves the permanent, writable mapping of the UEFI System table, which is only ever referenced during invocations of UEFI Runtime Services, at which time the UEFI virtual mapping is available, which also covers the system table. (This is guaranteed by the fact that SetVirtualAddressMap(), which is a runtime service itself, converts various entries in the table to their virtual equivalents, which implies that the table must be covered by a RuntimeServicesData region that has the EFI_MEMORY_RUNTIME attribute.)
So instead of creating this permanent mapping, record the virtual address of the system table inside the UEFI virtual mapping, and dereference that when accessing the table. This protects the contents of the system table from inadvertent (or deliberate) modification when no UEFI Runtime Services calls are in progress.
--- drivers/firmware/efi/arm-init.c | 2 ++ drivers/firmware/efi/arm-runtime.c | 27 ++++++++++++++++----------- 2 files changed, 18 insertions(+), 11 deletions(-)
drivers/firmware/efi/arm-init.c +2 -0
@@ -85,6 +85,8 @@ static int __init uefi_init(void) efi.systab->hdr.revision >>16 , efi.systab->hdr.revision & 0xffff); + efi.runtime_version = efi.systab->hdr.revision; +/* Show what we know for posterity */ c16 =early_memremap (efi_to_phys (efi.systab->fw_vendor),sizeof (vendor) *sizeof (efi_char16_t));
drivers/firmware/efi/arm-runtime.c +16 -11
@@ -42,10 +42,12 @@ static struct mm_struct efi_mm = {static bool __init efi_virtmap_init (void ) {efi_memory_desc_t *md; +bool systab_found; efi_mm.pgd =pgd_alloc (&efi_mm);init_new_context (NULL , &efi_mm); + systab_found = false;for_each_efi_memory_desc (&memmap, md) {phys_addr_t phys = md->phys_addr;int ret;
@@ -64,8 +66,20 @@ static bool __init efi_virtmap_init(void) &phys, ret);return false; } +/* +* If this entry covers the address of the UEFI system table, +* calculate and record its virtual address. +*/ +if (efi_system_table >= phys && + efi_system_table < phys + (md->num_pages *EFI_PAGE_SIZE )) { + efi.systab = (void *)(efi_system_table - phys + + md->virt_addr); + systab_found = true; + } } -return true; +if (!systab_found) +pr_err ("No virtual mapping found for the UEFI System Table\n" ); +return systab_found; }/*
@@ -99,15 +113,8 @@ static int __init arm_enable_runtime_services(void) memmap.map_end = memmap.map + mapsize; efi.memmap = &memmap; - efi.systab = (__force void *)ioremap_cache (efi_system_table, -sizeof (efi_system_table_t)); -if (!efi.systab) { -pr_err ("Failed to remap EFI System Table\n" ); -return -ENOMEM ; - } -if (!efi_virtmap_init ()) { -pr_err ("No UEFI virtual mapping was installed -- runtime services will not be available\n" ); +pr_err ("UEFI virtual mapping missing or invalid -- runtime services will not be available\n" );return -ENOMEM ; }
@@ -115,8 +122,6 @@ static int __init arm_enable_runtime_services(void)efi_native_runtime_setup ();set_bit (EFI_RUNTIME_SERVICES , &efi.flags); - efi.runtime_version = efi.systab->hdr.revision; -return 0 ; }early_initcall (arm_enable_runtime_services);
Signature · 1 line
2.5.0
PATCH 2/2 arm*: efi: drop writable mapping of the UEFI System table
Quoted 20 lines · Ard Biesheuvel
Commit
2eec5dedf770("efi/arm-init: Use read-only early mappings") updated the early ARM UEFI init code to create the temporary, early mapping of the UEFI System table using read-only attributes, as a hardening measure against inadvertent modification.However, this still leaves the permanent, writable mapping of the UEFI System table, which is only ever referenced during invocations of UEFI Runtime Services, at which time the UEFI virtual mapping is available, which also covers the system table. (This is guaranteed by the fact that
SetVirtualAddressMap(), which is a runtime service itself, converts various entries in the table to their virtual equivalents, which implies that the table must be covered by a RuntimeServicesData region that has theEFI_MEMORY_RUNTIMEattribute.)So instead of creating this permanent mapping, record the virtual address of the system table inside the UEFI virtual mapping, and dereference that when accessing the table. This protects the contents of the system table from inadvertent (or deliberate) modification when no UEFI Runtime Services calls are in progress.
Quoted 5 lines
Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/firmware/efi/arm-init.c | 2 ++ drivers/firmware/efi/arm-runtime.c | 27 ++++++++++++++++----------- 2 files changed, 18 insertions(+), 11 deletions(-)
Looks like a nice cleanup. Applied.
PATCH 2/2 arm*: efi: drop writable mapping of the UEFI System table
Quoted 21 lines · Matt Fleming
Quoted 20 lines · Ard Biesheuvel
Commit
2eec5dedf770("efi/arm-init: Use read-only early mappings") updated the early ARM UEFI init code to create the temporary, early mapping of the UEFI System table using read-only attributes, as a hardening measure against inadvertent modification.However, this still leaves the permanent, writable mapping of the UEFI System table, which is only ever referenced during invocations of UEFI Runtime Services, at which time the UEFI virtual mapping is available, which also covers the system table. (This is guaranteed by the fact that
SetVirtualAddressMap(), which is a runtime service itself, converts various entries in the table to their virtual equivalents, which implies that the table must be covered by a RuntimeServicesData region that has theEFI_MEMORY_RUNTIMEattribute.)So instead of creating this permanent mapping, record the virtual address of the system table inside the UEFI virtual mapping, and dereference that when accessing the table. This protects the contents of the system table from inadvertent (or deliberate) modification when no UEFI Runtime Services calls are in progress.
Quoted 7 lines
Quoted 5 lines
Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/firmware/efi/arm-init.c | 2 ++ drivers/firmware/efi/arm-runtime.c | 27 ++++++++++++++++----------- 2 files changed, 18 insertions(+), 11 deletions(-)Looks like a nice cleanup. Applied.
This patch causes a warning I hadn't spotted before sending it out. Could you fold this in please?
(quoted diff) +0 -0
b/drivers/firmware/efi/arm-runtime.c
index 6c97d4884fc7..fe749da9997e 100644
drivers/firmware/efi/arm-runtime.c +2 -2
@@ -72,8 +72,8 @@ static bool __init efi_virtmap_init(void)*/ if (efi_system_table >= phys && efi_system_table < phys + (md->num_pages *EFI_PAGE_SIZE )) { - efi.systab = (void *)(efi_system_table - phys + - md->virt_addr); + efi.systab = (void *)(unsigned long )(efi_system_table - + phys + md->virt_addr); systab_found = true; } }
Thanks,
Ard.
PATCH 2/2 arm*: efi: drop writable mapping of the UEFI System table
Quoted 10 lines · Ard Biesheuvel
This patch causes a warning I hadn't spotted before sending it out. Could you fold this in please?
drivers/firmware/efi/arm-runtime.c +0
-0
drivers/firmware/efi/arm-runtime.c static bool __init efi_virtmap_init(void) · line 79
Quoted 10 lines
drivers/firmware/efi/arm-runtime.c +2
-2@@ -72,8 +72,8 @@ static bool __init efi_virtmap_init(void)*/ if (efi_system_table >= phys && efi_system_table < phys + (md->num_pages *EFI_PAGE_SIZE )) { - efi.systab = (void *)(efi_system_table - phys + - md->virt_addr); + efi.systab = (void *)(unsigned long )(efi_system_table - + phys + md->virt_addr); systab_found = true; } }
Done, please check,
PATCH 2/2 arm*: efi: drop writable mapping of the UEFI System table
Quoted 24 lines · Matt Fleming
Quoted 10 lines · Ard Biesheuvel
This patch causes a warning I hadn't spotted before sending it out. Could you fold this in please?
drivers/firmware/efi/arm-runtime.c +0
-0drivers/firmware/efi/arm-runtime.c static bool __init efi_virtmap_init(void) · line 79
Quoted 10 lines
drivers/firmware/efi/arm-runtime.c +2
-2@@ -72,8 +72,8 @@ static bool __init efi_virtmap_init(void)*/ if (efi_system_table >= phys && efi_system_table < phys + (md->num_pages *EFI_PAGE_SIZE )) { - efi.systab = (void *)(efi_system_table - phys + - md->virt_addr); + efi.systab = (void *)(unsigned long )(efi_system_table - + phys + md->virt_addr); systab_found = true; } }Done, please check,
Quoted 1 line
Looks good to me, thanks.
PATCH 1/2 efi: get rid of EFI_SYSTEM_TABLES status bit
In reply to Ard BiesheuvelQuoted 11 lines · Ard Biesheuvel
The
EFI_SYSTEM_TABLESstatus bit is set by all EFI supporting architectures upon discovery of the EFI system table, but the bit is never tested in any code we have in the tree. So remove it.Signed-off-by Ard Biesheuvel <[email protected]>--- arch/ia64/kernel/efi.c | 2 -- arch/x86/platform/efi/efi.c | 2 -- drivers/firmware/efi/arm-runtime.c | 1 - include/linux/efi.h | 1 - 4 files changed, 6 deletions(-)
Oops. Looks good, thanks Ard.