diff mbox

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

Message ID 1404487509-26084-1-git-send-email-ard.biesheuvel@linaro.org
State Accepted
Commit 99a5603e2a1f146ac0c6414d8a3669aa749ccff8
Headers show

Commit Message

Ard Biesheuvel July 4, 2014, 3:25 p.m. UTC
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@linaro.org>
---
 arch/arm64/kernel/efi.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

Comments

Mark Salter July 4, 2014, 4:52 p.m. UTC | #1
On Fri, 2014-07-04 at 17:25 +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@linaro.org>
> ---

Acked-by: Mark Salter <msalter@redhat.com>
Catalin Marinas July 4, 2014, 5:01 p.m. UTC | #2
On Fri, Jul 04, 2014 at 05:52:36PM +0100, Mark Salter wrote:
> On Fri, 2014-07-04 at 17:25 +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@linaro.org>
> > ---
> 
> Acked-by: Mark Salter <msalter@redhat.com>

Thanks. Do I take this patch or Matt already has other EFI patches to
push? If the latter:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Ard Biesheuvel July 4, 2014, 5:19 p.m. UTC | #3
On 4 July 2014 19:01, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Fri, Jul 04, 2014 at 05:52:36PM +0100, Mark Salter wrote:
>> On Fri, 2014-07-04 at 17:25 +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@linaro.org>
>> > ---
>>
>> Acked-by: Mark Salter <msalter@redhat.com>
>
> Thanks. Do I take this patch or Matt already has other EFI patches to
> push? If the latter:
>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Thanks. You can take it for 3.16 if you like, otherwise it is probably
better if we ask Matt to take it through his tree with some other
stuff we have in flight at the moment.
Catalin Marinas July 5, 2014, 7:25 a.m. UTC | #4
On 4 Jul 2014, at 18:19, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 4 July 2014 19:01, Catalin Marinas <catalin.marinas@arm.com> wrote:
>> On Fri, Jul 04, 2014 at 05:52:36PM +0100, Mark Salter wrote:
>>> On Fri, 2014-07-04 at 17:25 +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@linaro.org>
>>>> ---
>>> 
>>> Acked-by: Mark Salter <msalter@redhat.com>
>> 
>> Thanks. Do I take this patch or Matt already has other EFI patches to
>> push? If the latter:
>> 
>> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> 
> Thanks. You can take it for 3.16 if you like, otherwise it is probably
> better if we ask Matt to take it through his tree with some other
> stuff we have in flight at the moment.

OK, I’ll leave it to Matt then together with the other EFI patches
he’s already queuing.

Thanks,

Catalin
Matt Fleming July 7, 2014, 8:11 p.m. UTC | #5
On Sat, 05 Jul, at 08:25:35AM, Catalin Marinas wrote:
> 
> OK, I’ll leave it to Matt then together with the other EFI patches
> he’s already queuing.

Thanks guys.

What release are we targetting with this patch? Is it a bug fix for
something likely to affect people? Have there been reports of users
actually hitting this issue?

We're at -rc4 now, so there's still time for getting bug fixes to Linus
but we may start being asked for justification from the tip folks en
route to Linus.

If it's just a case of improving the error paths, can it wait until
v3.17?
Ard Biesheuvel July 7, 2014, 8:38 p.m. UTC | #6
On 7 July 2014 22:11, Matt Fleming <matt@console-pimps.org> wrote:
> On Sat, 05 Jul, at 08:25:35AM, Catalin Marinas wrote:
>>
>> OK, I’ll leave it to Matt then together with the other EFI patches
>> he’s already queuing.
>
> Thanks guys.
>
> What release are we targetting with this patch? Is it a bug fix for
> something likely to affect people? Have there been reports of users
> actually hitting this issue?
>
> We're at -rc4 now, so there's still time for getting bug fixes to Linus
> but we may start being asked for justification from the tip folks en
> route to Linus.
>
> If it's just a case of improving the error paths, can it wait until
> v3.17?
>

This fixes a potential panic under buggy firmware, but no reports of
this actual bug are known, so there is no justification for a late
merge for 3.16.

Thanks,
Ard.
diff mbox

Patch

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 56c3327bbf79..e72f3100958f 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -414,13 +414,24 @@  static int __init arm64_enter_virtual_mode(void)
 	for_each_efi_memory_desc(&memmap, md) {
 		if (!(md->attribute & EFI_MEMORY_RUNTIME))
 			continue;
-		if (remap_region(md, &virt_md))
-			++count;
+		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 +464,14 @@  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 (virt_md = virtmap; count--; virt_md += memmap.desc_size) {
+		md = virt_md;
+		iounmap((__force void __iomem *)md->virt_addr);
+	}
+	kfree(virtmap);
+	return -1;
 }
 early_initcall(arm64_enter_virtual_mode);