[PATCH v2] efi/arm64: handle missing virtual mapping for UEFI System Table

2014-07-04 Thread Ard Biesheuvel
If we cannot resolve the virtual address of the UEFI System Table, its physical
offset must be missing from the virtual memory map, and there is really no point
in proceeding with installing the virtual memory map and the runtime services
dispatch table. So back out gracefully.

Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
---

v2:
- release mappings and free virtmap before bailing

 arch/arm64/kernel/efi.c | 28 ++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 56c3327bbf79..23942158e0f8 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -416,11 +416,23 @@ static int __init arm64_enter_virtual_mode(void)
continue;
if (remap_region(md, virt_md))
++count;
+   else
+   goto err_unmap;
}
 
efi.systab = (__force void *)efi_lookup_mapped_addr(efi_system_table);
-   if (efi.systab)
-   set_bit(EFI_SYSTEM_TABLES, efi.flags);
+   if (!efi.systab) {
+   /*
+* If we have no virtual mapping for the System Table at this
+* point, the memory map doesn't cover the physical offset where
+* it resides. This means the System Table will be inaccessible
+* to Runtime Services themselves once the virtual mapping is
+* installed.
+*/
+   pr_err(Failed to remap EFI System Table -- buggy firmware?\n);
+   goto err_unmap;
+   }
+   set_bit(EFI_SYSTEM_TABLES, efi.flags);
 
local_irq_save(flags);
cpu_switch_mm(idmap_pg_dir, init_mm);
@@ -453,5 +465,17 @@ static int __init arm64_enter_virtual_mode(void)
set_bit(EFI_RUNTIME_SERVICES, efi.flags);
 
return 0;
+
+err_unmap:
+   /* unmap all mappings that succeeded: there are 'count' of those */
+   for_each_efi_memory_desc(memmap, md) {
+   if (!(md-attribute  EFI_MEMORY_RUNTIME))
+   continue;
+   if (!count--)
+   break;
+   iounmap((__force void *)md-virt_addr);
+   }
+   kfree(virtmap);
+   return -1;
 }
 early_initcall(arm64_enter_virtual_mode);
-- 
1.8.3.2

--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] efi/arm64: handle missing virtual mapping for UEFI System Table

2014-07-04 Thread Mark Salter
On Fri, 2014-07-04 at 12:16 +0200, Ard Biesheuvel wrote:
 If we cannot resolve the virtual address of the UEFI System Table, its 
 physical
 offset must be missing from the virtual memory map, and there is really no 
 point
 in proceeding with installing the virtual memory map and the runtime services
 dispatch table. So back out gracefully.
 
 Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
 ---
 
 v2:
 - release mappings and free virtmap before bailing
 
  arch/arm64/kernel/efi.c | 28 ++--
  1 file changed, 26 insertions(+), 2 deletions(-)
 
 diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
 index 56c3327bbf79..23942158e0f8 100644
 --- a/arch/arm64/kernel/efi.c
 +++ b/arch/arm64/kernel/efi.c
 @@ -416,11 +416,23 @@ static int __init arm64_enter_virtual_mode(void)
   continue;
   if (remap_region(md, virt_md))
   ++count;
 + else
 + goto err_unmap;

How about:
if (!remap_region(md, virt_md))
goto err_unmap;
++count;


   }
  
   efi.systab = (__force void *)efi_lookup_mapped_addr(efi_system_table);
 - if (efi.systab)
 - set_bit(EFI_SYSTEM_TABLES, efi.flags);
 + if (!efi.systab) {
 + /*
 +  * If we have no virtual mapping for the System Table at this
 +  * point, the memory map doesn't cover the physical offset where
 +  * it resides. This means the System Table will be inaccessible
 +  * to Runtime Services themselves once the virtual mapping is
 +  * installed.
 +  */
 + pr_err(Failed to remap EFI System Table -- buggy firmware?\n);
 + goto err_unmap;
 + }
 + set_bit(EFI_SYSTEM_TABLES, efi.flags);
  
   local_irq_save(flags);
   cpu_switch_mm(idmap_pg_dir, init_mm);
 @@ -453,5 +465,17 @@ static int __init arm64_enter_virtual_mode(void)
   set_bit(EFI_RUNTIME_SERVICES, efi.flags);
  
   return 0;
 +
 +err_unmap:
 + /* unmap all mappings that succeeded: there are 'count' of those */
 + for_each_efi_memory_desc(memmap, md) {
 + if (!(md-attribute  EFI_MEMORY_RUNTIME))
 + continue;
 + if (!count--)
 + break;
 + iounmap((__force void *)md-virt_addr);
 + }

