All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] efi/arm64: handle missing virtual mapping for UEFI System Table
@ 2014-07-04 10:16 ` Ard Biesheuvel
  0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2014-07-04 10:16 UTC (permalink / raw)
  To: matt.fleming-ral2JQCrhuEAvxtiuMwx3w,
	msalter-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	leif.lindholm-QSEj5FYQhm4dnm+yROfE0A, 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>
---

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;
 	}
 
 	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);
+	}
+	kfree(virtmap);
+	return -1;
 }
 early_initcall(arm64_enter_virtual_mode);
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2] efi/arm64: handle missing virtual mapping for UEFI System Table
@ 2014-07-04 10:16 ` Ard Biesheuvel
  0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2014-07-04 10:16 UTC (permalink / raw)
  To: linux-arm-kernel

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>
---

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;
 	}
 
 	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);
+	}
+	kfree(virtmap);
+	return -1;
 }
 early_initcall(arm64_enter_virtual_mode);
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] efi/arm64: handle missing virtual mapping for UEFI System Table
  2014-07-04 10:16 ` Ard Biesheuvel
@ 2014-07-04 14:42     ` Mark Salter
  -1 siblings, 0 replies; 6+ messages in thread
From: Mark Salter @ 2014-07-04 14:42 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: matt.fleming-ral2JQCrhuEAvxtiuMwx3w,
	linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	leif.lindholm-QSEj5FYQhm4dnm+yROfE0A

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] efi/arm64: handle missing virtual mapping for UEFI System Table
@ 2014-07-04 14:42     ` Mark Salter
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Salter @ 2014-07-04 14:42 UTC (permalink / raw)
  To: linux-arm-kernel

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@linaro.org>
> ---
> 
> 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);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] efi/arm64: handle missing virtual mapping for UEFI System Table
  2014-07-04 14:42     ` Mark Salter
@ 2014-07-04 14:51         ` Ard Biesheuvel
  -1 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2014-07-04 14:51 UTC (permalink / raw)
  To: Mark Salter, Catalin Marinas
  Cc: Matt Fleming, linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Leif Lindholm

(adding Catalin)

On 4 July 2014 16:42, Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>
>> 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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] efi/arm64: handle missing virtual mapping for UEFI System Table
@ 2014-07-04 14:51         ` Ard Biesheuvel
  0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2014-07-04 14:51 UTC (permalink / raw)
  To: linux-arm-kernel

(adding Catalin)

On 4 July 2014 16:42, Mark Salter <msalter@redhat.com> 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@linaro.org>
>> ---
>>
>> 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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-07-04 14:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-04 10:16 [PATCH v2] efi/arm64: handle missing virtual mapping for UEFI System Table Ard Biesheuvel
2014-07-04 10:16 ` Ard Biesheuvel
     [not found] ` <1404468997-6925-1-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2014-07-04 14:42   ` Mark Salter
2014-07-04 14:42     ` Mark Salter
     [not found]     ` <1404484941.19665.56.camel-PDpCo7skNiwAicBL8TP8PQ@public.gmane.org>
2014-07-04 14:51       ` Ard Biesheuvel
2014-07-04 14:51         ` Ard Biesheuvel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.