All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] efi: get rid of EFI_SYSTEM_TABLES status bit
@ 2016-02-26 14:20 ` Ard Biesheuvel
  0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2016-02-26 14:20 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-efi-u79uwXL29TY76Z2rM5mHXA,
	matt-mF/unelCI9GS6iBeEJttW/XRex20P6io,
	leif.lindholm-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8
  Cc: Ard Biesheuvel

The EFI_SYSTEM_TABLES status bit is set by all EFI supporting architectures
upon discovery of the EFI system table, but the bit is never tested in any
code we have in the tree. So remove it.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/ia64/kernel/efi.c             | 2 --
 arch/x86/platform/efi/efi.c        | 2 --
 drivers/firmware/efi/arm-runtime.c | 1 -
 include/linux/efi.h                | 1 -
 4 files changed, 6 deletions(-)

diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
index caae3f4e4341..e3456698a2a7 100644
--- a/arch/ia64/kernel/efi.c
+++ b/arch/ia64/kernel/efi.c
@@ -531,8 +531,6 @@ efi_init (void)
 	       efi.systab->hdr.revision >> 16,
 	       efi.systab->hdr.revision & 0xffff, vendor);
 
-	set_bit(EFI_SYSTEM_TABLES, &efi.flags);
-
 	palo_phys      = EFI_INVALID_TABLE_ADDR;
 
 	if (efi_config_init(arch_tables) != 0)
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index ad285404ea7f..48dd701dacf6 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -352,8 +352,6 @@ static int __init efi_systab_init(void *phys)
 		       efi.systab->hdr.revision >> 16,
 		       efi.systab->hdr.revision & 0xffff);
 
-	set_bit(EFI_SYSTEM_TABLES, &efi.flags);
-
 	return 0;
 }
 
diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 6ae21e41a429..16c7d2a71156 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -105,7 +105,6 @@ static int __init arm_enable_runtime_services(void)
 		pr_err("Failed to remap EFI System Table\n");
 		return -ENOMEM;
 	}
-	set_bit(EFI_SYSTEM_TABLES, &efi.flags);
 
 	if (!efi_virtmap_init()) {
 		pr_err("No UEFI virtual mapping was installed -- runtime services will not be available\n");
diff --git a/include/linux/efi.h b/include/linux/efi.h
index e747eb08b2be..99bc85f708d4 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -975,7 +975,6 @@ extern int __init efi_setup_pcdp_console(char *);
  * possible, remove EFI-related code altogether.
  */
 #define EFI_BOOT		0	/* Were we booted from EFI? */
-#define EFI_SYSTEM_TABLES	1	/* Can we use EFI system tables? */
 #define EFI_CONFIG_TABLES	2	/* Can we use EFI config tables? */
 #define EFI_RUNTIME_SERVICES	3	/* Can we use runtime services? */
 #define EFI_MEMMAP		4	/* Can we use EFI memory map? */
-- 
2.5.0

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

* [PATCH 1/2] efi: get rid of EFI_SYSTEM_TABLES status bit
@ 2016-02-26 14:20 ` Ard Biesheuvel
  0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2016-02-26 14:20 UTC (permalink / raw)
  To: linux-arm-kernel

The EFI_SYSTEM_TABLES status bit is set by all EFI supporting architectures
upon discovery of the EFI system table, but the bit is never tested in any
code we have in the tree. So remove it.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/ia64/kernel/efi.c             | 2 --
 arch/x86/platform/efi/efi.c        | 2 --
 drivers/firmware/efi/arm-runtime.c | 1 -
 include/linux/efi.h                | 1 -
 4 files changed, 6 deletions(-)

diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
index caae3f4e4341..e3456698a2a7 100644
--- a/arch/ia64/kernel/efi.c
+++ b/arch/ia64/kernel/efi.c
@@ -531,8 +531,6 @@ efi_init (void)
 	       efi.systab->hdr.revision >> 16,
 	       efi.systab->hdr.revision & 0xffff, vendor);
 
