(adding Catalin)
On 4 July 2014 16:42, Mark Salter <msalter@xxxxxxxxxx> 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.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;
>
>
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