This is wrong. memmap still belongs to UEFI and hasn't been touched. The
new mappings are in virtmap. So, it is even simpler:

/* unmap all mappings that succeeded: there are 'count' of those */
for (virt_md = virtmap; count; virt_md++, count--)
iounmap((__force void __iomem *)virt_md-virt_addr);

 + kfree(virtmap);
 + return -1;
  }
  early_initcall(arm64_enter_virtual_mode);


--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] efi/arm64: handle missing virtual mapping for UEFI System Table

2014-07-04 Thread Ard Biesheuvel
(adding Catalin)

On 4 July 2014 16:42, Mark Salter msal...@redhat.com wrote:
 On Fri, 2014-07-04 at 12:16 +0200, Ard Biesheuvel wrote:
 If we cannot resolve the virtual address of the UEFI System Table, its 
 physical
 offset must be missing from the virtual memory map, and there is really no 
 point
 in proceeding with installing the virtual memory map and the runtime services
 dispatch table. So back out gracefully.

 Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
 ---

 v2:
 - release mappings and free virtmap before bailing

  arch/arm64/kernel/efi.c | 28 ++--
  1 file changed, 26 insertions(+), 2 deletions(-)

 diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
 index 56c3327bbf79..23942158e0f8 100644
 --- a/arch/arm64/kernel/efi.c
 +++ b/arch/arm64/kernel/efi.c
 @@ -416,11 +416,23 @@ static int __init arm64_enter_virtual_mode(void)
   continue;
   if (remap_region(md, virt_md))
   ++count;
 + else
 + goto err_unmap;

 How about:
 if (!remap_region(md, virt_md))
 goto err_unmap;
 ++count;



Sure.

   }

   efi.systab = (__force void *)efi_lookup_mapped_addr(efi_system_table);
 - if (efi.systab)
 - set_bit(EFI_SYSTEM_TABLES, efi.flags);
 + if (!efi.systab) {
 + /*
 +  * If we have no virtual mapping for the System Table at this
 +  * point, the memory map doesn't cover the physical offset 
 where
 +  * it resides. This means the System Table will be inaccessible
 +  * to Runtime Services themselves once the virtual mapping is
 +  * installed.
 +  */
 + pr_err(Failed to remap EFI System Table -- buggy 
 firmware?\n);
 + goto err_unmap;
 + }
 + set_bit(EFI_SYSTEM_TABLES, efi.flags);

   local_irq_save(flags);
   cpu_switch_mm(idmap_pg_dir, init_mm);
 @@ -453,5 +465,17 @@ static int __init arm64_enter_virtual_mode(void)
   set_bit(EFI_RUNTIME_SERVICES, efi.flags);

   return 0;
 +
 +err_unmap:
 + /* unmap all mappings that succeeded: there are 'count' of those */
 + for_each_efi_memory_desc(memmap, md) {
 + if (!(md-attribute  EFI_MEMORY_RUNTIME))
 + continue;
 + if (!count--)
 + break;
 + iounmap((__force void *)md-virt_addr);
 + }

 This is wrong. memmap still belongs to UEFI and hasn't been touched. The
 new mappings are in virtmap. So, it is even simpler:

 /* unmap all mappings that succeeded: there are 'count' of those */
 for (virt_md = virtmap; count; virt_md++, count--)
 iounmap((__force void __iomem *)virt_md-virt_addr);


Yes, you're right. Will respin.

-- 
Ard.
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html