[PATCH 0/2] arm64 efi fixes

[PATCH 0/2] arm64 efi fixes

[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

 arch/arm64/kernel/efi.c | 24 +++++++++++++++-----
 1 file changed, 18 insertions(+), 6 deletions(-)

-- 
1.9.1

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

[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 <ard.biesheuvel@linaro.org>
---
 arch/arm64/kernel/efi.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index fc5508e0df57..186b9e119d4a 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -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;
-- 
1.9.1

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

[Matt Fleming]

On Mon, 23 Nov, at 08:43:23AM, Ard Biesheuvel wrote:
> 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 <ard.biesheuvel@linaro.org>
> ---
>  arch/arm64/kernel/efi.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)

Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk>

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

[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 <ard.biesheuvel@linaro.org>
---
 arch/arm64/kernel/efi.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 186b9e119d4a..3aeb576965c0 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -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 */
-- 
1.9.1

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

[Matt Fleming]

On Mon, 23 Nov, at 08:43:24AM, Ard Biesheuvel wrote:
> 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 <ard.biesheuvel@linaro.org>
> ---
>  arch/arm64/kernel/efi.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

FWIW, 

Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk>

[PATCH 0/2] arm64 efi fixes

[Will Deacon]

On Mon, Nov 23, 2015 at 08:43:22AM +0100, Ard Biesheuvel wrote:
> 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 <will.deacon@arm.com>

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

Will

[PATCH 0/2] arm64 efi fixes

[Ard Biesheuvel]

On 23 November 2015 at 18:35, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Nov 23, 2015 at 08:43:22AM +0100, Ard Biesheuvel wrote:
>> 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 <will.deacon@arm.com>
>
> I assume Catalin will pick them up. Do they need to go to -stable?
>

No, I don't see the point.

Thanks,
Ard.

[PATCH 0/2] arm64 efi fixes

[Catalin Marinas]

On Mon, Nov 23, 2015 at 08:43:22AM +0100, Ard Biesheuvel wrote:
> 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.

-- 
Catalin