Re: [PATCH] TCG2 log support build fixes for non-x86_64
On Thu, Apr 4, 2019 at 5:41 AM Jarkko Sakkinen wrote: > > On Tue, Apr 02, 2019 at 02:55:54PM -0700, Matthew Garrett wrote: > > Couple of patches to fix ktest reported issues with the crypto-agile log > > format support. > > I guess I squash these to your earlier commits? Works for me.
Re: [PATCH 1/2] efi: Fix cast to pointer from integer of different size in TPM log code
On Thu, Apr 4, 2019 at 5:54 AM Ard Biesheuvel wrote: > > On Wed, 3 Apr 2019 at 04:56, Matthew Garrett > wrote: > > > > 8bfcff4a6a1d9d7226bb63a7da758b82d9ab4373 > > Which tree is this commit in? Sorry, these are in the tpmdd-next tree.
Re: [PATCH] efi: Check the number of EFI configuration tables entries
On Thu, 4 Apr 2019 at 18:24, Rob Bradford wrote: > > Only try and access the EFI configuration tables if there there are any > reported. This allows EFI to be continued to used on systems where there > are no configuration table entries. > > Signed-off-by: Rob Bradford > --- > arch/x86/platform/efi/quirks.c | 3 +++ > drivers/firmware/efi/efi.c | 3 +++ > 2 files changed, 6 insertions(+) > > diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c > index 95e77a667ba5..a6d0e6afee8f 100644 > --- a/arch/x86/platform/efi/quirks.c > +++ b/arch/x86/platform/efi/quirks.c > @@ -486,6 +486,9 @@ int __init efi_reuse_config(u64 tables, int nr_tables) > if (!data->smbios) > goto out_memremap; > > + if (nr_tables == 0) > + goto out_memremap; > + So what is the point of executing the function all the way up to this point? Can't we just return immediately at the beginning? > sz = sizeof(efi_config_table_64_t); > > p = tablep = early_memremap(tables, nr_tables * sz); > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c > index 415849bab233..185424a8b879 100644 > --- a/drivers/firmware/efi/efi.c > +++ b/drivers/firmware/efi/efi.c > @@ -628,6 +628,9 @@ int __init efi_config_init(efi_config_table_type_t > *arch_tables) > void *config_tables; > int sz, ret; > > + if (efi.systab->nr_tables == 0) > + return 0; > + > if (efi_enabled(EFI_64BIT)) > sz = sizeof(efi_config_table_64_t); > else > -- > 2.20.1 >
Re: [PATCH 1/2] efi: Fix cast to pointer from integer of different size in TPM log code
On Wed, 3 Apr 2019 at 04:56, Matthew Garrett wrote:
>
> 8bfcff4a6a1d9d7226bb63a7da758b82d9ab4373
Which tree is this commit in?
> introduced a cast from
> efi_physical_address_t to (void *), which are different sizes on 32-bit.
> Fix that. Caught by the 0-day test bot.
>
> Signed-off-by: Matthew Garrett
Acked-by: Ard Biesheuvel
I'll pick this up as a fix if we can clean up the commit log so it
doesn't refer to a commit that does not exist in mainline.
> ---
> drivers/firmware/efi/libstub/tpm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/tpm.c
> b/drivers/firmware/efi/libstub/tpm.c
> index b6e93e14fcf1..6b3b507a54eb 100644
> --- a/drivers/firmware/efi/libstub/tpm.c
> +++ b/drivers/firmware/efi/libstub/tpm.c
> @@ -114,8 +114,8 @@ void efi_retrieve_tpm2_eventlog(efi_system_table_t
> *sys_table_arg)
> */
> last_entry_size =
> __calc_tpm2_event_size((void
> *)last_entry_addr,
> - (void *)log_location,
> - false);
> + (void
> *)(long)log_location,
> + false);
> } else {
> last_entry_size = sizeof(struct tcpa_event) +
>((struct tcpa_event *)
> last_entry_addr)->event_size;
> --
> 2.21.0.392.gf8f6787159e-goog
>
Re: [PATCH] efi: Include tpm_eventlog.h after asm/efi.h to avoid memcpy breakage
On Wed, 3 Apr 2019 at 21:32, Matthew Garrett wrote: > > 769a8089c1fd2 (x86, efi, kasan: #undef memset/memcpy/memmove per arch) > disables the KASAN version of certain memory calls in the EFI boot stub. > tpm_eventlog.h references memcpy, so must be included after asm/efi.h in > order to allow 769a8089c1fd2 to take effect on its declarations. > I can't find any reference to memcpy in tpm_eventlog.h or tpm.h which it includes. So can you describe the exact problem? If the issue is due to one of the transitively included headers two levels down, then this is fragile as hell since the same header may be included by linux/efi.h due to, e.g., minor config changes. > Signed-off-by: Matthew Garrett > --- > drivers/firmware/efi/libstub/tpm.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/firmware/efi/libstub/tpm.c > b/drivers/firmware/efi/libstub/tpm.c > index b6e93e14fcf1..777c1e82495a 100644 > --- a/drivers/firmware/efi/libstub/tpm.c > +++ b/drivers/firmware/efi/libstub/tpm.c > @@ -8,9 +8,11 @@ > * Thiebaud Weksteen > */ > #include > -#include > #include > > +/* Must be included after asm/efi.h in order to avoid using the wrong memcpy > */ > +#include > + > #include "efistub.h" > > #ifdef CONFIG_RESET_ATTACK_MITIGATION > -- > 2.21.0.392.gf8f6787159e-goog >
Re: [PATCH] efi: Include tpm_eventlog.h after asm/efi.h to avoid memcpy breakage
On Wed, Apr 03, 2019 at 12:32:37PM -0700, Matthew Garrett wrote: > 769a8089c1fd2 (x86, efi, kasan: #undef memset/memcpy/memmove per arch) > disables the KASAN version of certain memory calls in the EFI boot stub. > tpm_eventlog.h references memcpy, so must be included after asm/efi.h in > order to allow 769a8089c1fd2 to take effect on its declarations. > > Signed-off-by: Matthew Garrett Reviewed-by: Jarkko Sakkinen /Jarkko
Re: [PATCH] TCG2 log support build fixes for non-x86_64
On Tue, Apr 02, 2019 at 02:55:54PM -0700, Matthew Garrett wrote: > Couple of patches to fix ktest reported issues with the crypto-agile log > format support. I guess I squash these to your earlier commits? /Jarkko
Re: [PATCH 2/2] tpm: Fix builds on platforms that lack early_memremap()
On Tue, Apr 02, 2019 at 02:55:56PM -0700, Matthew Garrett wrote: > On EFI systems, __calc_tpm2_event_size() needs to be able to map tables > at early boot time in order to extract information from them. > Unfortunately this interacts badly with other architectures that don't > provide the early_memremap() interface but which may still have other > mechanisms for obtaining crypto-agile logs. Abstract this away so we > can avoid the need for two implementations while still avoiding breakage > on architectures that don't require remapping of the table. > > Signed-off-by: Matthew Garrett Reviewed-by: Jarkko Sakkinen /Jarkko
Re: [PATCH 1/2] efi: Fix cast to pointer from integer of different size in TPM log code
On Tue, Apr 02, 2019 at 02:55:55PM -0700, Matthew Garrett wrote: > 8bfcff4a6a1d9d7226bb63a7da758b82d9ab4373 introduced a cast from > efi_physical_address_t to (void *), which are different sizes on 32-bit. > Fix that. Caught by the 0-day test bot. > > Signed-off-by: Matthew Garrett Reviewed-by: Jarkko Sakkinen /Jarkko
[PATCH] efi: Check the number of EFI configuration tables entries
Only try and access the EFI configuration tables if there there are any reported. This allows EFI to be continued to used on systems where there are no configuration table entries. Signed-off-by: Rob Bradford --- arch/x86/platform/efi/quirks.c | 3 +++ drivers/firmware/efi/efi.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c index 95e77a667ba5..a6d0e6afee8f 100644 --- a/arch/x86/platform/efi/quirks.c +++ b/arch/x86/platform/efi/quirks.c @@ -486,6 +486,9 @@ int __init efi_reuse_config(u64 tables, int nr_tables) if (!data->smbios) goto out_memremap; + if (nr_tables == 0) + goto out_memremap; + sz = sizeof(efi_config_table_64_t); p = tablep = early_memremap(tables, nr_tables * sz); diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index 415849bab233..185424a8b879 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -628,6 +628,9 @@ int __init efi_config_init(efi_config_table_type_t *arch_tables) void *config_tables; int sz, ret; + if (efi.systab->nr_tables == 0) + return 0; + if (efi_enabled(EFI_64BIT)) sz = sizeof(efi_config_table_64_t); else -- 2.20.1
[PATCH 4.9 67/91] efi/memattr: Dont bail on zero VA if it equals the regions PA
4.9-stable review patch. If anyone has any objections, please let me know.
--
[ Upstream commit 5de0fef0230f3c8d75cff450a71740a7bf2db866 ]
The EFI memory attributes code cross-references the EFI memory map with
the more granular EFI memory attributes table to ensure that they are in
sync before applying the strict permissions to the regions it describes.
Since we always install virtual mappings for the EFI runtime regions to
which these strict permissions apply, we currently perform a sanity check
on the EFI memory descriptor, and ensure that the EFI_MEMORY_RUNTIME bit
is set, and that the virtual address has been assigned.
However, in cases where a runtime region exists at physical address 0x0,
and the virtual mapping equals the physical mapping, e.g., when running
in mixed mode on x86, we encounter a memory descriptor with the runtime
attribute and virtual address 0x0, and incorrectly draw the conclusion
that a runtime region exists for which no virtual mapping was installed,
and give up altogether. The consequence of this is that firmware mappings
retain their read-write-execute permissions, making the system more
vulnerable to attacks.
So let's only bail if the virtual address of 0x0 has been assigned to a
physical region that does not reside at address 0x0.
Signed-off-by: Ard Biesheuvel
Acked-by: Sai Praneeth Prakhya
Cc: AKASHI Takahiro
Cc: Alexander Graf
Cc: Bjorn Andersson
Cc: Borislav Petkov
Cc: Heinrich Schuchardt
Cc: Jeffrey Hugo
Cc: Lee Jones
Cc: Leif Lindholm
Cc: Linus Torvalds
Cc: Matt Fleming
Cc: Peter Jones
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: linux-efi@vger.kernel.org
Fixes: 10f0d2f577053 ("efi: Implement generic support for the Memory ...")
Link: http://lkml.kernel.org/r/20190202094119.13230-4-ard.biesheu...@linaro.org
Signed-off-by: Ingo Molnar
Signed-off-by: Sasha Levin
---
drivers/firmware/efi/memattr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
index 236004b9a50d..9faa09e7c31f 100644
--- a/drivers/firmware/efi/memattr.c
+++ b/drivers/firmware/efi/memattr.c
@@ -93,7 +93,7 @@ static bool entry_is_valid(const efi_memory_desc_t *in,
efi_memory_desc_t *out)
if (!(md->attribute & EFI_MEMORY_RUNTIME))
continue;
- if (md->virt_addr == 0) {
+ if (md->virt_addr == 0 && md->phys_addr != 0) {
/* no virtual mapping has been installed by the stub */
break;
}
--
2.19.1
[PATCH 4.14 087/121] efi/memattr: Dont bail on zero VA if it equals the regions PA
4.14-stable review patch. If anyone has any objections, please let me know.
--
[ Upstream commit 5de0fef0230f3c8d75cff450a71740a7bf2db866 ]
The EFI memory attributes code cross-references the EFI memory map with
the more granular EFI memory attributes table to ensure that they are in
sync before applying the strict permissions to the regions it describes.
Since we always install virtual mappings for the EFI runtime regions to
which these strict permissions apply, we currently perform a sanity check
on the EFI memory descriptor, and ensure that the EFI_MEMORY_RUNTIME bit
is set, and that the virtual address has been assigned.
However, in cases where a runtime region exists at physical address 0x0,
and the virtual mapping equals the physical mapping, e.g., when running
in mixed mode on x86, we encounter a memory descriptor with the runtime
attribute and virtual address 0x0, and incorrectly draw the conclusion
that a runtime region exists for which no virtual mapping was installed,
and give up altogether. The consequence of this is that firmware mappings
retain their read-write-execute permissions, making the system more
vulnerable to attacks.
So let's only bail if the virtual address of 0x0 has been assigned to a
physical region that does not reside at address 0x0.
Signed-off-by: Ard Biesheuvel
Acked-by: Sai Praneeth Prakhya
Cc: AKASHI Takahiro
Cc: Alexander Graf
Cc: Bjorn Andersson
Cc: Borislav Petkov
Cc: Heinrich Schuchardt
Cc: Jeffrey Hugo
Cc: Lee Jones
Cc: Leif Lindholm
Cc: Linus Torvalds
Cc: Matt Fleming
Cc: Peter Jones
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: linux-efi@vger.kernel.org
Fixes: 10f0d2f577053 ("efi: Implement generic support for the Memory ...")
Link: http://lkml.kernel.org/r/20190202094119.13230-4-ard.biesheu...@linaro.org
Signed-off-by: Ingo Molnar
Signed-off-by: Sasha Levin
---
drivers/firmware/efi/memattr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
index 8986757eafaf..aac972b056d9 100644
--- a/drivers/firmware/efi/memattr.c
+++ b/drivers/firmware/efi/memattr.c
@@ -94,7 +94,7 @@ static bool entry_is_valid(const efi_memory_desc_t *in,
efi_memory_desc_t *out)
if (!(md->attribute & EFI_MEMORY_RUNTIME))
continue;
- if (md->virt_addr == 0) {
+ if (md->virt_addr == 0 && md->phys_addr != 0) {
/* no virtual mapping has been installed by the stub */
break;
}
--
2.19.1
[PATCH 4.14 089/121] efi/arm/arm64: Allow SetVirtualAddressMap() to be omitted
4.14-stable review patch. If anyone has any objections, please let me know.
--
[ Upstream commit 4e46c2a956215482418d7b315749fb1b6c6bc224 ]
The UEFI spec revision 2.7 errata A section 8.4 has the following to
say about the virtual memory runtime services:
"This section contains function definitions for the virtual memory
support that may be optionally used by an operating system at runtime.
If an operating system chooses to make EFI runtime service calls in a
virtual addressing mode instead of the flat physical mode, then the
operating system must use the services in this section to switch the
EFI runtime services from flat physical addressing to virtual
addressing."
So it is pretty clear that calling SetVirtualAddressMap() is entirely
optional, and so there is no point in doing so unless it achieves
anything useful for us.
This is not the case for 64-bit ARM. The identity mapping used by the
firmware is arbitrarily converted into another permutation of userland
addresses (i.e., bits [63:48] cleared), and the runtime code could easily
deal with the original layout in exactly the same way as it deals with
the converted layout. However, due to constraints related to page size
differences if the OS is not running with 4k pages, and related to
systems that may expose the individual sections of PE/COFF runtime
modules as different memory regions, creating the virtual layout is a
bit fiddly, and requires us to sort the memory map and reason about
adjacent regions with identical memory types etc etc.
So the obvious fix is to stop calling SetVirtualAddressMap() altogether
on arm64 systems. However, to avoid surprises, which are notoriously
hard to diagnose when it comes to OS<->firmware interactions, let's
start by making it an opt-out feature, and implement support for the
'efi=novamap' kernel command line parameter on ARM and arm64 systems.
( Note that 32-bit ARM generally does require SetVirtualAddressMap() to be
used, given that the physical memory map and the kernel virtual address
map are not guaranteed to be non-overlapping like on arm64. However,
having support for efi=novamap,noruntime on 32-bit ARM, combined with
the recently proposed support for earlycon=efifb, is likely to be useful
to diagnose boot issues on such systems if they have no accessible serial
port. )
Tested-by: Jeffrey Hugo
Tested-by: Bjorn Andersson
Tested-by: Lee Jones
Signed-off-by: Ard Biesheuvel
Cc: AKASHI Takahiro
Cc: Alexander Graf
Cc: Borislav Petkov
Cc: Heinrich Schuchardt
Cc: Leif Lindholm
Cc: Linus Torvalds
Cc: Matt Fleming
Cc: Peter Jones
Cc: Peter Zijlstra
Cc: Sai Praneeth Prakhya
Cc: Thomas Gleixner
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20190202094119.13230-8-ard.biesheu...@linaro.org
Signed-off-by: Ingo Molnar
Signed-off-by: Sasha Levin
---
drivers/firmware/efi/libstub/arm-stub.c| 5 +
drivers/firmware/efi/libstub/efi-stub-helper.c | 10 ++
drivers/firmware/efi/libstub/efistub.h | 1 +
drivers/firmware/efi/libstub/fdt.c | 3 +++
4 files changed, 19 insertions(+)
diff --git a/drivers/firmware/efi/libstub/arm-stub.c
b/drivers/firmware/efi/libstub/arm-stub.c
index 01a9d78ee415..3b1e1dc3fb46 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -364,6 +364,11 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map,
unsigned long map_size,
paddr = in->phys_addr;
size = in->num_pages * EFI_PAGE_SIZE;
+ if (novamap()) {
+ in->virt_addr = in->phys_addr;
+ continue;
+ }
+
/*
* Make the mapping compatible with 64k pages: this allows
* a 4k page size kernel to kexec a 64k page size kernel and
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c
b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 50a9cab5a834..39f87e6dac5c 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -34,6 +34,7 @@ static unsigned long __chunk_size = EFI_READ_CHUNK_SIZE;
static int __section(.data) __nokaslr;
static int __section(.data) __quiet;
+static int __section(.data) __novamap;
int __pure nokaslr(void)
{
@@ -43,6 +44,10 @@ int __pure is_quiet(void)
{
return __quiet;
}
+int __pure novamap(void)
+{
+ return __novamap;
+}
#define EFI_MMAP_NR_SLACK_SLOTS8
@@ -454,6 +459,11 @@ efi_status_t efi_parse_options(char const *cmdline)
__chunk_size = -1UL;
}
+ if (!strncmp(str, "novamap", 7)) {
+ str += strlen("novamap");
+ __novamap = 1;
+ }
+
/* Group words together, delimited by "," */
while (*str && *str != ' ' && *str != ',')
str++;
diff --git
[PATCH 4.19 137/187] efi/arm/arm64: Allow SetVirtualAddressMap() to be omitted
4.19-stable review patch. If anyone has any objections, please let me know.
--
[ Upstream commit 4e46c2a956215482418d7b315749fb1b6c6bc224 ]
The UEFI spec revision 2.7 errata A section 8.4 has the following to
say about the virtual memory runtime services:
"This section contains function definitions for the virtual memory
support that may be optionally used by an operating system at runtime.
If an operating system chooses to make EFI runtime service calls in a
virtual addressing mode instead of the flat physical mode, then the
operating system must use the services in this section to switch the
EFI runtime services from flat physical addressing to virtual
addressing."
So it is pretty clear that calling SetVirtualAddressMap() is entirely
optional, and so there is no point in doing so unless it achieves
anything useful for us.
This is not the case for 64-bit ARM. The identity mapping used by the
firmware is arbitrarily converted into another permutation of userland
addresses (i.e., bits [63:48] cleared), and the runtime code could easily
deal with the original layout in exactly the same way as it deals with
the converted layout. However, due to constraints related to page size
differences if the OS is not running with 4k pages, and related to
systems that may expose the individual sections of PE/COFF runtime
modules as different memory regions, creating the virtual layout is a
bit fiddly, and requires us to sort the memory map and reason about
adjacent regions with identical memory types etc etc.
So the obvious fix is to stop calling SetVirtualAddressMap() altogether
on arm64 systems. However, to avoid surprises, which are notoriously
hard to diagnose when it comes to OS<->firmware interactions, let's
start by making it an opt-out feature, and implement support for the
'efi=novamap' kernel command line parameter on ARM and arm64 systems.
( Note that 32-bit ARM generally does require SetVirtualAddressMap() to be
used, given that the physical memory map and the kernel virtual address
map are not guaranteed to be non-overlapping like on arm64. However,
having support for efi=novamap,noruntime on 32-bit ARM, combined with
the recently proposed support for earlycon=efifb, is likely to be useful
to diagnose boot issues on such systems if they have no accessible serial
port. )
Tested-by: Jeffrey Hugo
Tested-by: Bjorn Andersson
Tested-by: Lee Jones
Signed-off-by: Ard Biesheuvel
Cc: AKASHI Takahiro
Cc: Alexander Graf
Cc: Borislav Petkov
Cc: Heinrich Schuchardt
Cc: Leif Lindholm
Cc: Linus Torvalds
Cc: Matt Fleming
Cc: Peter Jones
Cc: Peter Zijlstra
Cc: Sai Praneeth Prakhya
Cc: Thomas Gleixner
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20190202094119.13230-8-ard.biesheu...@linaro.org
Signed-off-by: Ingo Molnar
Signed-off-by: Sasha Levin
---
drivers/firmware/efi/libstub/arm-stub.c| 5 +
drivers/firmware/efi/libstub/efi-stub-helper.c | 10 ++
drivers/firmware/efi/libstub/efistub.h | 1 +
drivers/firmware/efi/libstub/fdt.c | 3 +++
4 files changed, 19 insertions(+)
diff --git a/drivers/firmware/efi/libstub/arm-stub.c
b/drivers/firmware/efi/libstub/arm-stub.c
index 6920033de6d4..6c09644d620e 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -340,6 +340,11 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map,
unsigned long map_size,
paddr = in->phys_addr;
size = in->num_pages * EFI_PAGE_SIZE;
+ if (novamap()) {
+ in->virt_addr = in->phys_addr;
+ continue;
+ }
+
/*
* Make the mapping compatible with 64k pages: this allows
* a 4k page size kernel to kexec a 64k page size kernel and
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c
b/drivers/firmware/efi/libstub/efi-stub-helper.c
index e94975f4655b..442f51c2a53d 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -34,6 +34,7 @@ static unsigned long __chunk_size = EFI_READ_CHUNK_SIZE;
static int __section(.data) __nokaslr;
static int __section(.data) __quiet;
+static int __section(.data) __novamap;
int __pure nokaslr(void)
{
@@ -43,6 +44,10 @@ int __pure is_quiet(void)
{
return __quiet;
}
+int __pure novamap(void)
+{
+ return __novamap;
+}
#define EFI_MMAP_NR_SLACK_SLOTS8
@@ -482,6 +487,11 @@ efi_status_t efi_parse_options(char const *cmdline)
__chunk_size = -1UL;
}
+ if (!strncmp(str, "novamap", 7)) {
+ str += strlen("novamap");
+ __novamap = 1;
+ }
+
/* Group words together, delimited by "," */
while (*str && *str != ' ' && *str != ',')
str++;
diff --git
[PATCH 5.0 126/246] efi: Fix build error due to enum collision between efi.h and ima.h
5.0-stable review patch. If anyone has any objections, please let me know.
--
[ Upstream commit 5c418dc789a3898717ebf2caa5716ba91a7150b2 ]
The following commit:
a893ea15d764 ("tpm: move tpm_chip definition to include/linux/tpm.h")
introduced a build error when both IMA and EFI are enabled:
In file included from ../security/integrity/ima/ima_fs.c:30:
../security/integrity/ima/ima.h:176:7: error: redeclaration of enumerator
"NONE"
What happens is that both headers (ima.h and efi.h) defines the same
'NONE' constant, and it broke when they started getting included from
the same file:
Rework to prefix the EFI enum with 'EFI_*'.
Signed-off-by: Anders Roxell
Signed-off-by: Ard Biesheuvel
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20190215165551.12220-2-ard.biesheu...@linaro.org
[ Cleaned up the changelog a bit. ]
Signed-off-by: Ingo Molnar
Signed-off-by: Sasha Levin
---
arch/x86/platform/efi/quirks.c | 4 +--
drivers/firmware/efi/runtime-wrappers.c | 48 -
include/linux/efi.h | 26 +++---
3 files changed, 39 insertions(+), 39 deletions(-)
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 17456a1d3f04..6c571ae86947 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -717,7 +717,7 @@ void efi_recover_from_page_fault(unsigned long phys_addr)
* "efi_mm" cannot be used to check if the page fault had occurred
* in the firmware context because efi=old_map doesn't use efi_pgd.
*/
- if (efi_rts_work.efi_rts_id == NONE)
+ if (efi_rts_work.efi_rts_id == EFI_NONE)
return;
/*
@@ -742,7 +742,7 @@ void efi_recover_from_page_fault(unsigned long phys_addr)
* because this case occurs *very* rarely and hence could be improved
* on a need by basis.
*/
- if (efi_rts_work.efi_rts_id == RESET_SYSTEM) {
+ if (efi_rts_work.efi_rts_id == EFI_RESET_SYSTEM) {
pr_info("efi_reset_system() buggy! Reboot through BIOS\n");
machine_real_restart(MRR_BIOS);
return;
diff --git a/drivers/firmware/efi/runtime-wrappers.c
b/drivers/firmware/efi/runtime-wrappers.c
index e2abfdb5cee6..698745c249e8 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -85,7 +85,7 @@ struct efi_runtime_work efi_rts_work;
pr_err("Failed to queue work to efi_rts_wq.\n");\
\
exit: \
- efi_rts_work.efi_rts_id = NONE; \
+ efi_rts_work.efi_rts_id = EFI_NONE; \
efi_rts_work.status;\
})
@@ -175,50 +175,50 @@ static void efi_call_rts(struct work_struct *work)
arg5 = efi_rts_work.arg5;
switch (efi_rts_work.efi_rts_id) {
- case GET_TIME:
+ case EFI_GET_TIME:
status = efi_call_virt(get_time, (efi_time_t *)arg1,
(efi_time_cap_t *)arg2);
break;
- case SET_TIME:
+ case EFI_SET_TIME:
status = efi_call_virt(set_time, (efi_time_t *)arg1);
break;
- case GET_WAKEUP_TIME:
+ case EFI_GET_WAKEUP_TIME:
status = efi_call_virt(get_wakeup_time, (efi_bool_t *)arg1,
(efi_bool_t *)arg2, (efi_time_t *)arg3);
break;
- case SET_WAKEUP_TIME:
+ case EFI_SET_WAKEUP_TIME:
status = efi_call_virt(set_wakeup_time, *(efi_bool_t *)arg1,
(efi_time_t *)arg2);
break;
- case GET_VARIABLE:
+ case EFI_GET_VARIABLE:
status = efi_call_virt(get_variable, (efi_char16_t *)arg1,
(efi_guid_t *)arg2, (u32 *)arg3,
(unsigned long *)arg4, (void *)arg5);
break;
- case GET_NEXT_VARIABLE:
+ case EFI_GET_NEXT_VARIABLE:
status = efi_call_virt(get_next_variable, (unsigned long *)arg1,
(efi_char16_t *)arg2,
(efi_guid_t *)arg3);
break;
- case SET_VARIABLE:
+ case EFI_SET_VARIABLE:
status = efi_call_virt(set_variable, (efi_char16_t *)arg1,
(efi_guid_t *)arg2, *(u32 *)arg3,
*(unsigned long *)arg4, (void *)arg5);
break;
- case QUERY_VARIABLE_INFO:
+ case EFI_QUERY_VARIABLE_INFO:
status = efi_call_virt(query_variable_info,
[PATCH 5.0 175/246] efi/memattr: Dont bail on zero VA if it equals the regions PA
5.0-stable review patch. If anyone has any objections, please let me know.
--
[ Upstream commit 5de0fef0230f3c8d75cff450a71740a7bf2db866 ]
The EFI memory attributes code cross-references the EFI memory map with
the more granular EFI memory attributes table to ensure that they are in
sync before applying the strict permissions to the regions it describes.
Since we always install virtual mappings for the EFI runtime regions to
which these strict permissions apply, we currently perform a sanity check
on the EFI memory descriptor, and ensure that the EFI_MEMORY_RUNTIME bit
is set, and that the virtual address has been assigned.
However, in cases where a runtime region exists at physical address 0x0,
and the virtual mapping equals the physical mapping, e.g., when running
in mixed mode on x86, we encounter a memory descriptor with the runtime
attribute and virtual address 0x0, and incorrectly draw the conclusion
that a runtime region exists for which no virtual mapping was installed,
and give up altogether. The consequence of this is that firmware mappings
retain their read-write-execute permissions, making the system more
vulnerable to attacks.
So let's only bail if the virtual address of 0x0 has been assigned to a
physical region that does not reside at address 0x0.
Signed-off-by: Ard Biesheuvel
Acked-by: Sai Praneeth Prakhya
Cc: AKASHI Takahiro
Cc: Alexander Graf
Cc: Bjorn Andersson
Cc: Borislav Petkov
Cc: Heinrich Schuchardt
Cc: Jeffrey Hugo
Cc: Lee Jones
Cc: Leif Lindholm
Cc: Linus Torvalds
Cc: Matt Fleming
Cc: Peter Jones
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: linux-efi@vger.kernel.org
Fixes: 10f0d2f577053 ("efi: Implement generic support for the Memory ...")
Link: http://lkml.kernel.org/r/20190202094119.13230-4-ard.biesheu...@linaro.org
Signed-off-by: Ingo Molnar
Signed-off-by: Sasha Levin
---
drivers/firmware/efi/memattr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
index 8986757eafaf..aac972b056d9 100644
--- a/drivers/firmware/efi/memattr.c
+++ b/drivers/firmware/efi/memattr.c
@@ -94,7 +94,7 @@ static bool entry_is_valid(const efi_memory_desc_t *in,
efi_memory_desc_t *out)
if (!(md->attribute & EFI_MEMORY_RUNTIME))
continue;
- if (md->virt_addr == 0) {
+ if (md->virt_addr == 0 && md->phys_addr != 0) {
/* no virtual mapping has been installed by the stub */
break;
}
--
2.19.1
[PATCH 5.0 180/246] efi/arm/arm64: Allow SetVirtualAddressMap() to be omitted
5.0-stable review patch. If anyone has any objections, please let me know.
--
[ Upstream commit 4e46c2a956215482418d7b315749fb1b6c6bc224 ]
The UEFI spec revision 2.7 errata A section 8.4 has the following to
say about the virtual memory runtime services:
"This section contains function definitions for the virtual memory
support that may be optionally used by an operating system at runtime.
If an operating system chooses to make EFI runtime service calls in a
virtual addressing mode instead of the flat physical mode, then the
operating system must use the services in this section to switch the
EFI runtime services from flat physical addressing to virtual
addressing."
So it is pretty clear that calling SetVirtualAddressMap() is entirely
optional, and so there is no point in doing so unless it achieves
anything useful for us.
This is not the case for 64-bit ARM. The identity mapping used by the
firmware is arbitrarily converted into another permutation of userland
addresses (i.e., bits [63:48] cleared), and the runtime code could easily
deal with the original layout in exactly the same way as it deals with
the converted layout. However, due to constraints related to page size
differences if the OS is not running with 4k pages, and related to
systems that may expose the individual sections of PE/COFF runtime
modules as different memory regions, creating the virtual layout is a
bit fiddly, and requires us to sort the memory map and reason about
adjacent regions with identical memory types etc etc.
So the obvious fix is to stop calling SetVirtualAddressMap() altogether
on arm64 systems. However, to avoid surprises, which are notoriously
hard to diagnose when it comes to OS<->firmware interactions, let's
start by making it an opt-out feature, and implement support for the
'efi=novamap' kernel command line parameter on ARM and arm64 systems.
( Note that 32-bit ARM generally does require SetVirtualAddressMap() to be
used, given that the physical memory map and the kernel virtual address
map are not guaranteed to be non-overlapping like on arm64. However,
having support for efi=novamap,noruntime on 32-bit ARM, combined with
the recently proposed support for earlycon=efifb, is likely to be useful
to diagnose boot issues on such systems if they have no accessible serial
port. )
Tested-by: Jeffrey Hugo
Tested-by: Bjorn Andersson
Tested-by: Lee Jones
Signed-off-by: Ard Biesheuvel
Cc: AKASHI Takahiro
Cc: Alexander Graf
Cc: Borislav Petkov
Cc: Heinrich Schuchardt
Cc: Leif Lindholm
Cc: Linus Torvalds
Cc: Matt Fleming
Cc: Peter Jones
Cc: Peter Zijlstra
Cc: Sai Praneeth Prakhya
Cc: Thomas Gleixner
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20190202094119.13230-8-ard.biesheu...@linaro.org
Signed-off-by: Ingo Molnar
Signed-off-by: Sasha Levin
---
drivers/firmware/efi/libstub/arm-stub.c| 5 +
drivers/firmware/efi/libstub/efi-stub-helper.c | 10 ++
drivers/firmware/efi/libstub/efistub.h | 1 +
drivers/firmware/efi/libstub/fdt.c | 3 +++
4 files changed, 19 insertions(+)
diff --git a/drivers/firmware/efi/libstub/arm-stub.c
b/drivers/firmware/efi/libstub/arm-stub.c
index c037c6c5d0b7..04e6ecd72cd9 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -367,6 +367,11 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map,
unsigned long map_size,
paddr = in->phys_addr;
size = in->num_pages * EFI_PAGE_SIZE;
+ if (novamap()) {
+ in->virt_addr = in->phys_addr;
+ continue;
+ }
+
/*
* Make the mapping compatible with 64k pages: this allows
* a 4k page size kernel to kexec a 64k page size kernel and
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c
b/drivers/firmware/efi/libstub/efi-stub-helper.c
index e94975f4655b..442f51c2a53d 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -34,6 +34,7 @@ static unsigned long __chunk_size = EFI_READ_CHUNK_SIZE;
static int __section(.data) __nokaslr;
static int __section(.data) __quiet;
+static int __section(.data) __novamap;
int __pure nokaslr(void)
{
@@ -43,6 +44,10 @@ int __pure is_quiet(void)
{
return __quiet;
}
+int __pure novamap(void)
+{
+ return __novamap;
+}
#define EFI_MMAP_NR_SLACK_SLOTS8
@@ -482,6 +487,11 @@ efi_status_t efi_parse_options(char const *cmdline)
__chunk_size = -1UL;
}
+ if (!strncmp(str, "novamap", 7)) {
+ str += strlen("novamap");
+ __novamap = 1;
+ }
+
/* Group words together, delimited by "," */
while (*str && *str != ' ' && *str != ',')
str++;
diff --git
[PATCH 4.19 133/187] efi/memattr: Dont bail on zero VA if it equals the regions PA
4.19-stable review patch. If anyone has any objections, please let me know.
--
[ Upstream commit 5de0fef0230f3c8d75cff450a71740a7bf2db866 ]
The EFI memory attributes code cross-references the EFI memory map with
the more granular EFI memory attributes table to ensure that they are in
sync before applying the strict permissions to the regions it describes.
Since we always install virtual mappings for the EFI runtime regions to
which these strict permissions apply, we currently perform a sanity check
on the EFI memory descriptor, and ensure that the EFI_MEMORY_RUNTIME bit
is set, and that the virtual address has been assigned.
However, in cases where a runtime region exists at physical address 0x0,
and the virtual mapping equals the physical mapping, e.g., when running
in mixed mode on x86, we encounter a memory descriptor with the runtime
attribute and virtual address 0x0, and incorrectly draw the conclusion
that a runtime region exists for which no virtual mapping was installed,
and give up altogether. The consequence of this is that firmware mappings
retain their read-write-execute permissions, making the system more
vulnerable to attacks.
So let's only bail if the virtual address of 0x0 has been assigned to a
physical region that does not reside at address 0x0.
Signed-off-by: Ard Biesheuvel
Acked-by: Sai Praneeth Prakhya
Cc: AKASHI Takahiro
Cc: Alexander Graf
Cc: Bjorn Andersson
Cc: Borislav Petkov
Cc: Heinrich Schuchardt
Cc: Jeffrey Hugo
Cc: Lee Jones
Cc: Leif Lindholm
Cc: Linus Torvalds
Cc: Matt Fleming
Cc: Peter Jones
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: linux-efi@vger.kernel.org
Fixes: 10f0d2f577053 ("efi: Implement generic support for the Memory ...")
Link: http://lkml.kernel.org/r/20190202094119.13230-4-ard.biesheu...@linaro.org
Signed-off-by: Ingo Molnar
Signed-off-by: Sasha Levin
---
drivers/firmware/efi/memattr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
index 8986757eafaf..aac972b056d9 100644
--- a/drivers/firmware/efi/memattr.c
+++ b/drivers/firmware/efi/memattr.c
@@ -94,7 +94,7 @@ static bool entry_is_valid(const efi_memory_desc_t *in,
efi_memory_desc_t *out)
if (!(md->attribute & EFI_MEMORY_RUNTIME))
continue;
- if (md->virt_addr == 0) {
+ if (md->virt_addr == 0 && md->phys_addr != 0) {
/* no virtual mapping has been installed by the stub */
break;
}
--
2.19.1