-	set_bit(EFI_SYSTEM_TABLES, &efi.flags);
-
 	palo_phys      = EFI_INVALID_TABLE_ADDR;
 
 	if (efi_config_init(arch_tables) != 0)
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index ad285404ea7f..48dd701dacf6 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -352,8 +352,6 @@ static int __init efi_systab_init(void *phys)
 		       efi.systab->hdr.revision >> 16,
 		       efi.systab->hdr.revision & 0xffff);
 
-	set_bit(EFI_SYSTEM_TABLES, &efi.flags);
-
 	return 0;
 }
 
diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 6ae21e41a429..16c7d2a71156 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -105,7 +105,6 @@ static int __init arm_enable_runtime_services(void)
 		pr_err("Failed to remap EFI System Table\n");
 		return -ENOMEM;
 	}
-	set_bit(EFI_SYSTEM_TABLES, &efi.flags);
 
 	if (!efi_virtmap_init()) {
 		pr_err("No UEFI virtual mapping was installed -- runtime services will not be available\n");
diff --git a/include/linux/efi.h b/include/linux/efi.h
index e747eb08b2be..99bc85f708d4 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -975,7 +975,6 @@ extern int __init efi_setup_pcdp_console(char *);
  * possible, remove EFI-related code altogether.
  */
 #define EFI_BOOT		0	/* Were we booted from EFI? */
-#define EFI_SYSTEM_TABLES	1	/* Can we use EFI system tables? */
 #define EFI_CONFIG_TABLES	2	/* Can we use EFI config tables? */
 #define EFI_RUNTIME_SERVICES	3	/* Can we use runtime services? */
 #define EFI_MEMMAP		4	/* Can we use EFI memory map? */
-- 
2.5.0

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

* [PATCH 2/2] arm*: efi: drop writable mapping of the UEFI System table
  2016-02-26 14:20 ` Ard Biesheuvel
@ 2016-02-26 14:20     ` Ard Biesheuvel
  -1 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2016-02-26 14:20 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-efi-u79uwXL29TY76Z2rM5mHXA,
	matt-mF/unelCI9GS6iBeEJttW/XRex20P6io,
	leif.lindholm-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8
  Cc: Ard Biesheuvel

Commit 2eec5dedf770 ("efi/arm-init: Use read-only early mappings")
updated the early ARM UEFI init code to create the temporary, early
mapping of the UEFI System table using read-only attributes, as a
hardening measure against inadvertent modification.

However, this still leaves the permanent, writable mapping of the UEFI
System table, which is only ever referenced during invocations of UEFI
Runtime Services, at which time the UEFI virtual mapping is available,
which also covers the system table. (This is guaranteed by the fact that
SetVirtualAddressMap(), which is a runtime service itself, converts
various entries in the table to their virtual equivalents, which implies
that the table must be covered by a RuntimeServicesData region that has
the EFI_MEMORY_RUNTIME attribute.)

So instead of creating this permanent mapping, record the virtual address
of the system table inside the UEFI virtual mapping, and dereference that
when accessing the table. This protects the contents of the system table
from inadvertent (or deliberate) modification when no UEFI Runtime
Services calls are in progress.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/firmware/efi/arm-init.c    |  2 ++
 drivers/firmware/efi/arm-runtime.c | 27 ++++++++++++++++-----------
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index 9e15d571b53c..415ddfc213a7 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -85,6 +85,8 @@ static int __init uefi_init(void)
 			efi.systab->hdr.revision >> 16,
 			efi.systab->hdr.revision & 0xffff);
 
+	efi.runtime_version = efi.systab->hdr.revision;
+
 	/* Show what we know for posterity */
 	c16 = early_memremap(efi_to_phys(efi.systab->fw_vendor),
 			     sizeof(vendor) * sizeof(efi_char16_t));
diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 16c7d2a71156..6c97d4884fc7 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -42,10 +42,12 @@ static struct mm_struct efi_mm = {
 static bool __init efi_virtmap_init(void)
 {
 	efi_memory_desc_t *md;
+	bool systab_found;
 
 	efi_mm.pgd = pgd_alloc(&efi_mm);
 	init_new_context(NULL, &efi_mm);
 
+	systab_found = false;
 	for_each_efi_memory_desc(&memmap, md) {
 		phys_addr_t phys = md->phys_addr;
 		int ret;
@@ -64,8 +66,20 @@ static bool __init efi_virtmap_init(void)
 				&phys, ret);
 			return false;
 		}
+		/*
+		 * If this entry covers the address of the UEFI system table,
+		 * calculate and record its virtual address.
+		 */
+		if (efi_system_table >= phys &&
+		    efi_system_table < phys + (md->num_pages * EFI_PAGE_SIZE)) {
+			efi.systab = (void *)(efi_system_table - phys +
+					      md->virt_addr);
+			systab_found = true;
+		}
 	}
-	return true;
+	if (!systab_found)
+		pr_err("No virtual mapping found for the UEFI System Table\n");
+	return systab_found;
 }
 
 /*
@@ -99,15 +113,8 @@ static int __init arm_enable_runtime_services(void)
 	memmap.map_end = memmap.map + mapsize;
 	efi.memmap = &memmap;
 
-	efi.systab = (__force void *)ioremap_cache(efi_system_table,
-						   sizeof(efi_system_table_t));
-	if (!efi.systab) {
-		pr_err("Failed to remap EFI System Table\n");
-		return -ENOMEM;
-	}
-
 	if (!efi_virtmap_init()) {
-		pr_err("No UEFI virtual mapping was installed -- runtime services will not be available\n");
+		pr_err("UEFI virtual mapping missing or invalid -- runtime services will not be available\n");
 		return -ENOMEM;
 	}
 
@@ -115,8 +122,6 @@ static int __init arm_enable_runtime_services(void)
 	efi_native_runtime_setup();
 	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
 
-	efi.runtime_version = efi.systab->hdr.revision;
-
 	return 0;
 }
 early_initcall(arm_enable_runtime_services);
-- 
2.5.0

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

* [PATCH 2/2] arm*: efi: drop writable mapping of the UEFI System table
@ 2016-02-26 14:20     ` Ard Biesheuvel
  0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2016-02-26 14:20 UTC (permalink / raw)
  To: linux-arm-kernel

Commit 2eec5dedf770 ("efi/arm-init: Use read-only early mappings")
updated the early ARM UEFI init code to create the temporary, early
mapping of the UEFI System table using read-only attributes, as a
hardening measure against inadvertent modification.

However, this still leaves the permanent, writable mapping of the UEFI
System table, which is only ever referenced during invocations of UEFI
Runtime Services, at which time the UEFI virtual mapping is available,
which also covers the system table. (This is guaranteed by the fact that
SetVirtualAddressMap(), which is a runtime service itself, converts
various entries in the table to their virtual equivalents, which implies
that the table must be covered by a RuntimeServicesData region that has
the EFI_MEMORY_RUNTIME attribute.)

So instead of creating this permanent mapping, record the virtual address
of the system table inside the UEFI virtual mapping, and dereference that
when accessing the table. This protects the contents of the system table
from inadvertent (or deliberate) modification when no UEFI Runtime
Services calls are in progress.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/arm-init.c    |  2 ++
 drivers/firmware/efi/arm-runtime.c | 27 ++++++++++++++++-----------
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index 9e15d571b53c..415ddfc213a7 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -85,6 +85,8 @@ static int __init uefi_init(void)
 			efi.systab->hdr.revision >> 16,
 			efi.systab->hdr.revision & 0xffff);
 
+	efi.runtime_version = efi.systab->hdr.revision;
+
 	/* Show what we know for posterity */
 	c16 = early_memremap(efi_to_phys(efi.systab->fw_vendor),
 			     sizeof(vendor) * sizeof(efi_char16_t));
diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 16c7d2a71156..6c97d4884fc7 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -42,10 +42,12 @@ static struct mm_struct efi_mm = {
 static bool __init efi_virtmap_init(void)
 {
 	efi_memory_desc_t *md;
+	bool systab_found;
 
 	efi_mm.pgd = pgd_alloc(&efi_mm);
 	init_new_context(NULL, &efi_mm);
 
+	systab_found = false;
 	for_each_efi_memory_desc(&memmap, md) {
 		phys_addr_t phys = md->phys_addr;
 		int ret;
@@ -64,8 +66,20 @@ static bool __init efi_virtmap_init(void)
 				&phys, ret);
 			return false;
 		}
+		/*
+		 * If this entry covers the address of the UEFI system table,
+		 * calculate and record its virtual address.
+		 */
+		if (efi_system_table >= phys &&
+		    efi_system_table < phys + (md->num_pages * EFI_PAGE_SIZE)) {
+			efi.systab = (void *)(efi_system_table - phys +
+					      md->virt_addr);
+			systab_found = true;
+		}
 	}
-	return true;
+	if (!systab_found)
+		pr_err("No virtual mapping found for the UEFI System Table\n");
+	return systab_found;
 }
 
 /*
@@ -99,15 +113,8 @@ static int __init arm_enable_runtime_services(void)
 	memmap.map_end = memmap.map + mapsize;
 	efi.memmap = &memmap;
 
-	efi.systab = (__force void *)ioremap_cache(efi_system_table,
-						   sizeof(efi_system_table_t));
-	if (!efi.systab) {
-		pr_err("Failed to remap EFI System Table\n");
-		return -ENOMEM;
-	}
-
 	if (!efi_virtmap_init()) {
-		pr_err("No UEFI virtual mapping was installed -- runtime services will not be available\n");
+		pr_err("UEFI virtual mapping missing or invalid -- runtime services will not be available\n");
 		return -ENOMEM;
 	}
 
@@ -115,8 +122,6 @@ static int __init arm_enable_runtime_services(void)
 	efi_native_runtime_setup();
 	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
 
-	efi.runtime_version = efi.systab->hdr.revision;
-
 	return 0;
 }
 early_initcall(arm_enable_runtime_services);
-- 
2.5.0

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

* Re: [PATCH 1/2] efi: get rid of EFI_SYSTEM_TABLES status bit
  2016-02-26 14:20 ` Ard Biesheuvel
@ 2016-02-26 14:43     ` Matt Fleming
  -1 siblings, 0 replies; 14+ messages in thread
From: Matt Fleming @ 2016-02-26 14:43 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-efi-u79uwXL29TY76Z2rM5mHXA,
	leif.lindholm-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8

On Fri, 26 Feb, at 03:20:34PM, Ard Biesheuvel wrote:
> The EFI_SYSTEM_TABLES status bit is set by all EFI supporting architectures
> upon discovery of the EFI system table, but the bit is never tested in any
> code we have in the tree. So remove it.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/ia64/kernel/efi.c             | 2 --
>  arch/x86/platform/efi/efi.c        | 2 --
>  drivers/firmware/efi/arm-runtime.c | 1 -
>  include/linux/efi.h                | 1 -
>  4 files changed, 6 deletions(-)

Oops. Looks good, thanks Ard.

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

* [PATCH 1/2] efi: get rid of EFI_SYSTEM_TABLES status bit
@ 2016-02-26 14:43     ` Matt Fleming
  0 siblings, 0 replies; 14+ messages in thread
From: Matt Fleming @ 2016-02-26 14:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 26 Feb, at 03:20:34PM, Ard Biesheuvel wrote:
> The EFI_SYSTEM_TABLES status bit is set by all EFI supporting architectures
> upon discovery of the EFI system table, but the bit is never tested in any
> code we have in the tree. So remove it.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/ia64/kernel/efi.c             | 2 --
>  arch/x86/platform/efi/efi.c        | 2 --
>  drivers/firmware/efi/arm-runtime.c | 1 -
>  include/linux/efi.h                | 1 -
>  4 files changed, 6 deletions(-)

Oops. Looks good, thanks Ard.

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

* Re: [PATCH 2/2] arm*: efi: drop writable mapping of the UEFI System table
  2016-02-26 14:20     ` Ard Biesheuvel
@ 2016-02-26 15:01         ` Matt Fleming
  -1 siblings, 0 replies; 14+ messages in thread
From: Matt Fleming @ 2016-02-26 15:01 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-efi-u79uwXL29TY76Z2rM5mHXA,
	leif.lindholm-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8

On Fri, 26 Feb, at 03:20:35PM, Ard Biesheuvel wrote:
> Commit 2eec5dedf770 ("efi/arm-init: Use read-only early mappings")
> updated the early ARM UEFI init code to create the temporary, early
> mapping of the UEFI System table using read-only attributes, as a
> hardening measure against inadvertent modification.
> 
> However, this still leaves the permanent, writable mapping of the UEFI
> System table, which is only ever referenced during invocations of UEFI
> Runtime Services, at which time the UEFI virtual mapping is available,
> which also covers the system table. (This is guaranteed by the fact that
> SetVirtualAddressMap(), which is a runtime service itself, converts
> various entries in the table to their virtual equivalents, which implies
> that the table must be covered by a RuntimeServicesData region that has
> the EFI_MEMORY_RUNTIME attribute.)
> 
> So instead of creating this permanent mapping, record the virtual address
> of the system table inside the UEFI virtual mapping, and dereference that
> when accessing the table. This protects the contents of the system table
> from inadvertent (or deliberate) modification when no UEFI Runtime
> Services calls are in progress.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/firmware/efi/arm-init.c    |  2 ++
>  drivers/firmware/efi/arm-runtime.c | 27 ++++++++++++++++-----------
>  2 files changed, 18 insertions(+), 11 deletions(-)

Looks like a nice cleanup. Applied.

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

* [PATCH 2/2] arm*: efi: drop writable mapping of the UEFI System table
@ 2016-02-26 15:01         ` Matt Fleming
  0 siblings, 0 replies; 14+ messages in thread
From: Matt Fleming @ 2016-02-26 15:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 26 Feb, at 03:20:35PM, Ard Biesheuvel wrote:
> Commit 2eec5dedf770 ("efi/arm-init: Use read-only early mappings")
> updated the early ARM UEFI init code to create the temporary, early
> mapping of the UEFI System table using read-only attributes, as a
> hardening measure against inadvertent modification.
> 
> However, this still leaves the permanent, writable mapping of the UEFI
> System table, which is only ever referenced during invocations of UEFI
> Runtime Services, at which time the UEFI virtual mapping is available,
> which also covers the system table. (This is guaranteed by the fact that
> SetVirtualAddressMap(), which is a runtime service itself, converts
> various entries in the table to their virtual equivalents, which implies
> that the table must be covered by a RuntimeServicesData region that has
> the EFI_MEMORY_RUNTIME attribute.)
> 
> So instead of creating this permanent mapping, record the virtual address
> of the system table inside the UEFI virtual mapping, and dereference that
> when accessing the table. This protects the contents of the system table
> from inadvertent (or deliberate) modification when no UEFI Runtime
> Services calls are in progress.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  drivers/firmware/efi/arm-init.c    |  2 ++
>  drivers/firmware/efi/arm-runtime.c | 27 ++++++++++++++++-----------
>  2 files changed, 18 insertions(+), 11 deletions(-)

Looks like a nice cleanup. Applied.

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

* Re: [PATCH 2/2] arm*: efi: drop writable mapping of the UEFI System table
  2016-02-26 15:01         ` Matt Fleming
@ 2016-03-22 15:08             ` Ard Biesheuvel
  -1 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2016-03-22 15:08 UTC (permalink / raw)
  To: Matt Fleming
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-efi-u79uwXL29TY76Z2rM5mHXA, Leif Lindholm, Mark Rutland

On 26 February 2016 at 16:01, Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:
> On Fri, 26 Feb, at 03:20:35PM, Ard Biesheuvel wrote:
>> Commit 2eec5dedf770 ("efi/arm-init: Use read-only early mappings")
>> updated the early ARM UEFI init code to create the temporary, early
>> mapping of the UEFI System table using read-only attributes, as a
>> hardening measure against inadvertent modification.
>>
>> However, this still leaves the permanent, writable mapping of the UEFI
>> System table, which is only ever referenced during invocations of UEFI
>> Runtime Services, at which time the UEFI virtual mapping is available,
>> which also covers the system table. (This is guaranteed by the fact that
>> SetVirtualAddressMap(), which is a runtime service itself, converts
>> various entries in the table to their virtual equivalents, which implies
>> that the table must be covered by a RuntimeServicesData region that has
>> the EFI_MEMORY_RUNTIME attribute.)
>>
>> So instead of creating this permanent mapping, record the virtual address
>> of the system table inside the UEFI virtual mapping, and dereference that
>> when accessing the table. This protects the contents of the system table
>> from inadvertent (or deliberate) modification when no UEFI Runtime
>> Services calls are in progress.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  drivers/firmware/efi/arm-init.c    |  2 ++
>>  drivers/firmware/efi/arm-runtime.c | 27 ++++++++++++++++-----------
>>  2 files changed, 18 insertions(+), 11 deletions(-)
>
> Looks like a nice cleanup. Applied.

This patch causes a warning I hadn't spotted before sending it out.
Could you fold this in please?

diff --git a/drivers/firmware/efi/arm-runtime.c
b/drivers/firmware/efi/arm-runtime.c
index 6c97d4884fc7..fe749da9997e 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -72,8 +72,8 @@ static bool __init efi_virtmap_init(void)
                 */
                if (efi_system_table >= phys &&
                    efi_system_table < phys + (md->num_pages * EFI_PAGE_SIZE)) {
-                       efi.systab = (void *)(efi_system_table - phys +
-                                             md->virt_addr);
+                       efi.systab = (void *)(unsigned long)(efi_system_table -
+                                                         phys + md->virt_addr);
                        systab_found = true;
                }
        }

