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 <[email protected]>
> ---
> 
> 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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to