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.biesheuvel@xxxxxxxxxx>
> ---
>
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