Thanks,
Ard.

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

* [PATCH 2/2] arm*: efi: drop writable mapping of the UEFI System table
@ 2016-03-22 15:08             ` Ard Biesheuvel
  0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2016-03-22 15:08 UTC (permalink / raw)
  To: linux-arm-kernel

On 26 February 2016 at 16:01, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> On Fri, 26 Feb, at 03:20:35PM, Ard Biesheuvel wrote:
>> Commit 2eec5dedf770 ("efi/arm-init: Use read-only early mappings")
>> updated the early ARM UEFI init code to create the temporary, early
>> mapping of the UEFI System table using read-only attributes, as a
>> hardening measure against inadvertent modification.
>>
>> However, this still leaves the permanent, writable mapping of the UEFI
>> System table, which is only ever referenced during invocations of UEFI
>> Runtime Services, at which time the UEFI virtual mapping is available,
>> which also covers the system table. (This is guaranteed by the fact that
>> SetVirtualAddressMap(), which is a runtime service itself, converts
>> various entries in the table to their virtual equivalents, which implies
>> that the table must be covered by a RuntimeServicesData region that has
>> the EFI_MEMORY_RUNTIME attribute.)
>>
>> So instead of creating this permanent mapping, record the virtual address
>> of the system table inside the UEFI virtual mapping, and dereference that
>> when accessing the table. This protects the contents of the system table
>> from inadvertent (or deliberate) modification when no UEFI Runtime
>> Services calls are in progress.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  drivers/firmware/efi/arm-init.c    |  2 ++
>>  drivers/firmware/efi/arm-runtime.c | 27 ++++++++++++++++-----------
>>  2 files changed, 18 insertions(+), 11 deletions(-)
>
> Looks like a nice cleanup. Applied.

This patch causes a warning I hadn't spotted before sending it out.
Could you fold this in please?

diff --git a/drivers/firmware/efi/arm-runtime.c
b/drivers/firmware/efi/arm-runtime.c
index 6c97d4884fc7..fe749da9997e 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -72,8 +72,8 @@ static bool __init efi_virtmap_init(void)
                 */
                if (efi_system_table >= phys &&
                    efi_system_table < phys + (md->num_pages * EFI_PAGE_SIZE)) {
-                       efi.systab = (void *)(efi_system_table - phys +
-                                             md->virt_addr);
+                       efi.systab = (void *)(unsigned long)(efi_system_table -
+                                                         phys + md->virt_addr);
                        systab_found = true;
                }
        }

Thanks,
Ard.

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

* Re: [PATCH 2/2] arm*: efi: drop writable mapping of the UEFI System table
  2016-03-22 15:08             ` Ard Biesheuvel
