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

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

[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.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm64/kernel/efi.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

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);
-- 
1.8.3.2

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

[Mark Salter]

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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---

Acked-by: Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

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

[Catalin Marinas]

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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > ---
> 
> Acked-by: Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

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

Acked-by: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>

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

[Ard Biesheuvel]

On 4 July 2014 19:01, Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org> 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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> > ---
>>
>> Acked-by: Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>
> Thanks. Do I take this patch or Matt already has other EFI patches to
> push? If the latter:
>
> Acked-by: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>

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.

-- 
Ard.

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

[Catalin Marinas]

On 4 Jul 2014, at 18:19, Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On 4 July 2014 19:01, Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org> 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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>> ---
>>> 
>>> Acked-by: Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> 
>> Thanks. Do I take this patch or Matt already has other EFI patches to
>> push? If the latter:
>> 
>> Acked-by: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
> 
> 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--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Matt Fleming]

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?

-- 
Matt Fleming, Intel Open Source Technology Center

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

[Ard Biesheuvel]

On 7 July 2014 22:11, Matt Fleming <matt-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.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.

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

[Ard Biesheuvel]

On 7 July 2014 22:38, Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On 7 July 2014 22:11, Matt Fleming <matt-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.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.
>

Hey Matt,

I was wondering if you're on track to queue up your -next branch for 3.17?

If so, could you please include $subject? And if you don't mind, could
you squash the following fixup into "efi: efistub: Convert into static
library" as well?

diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index cdaedad3afe5..afaeb734295a 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -4,8 +4,7 @@

 CPPFLAGS_vmlinux.lds   := -DTEXT_OFFSET=$(TEXT_OFFSET)
 AFLAGS_head.o          := -DTEXT_OFFSET=$(TEXT_OFFSET)
-CFLAGS_efi-stub.o      := -DTEXT_OFFSET=$(TEXT_OFFSET) \
-                          -I$(src)/../../../scripts/dtc/libfdt
+CFLAGS_efi-stub.o      := -DTEXT_OFFSET=$(TEXT_OFFSET)

 CFLAGS_REMOVE_ftrace.o = -pg
 CFLAGS_REMOVE_insn.o = -pg


Much appreciated,

Cheers,
Ard.

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

[Matt Fleming]

On Fri, 18 Jul, at 08:42:27PM, Ard Biesheuvel wrote:
> 
> Hey Matt,
> 
> I was wondering if you're on track to queue up your -next branch for 3.17?
> 
> If so, could you please include $subject? And if you don't mind, could
> you squash the following fixup into "efi: efistub: Convert into static
> library" as well?
 
Umm, well crap, how did I manage to drop this patch?

I've picked this up now (with all the Acks), thanks. Please could you go
through the EFI 'next' branch and just make sure there's nothing missing
for v3.17?

I was really hoping to get my queue sent to tip today, but I can always
send a second pull if it turns out I did miss something.

> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index cdaedad3afe5..afaeb734295a 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -4,8 +4,7 @@
> 
>  CPPFLAGS_vmlinux.lds   := -DTEXT_OFFSET=$(TEXT_OFFSET)
>  AFLAGS_head.o          := -DTEXT_OFFSET=$(TEXT_OFFSET)
> -CFLAGS_efi-stub.o      := -DTEXT_OFFSET=$(TEXT_OFFSET) \
> -                          -I$(src)/../../../scripts/dtc/libfdt
> +CFLAGS_efi-stub.o      := -DTEXT_OFFSET=$(TEXT_OFFSET)
> 
>  CFLAGS_REMOVE_ftrace.o = -pg
>  CFLAGS_REMOVE_insn.o = -pg

Sure, I've fixed this up.

-- 
Matt Fleming, Intel Open Source Technology Center

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

[Ard Biesheuvel]

On 18 July 2014 21:31, Matt Fleming <matt-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org> wrote:
> On Fri, 18 Jul, at 08:42:27PM, Ard Biesheuvel wrote:
>>
>> Hey Matt,
>>
>> I was wondering if you're on track to queue up your -next branch for 3.17?
>>
>> If so, could you please include $subject? And if you don't mind, could
>> you squash the following fixup into "efi: efistub: Convert into static
>> library" as well?
>
> Umm, well crap, how did I manage to drop this patch?
>
> I've picked this up now (with all the Acks), thanks. Please could you go
> through the EFI 'next' branch and just make sure there's nothing missing
> for v3.17?
>

Thanks, looks lovely. The only thing I notice now is that the commit
message from the patch below is missing the second line, which used to
read

"#including .c files to building the "

or something to that effect. Obviously, the leading hash sign resulted
in this line having been dropped, so this is something that has been
in the patches all along.

IMO there is really no point in bothering fixing this, but perhaps
your inner grammar nazi disagrees.


Cheers,
Ard.


> I was really hoping to get my queue sent to tip today, but I can always
> send a second pull if it turns out I did miss something.
>
>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>> index cdaedad3afe5..afaeb734295a 100644
>> --- a/arch/arm64/kernel/Makefile
>> +++ b/arch/arm64/kernel/Makefile
>> @@ -4,8 +4,7 @@
>>
>>  CPPFLAGS_vmlinux.lds   := -DTEXT_OFFSET=$(TEXT_OFFSET)
>>  AFLAGS_head.o          := -DTEXT_OFFSET=$(TEXT_OFFSET)
>> -CFLAGS_efi-stub.o      := -DTEXT_OFFSET=$(TEXT_OFFSET) \
>> -                          -I$(src)/../../../scripts/dtc/libfdt
>> +CFLAGS_efi-stub.o      := -DTEXT_OFFSET=$(TEXT_OFFSET)
>>
>>  CFLAGS_REMOVE_ftrace.o = -pg
>>  CFLAGS_REMOVE_insn.o = -pg
>
> Sure, I've fixed this up.
>
> --
> Matt Fleming, Intel Open Source Technology Center

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

[Matt Fleming]

On Fri, 18 Jul, at 10:11:13PM, Ard Biesheuvel wrote:
> 
> Thanks, looks lovely. The only thing I notice now is that the commit
> message from the patch below is missing the second line, which used to
> read
> 
> "#including .c files to building the "
> 
> or something to that effect. Obviously, the leading hash sign resulted
> in this line having been dropped, so this is something that has been
> in the patches all along.

Oops, that's my over-zealous patch munging. Well spotted!
 
> IMO there is really no point in bothering fixing this, but perhaps
> your inner grammar nazi disagrees.

He's a tyrant, I fixed it up, mainly because I broke it in the first
place.

-- 
Matt Fleming, Intel Open Source Technology Center