Skip to contentSkip to conversation

arm64 efi fixes

PATCH New Ard Biesheuvel 4 participants 8 messages Nov 23, 2015 – Nov 26, 2015

Subsystems (2)
arm64 EFI
lore
Share & tools
lore

Ard Biesheuvel

author
#0 lore raw

cover arm64 efi fixes

Two minor issues that were spotted by Matt Fleming and Russell King during review of the UEFI for 32-bit ARM series. No point in delaying these fixes until the whole series gets acked, so I am sending them out separately beforehand.

Ard Biesheuvel (2):
arm64/efi: deal with NULL return value of early_memremap()
arm64: efi: fix initcall return values

 arch/arm64/kernel/efi.c | 24 +++++++++++++++-----
 1 file changed, 18 insertions(+), 6 deletions(-)
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH 1/2 arm64/efi: deal with NULL return value of early_memremap()

Add NULL return value checks to two invocations of early_memremap() in the UEFI init code. For the UEFI configuration tables, we just warn since we have a better chance of being able to report the issue in a way that can actually be noticed by a human operator if we don't abort right away. For the UEFI memory map, however, all we can do is panic() since we cannot proceed without a description of memory.

Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/kernel/efi.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
arch/arm64/kernel/efi.c +13 -1
index fc5508e0df57..186b9e119d4a 100644
@@ -127,7 +127,11 @@ static int __init uefi_init(void)
 	table_size = sizeof(efi_config_table_64_t) * efi.systab->nr_tables;
 	config_tables = early_memremap(efi_to_phys(efi.systab->tables),
 				       table_size);
-
+	if (config_tables == NULL) {
+		pr_warn("Unable to map EFI config table array.\n");
+		retval = -ENOMEM;
+		goto out;
+	}
 	retval = efi_config_parse_tables(config_tables, efi.systab->nr_tables,
 					 sizeof(efi_config_table_64_t), NULL);
 

@@ -209,6 +213,14 @@ void __init efi_init(void)
 			 PAGE_ALIGN(params.mmap_size + (params.mmap & ~PAGE_MASK)));
 	memmap.phys_map = params.mmap;
 	memmap.map = early_memremap(params.mmap, params.mmap_size);
+	if (memmap.map == NULL) {
+		/*
+		* If we are booting via UEFI, the UEFI memory map is the only
+		* description of memory we have, so there is little point in
+		* proceeding if we cannot access it.
+		*/
+		panic("Unable to map EFI memory map.\n");
+	}
 	memmap.map_end = memmap.map + params.mmap_size;
 	memmap.desc_size = params.desc_size;
 	memmap.desc_version = params.desc_ver;
Signature · 1 line
1.9.1
#2 lore raw

PATCH 1/2 arm64/efi: deal with NULL return value of early_memremap()

Quoted 11 lines · Ard Biesheuvel

Add NULL return value checks to two invocations of early_memremap() in the UEFI init code. For the UEFI configuration tables, we just warn since we have a better chance of being able to report the issue in a way that can actually be noticed by a human operator if we don't abort right away. For the UEFI memory map, however, all we can do is panic() since we cannot proceed without a description of memory.

Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/kernel/efi.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
Reviewed-by Matt Fleming <[email protected]>
author
#3 lore raw

PATCH 2/2 arm64: efi: fix initcall return values

In reply to Ard Biesheuvel

Even though initcall return values are typically ignored, the prototype is to return 0 on success or a negative errno value on error. So fix the arm_enable_runtime_services() implementation to return 0 on conditions that are not in fact errors, and return a meaningful error code otherwise.

Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/kernel/efi.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
arch/arm64/kernel/efi.c +5 -5
index 186b9e119d4a..3aeb576965c0 100644
@@ -285,12 +285,12 @@ static int __init arm64_enable_runtime_services(void)
 
 	if (!efi_enabled(EFI_BOOT)) {
 		pr_info("EFI services will not be available.\n");
-		return -1;
+		return 0;
 	}
 
 	if (efi_runtime_disabled()) {
 		pr_info("EFI runtime services will be disabled.\n");
-		return -1;
+		return 0;
 	}
 
 	pr_info("Remapping and enabling EFI services.\n");

@@ -300,7 +300,7 @@ static int __init arm64_enable_runtime_services(void)
 						   mapsize);
 	if (!memmap.map) {
 		pr_err("Failed to remap EFI memory map\n");
-		return -1;
+		return -ENOMEM;
 	}
 	memmap.map_end = memmap.map + mapsize;
 	efi.memmap = &memmap;

@@ -309,13 +309,13 @@ static int __init arm64_enable_runtime_services(void)
 						   sizeof(efi_system_table_t));
 	if (!efi.systab) {
 		pr_err("Failed to remap EFI System Table\n");
-		return -1;
+		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");
-		return -1;
+		return -ENOMEM;
 	}
 
 	/* Set up runtime services function pointers */
Signature · 1 line
1.9.1
#4 lore raw

PATCH 2/2 arm64: efi: fix initcall return values

Quoted 10 lines · Ard Biesheuvel

Even though initcall return values are typically ignored, the prototype is to return 0 on success or a negative errno value on error. So fix the arm_enable_runtime_services() implementation to return 0 on conditions that are not in fact errors, and return a meaningful error code otherwise.

Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/kernel/efi.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

FWIW,

Reviewed-by Matt Fleming <[email protected]>
#5 lore raw

cover arm64 efi fixes

In reply to Ard Biesheuvel
Quoted 8 lines · Ard Biesheuvel

Two minor issues that were spotted by Matt Fleming and Russell King during review of the UEFI for 32-bit ARM series. No point in delaying these fixes until the whole series gets acked, so I am sending them out separately beforehand.

Ard Biesheuvel (2):
arm64/efi: deal with NULL return value of early_memremap()
arm64: efi: fix initcall return values

These both look like straightforward fixes to me:

Acked-by Will Deacon <[email protected]>

I assume Catalin will pick them up. Do they need to go to -stable?

Will

author
#6 lore raw

cover arm64 efi fixes

Quoted 10 lines · Will Deacon
Quoted 8 lines · Ard Biesheuvel

Two minor issues that were spotted by Matt Fleming and Russell King during review of the UEFI for 32-bit ARM series. No point in delaying these fixes until the whole series gets acked, so I am sending them out separately beforehand.

Ard Biesheuvel (2):
arm64/efi: deal with NULL return value of early_memremap()
arm64: efi: fix initcall return values

Quoted 6 lines

These both look like straightforward fixes to me:

Acked-by Will Deacon <[email protected]>

I assume Catalin will pick them up. Do they need to go to -stable?

No, I don't see the point.

Thanks,
Ard.

maintainer
#7 lore raw

cover arm64 efi fixes

In reply to Ard Biesheuvel
Quoted 8 lines · Ard Biesheuvel

Two minor issues that were spotted by Matt Fleming and Russell King during review of the UEFI for 32-bit ARM series. No point in delaying these fixes until the whole series gets acked, so I am sending them out separately beforehand.

Ard Biesheuvel (2):
arm64/efi: deal with NULL return value of early_memremap()
arm64: efi: fix initcall return values

Patches applied. Thanks.

Signature · 1 line
Catalin