@ 2016-03-23 16:16                 ` Matt Fleming
  -1 siblings, 0 replies; 14+ messages in thread
From: Matt Fleming @ 2016-03-23 16:16 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-efi-u79uwXL29TY76Z2rM5mHXA, Leif Lindholm, Mark Rutland

On Tue, 22 Mar, at 04:08:24PM, Ard Biesheuvel wrote:
> 
> This patch causes a warning I hadn't spotted before sending it out.
> Could you fold this in please?
> 
> diff --git a/drivers/firmware/efi/arm-runtime.c
> b/drivers/firmware/efi/arm-runtime.c
> index 6c97d4884fc7..fe749da9997e 100644
> --- a/drivers/firmware/efi/arm-runtime.c
> +++ b/drivers/firmware/efi/arm-runtime.c
> @@ -72,8 +72,8 @@ static bool __init efi_virtmap_init(void)
>                  */
>                 if (efi_system_table >= phys &&
>                     efi_system_table < phys + (md->num_pages * EFI_PAGE_SIZE)) {
> -                       efi.systab = (void *)(efi_system_table - phys +
> -                                             md->virt_addr);
> +                       efi.systab = (void *)(unsigned long)(efi_system_table -
> +                                                         phys + md->virt_addr);
>                         systab_found = true;
>                 }
>         }

Done, please check,

  https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/commit/?h=next&id=4fd5c619dcf4f3d2c86d4a686ce66fc01e877a08

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

* [PATCH 2/2] arm*: efi: drop writable mapping of the UEFI System table
@ 2016-03-23 16:16                 ` Matt Fleming
  0 siblings, 0 replies; 14+ messages in thread
From: Matt Fleming @ 2016-03-23 16:16 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 22 Mar, at 04:08:24PM, Ard Biesheuvel wrote:
> 
> This patch causes a warning I hadn't spotted before sending it out.
> Could you fold this in please?
> 
> diff --git a/drivers/firmware/efi/arm-runtime.c
> b/drivers/firmware/efi/arm-runtime.c
> index 6c97d4884fc7..fe749da9997e 100644
> --- a/drivers/firmware/efi/arm-runtime.c
> +++ b/drivers/firmware/efi/arm-runtime.c
> @@ -72,8 +72,8 @@ static bool __init efi_virtmap_init(void)
>                  */
>                 if (efi_system_table >= phys &&
>                     efi_system_table < phys + (md->num_pages * EFI_PAGE_SIZE)) {
> -                       efi.systab = (void *)(efi_system_table - phys +
> -                                             md->virt_addr);
> +                       efi.systab = (void *)(unsigned long)(efi_system_table -
> +                                                         phys + md->virt_addr);
>                         systab_found = true;
>                 }
>         }

Done, please check,

  https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/commit/?h=next&id=4fd5c619dcf4f3d2c86d4a686ce66fc01e877a08

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

* Re: [PATCH 2/2] arm*: efi: drop writable mapping of the UEFI System table
  2016-03-23 16:16                 ` Matt Fleming
@ 2016-03-23 22:11                     ` Ard Biesheuvel
  -1 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2016-03-23 22:11 UTC (permalink / raw)
  To: Matt Fleming
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-efi-u79uwXL29TY76Z2rM5mHXA, Leif Lindholm, Mark Rutland

On 23 March 2016 at 17:16, Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:
> On Tue, 22 Mar, at 04:08:24PM, Ard Biesheuvel wrote:
>>
>> This patch causes a warning I hadn't spotted before sending it out.
>> Could you fold this in please?
>>
>> diff --git a/drivers/firmware/efi/arm-runtime.c
>> b/drivers/firmware/efi/arm-runtime.c
>> index 6c97d4884fc7..fe749da9997e 100644
>> --- a/drivers/firmware/efi/arm-runtime.c
>> +++ b/drivers/firmware/efi/arm-runtime.c
>> @@ -72,8 +72,8 @@ static bool __init efi_virtmap_init(void)
>>                  */
>>                 if (efi_system_table >= phys &&
>>                     efi_system_table < phys + (md->num_pages * EFI_PAGE_SIZE)) {
>> -                       efi.systab = (void *)(efi_system_table - phys +
>> -                                             md->virt_addr);
>> +                       efi.systab = (void *)(unsigned long)(efi_system_table -
>> +                                                         phys + md->virt_addr);
>>                         systab_found = true;
>>                 }
>>         }
>
> Done, please check,
>
>   https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/commit/?h=next&id=4fd5c619dcf4f3d2c86d4a686ce66fc01e877a08

Looks good to me, thanks.

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

* [PATCH 2/2] arm*: efi: drop writable mapping of the UEFI System table
@ 2016-03-23 22:11                     ` Ard Biesheuvel
  0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2016-03-23 22:11 UTC (permalink / raw)
  To: linux-arm-kernel

On 23 March 2016 at 17:16, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> On Tue, 22 Mar, at 04:08:24PM, Ard Biesheuvel wrote:
>>
>> This patch causes a warning I hadn't spotted before sending it out.
>> Could you fold this in please?
>>
>> diff --git a/drivers/firmware/efi/arm-runtime.c
>> b/drivers/firmware/efi/arm-runtime.c
>> index 6c97d4884fc7..fe749da9997e 100644
>> --- a/drivers/firmware/efi/arm-runtime.c
>> +++ b/drivers/firmware/efi/arm-runtime.c
>> @@ -72,8 +72,8 @@ static bool __init efi_virtmap_init(void)
>>                  */
>>                 if (efi_system_table >= phys &&
>>                     efi_system_table < phys + (md->num_pages * EFI_PAGE_SIZE)) {
>> -                       efi.systab = (void *)(efi_system_table - phys +
>> -                                             md->virt_addr);
>> +                       efi.systab = (void *)(unsigned long)(efi_system_table -
>> +                                                         phys + md->virt_addr);
>>                         systab_found = true;
>>                 }
>>         }
>
> Done, please check,
>
>   https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/commit/?h=next&id=4fd5c619dcf4f3d2c86d4a686ce66fc01e877a08

Looks good to me, thanks.

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

end of thread, other threads:[~2016-03-23 22:11 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-26 14:20 [PATCH 1/2] efi: get rid of EFI_SYSTEM_TABLES status bit Ard Biesheuvel
2016-02-26 14:20 ` Ard Biesheuvel
     [not found] ` <1456496435-12679-1-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2016-02-26 14:20   ` [PATCH 2/2] arm*: efi: drop writable mapping of the UEFI System table Ard Biesheuvel
2016-02-26 14:20     ` Ard Biesheuvel
     [not found]     ` <1456496435-12679-2-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2016-02-26 15:01       ` Matt Fleming
2016-02-26 15:01         ` Matt Fleming
     [not found]         ` <20160226150156.GC7475-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
2016-03-22 15:08           ` Ard Biesheuvel
2016-03-22 15:08             ` Ard Biesheuvel
     [not found]             ` <CAKv+Gu8+pYR_jxjcVeXgekVqD_BugtRLgNNhOkvhuaEpddbHRg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-03-23 16:16               ` Matt Fleming
2016-03-23 16:16                 ` Matt Fleming
     [not found]                 ` <20160323161624.GG11676-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
2016-03-23 22:11                   ` Ard Biesheuvel
2016-03-23 22:11                     ` Ard Biesheuvel
2016-02-26 14:43   ` [PATCH 1/2] efi: get rid of EFI_SYSTEM_TABLES status bit Matt Fleming
2016-02-26 14:43     ` Matt Fleming

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.