[PATCH 0/5] x86/efi: Re-enable memory attributes table for kexec

[PATCH 0/5] x86/efi: Re-enable memory attributes table for kexec

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

The EFI memory attributes table augments the EFI memory map, and
provides permission attributes for all runtime code and data regions
that are otherwise mapped read-write-execute in their entirety.

Currently, this table is disregarded when doing kexec boot on x86, for
two reasons:
- the boot services data region that holds the table is not reserved
  correctly, and may contain garbage at kexec time
- a misguided sanity check on the size of the table is likely to trigger
  on kexec, as the EFI memory map has been trimmed down by that time.

Fix both issues, so that the EFI memory attributes table can be taken
into account again at kexec time. Note that this requires that the call
to efi_memattr_init() is moved to a later point for x86.

Cc: Dave Young <dyoung@redhat.com>
Cc: Gregory Price <gourry@gourry.net>
Cc: Usama Arif <usamaarif642@gmail.com>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Breno Leitao <leitao@debian.org>

Ard Biesheuvel (5):
  efi/memattr: Fix thinko in table size sanity check
  x86/efi: Gather initial memory reservation and table handling logic
  x86/efi: Defer the call to efi_memattr_init()
  efi: Use efi_mem_reserve() to reserve the memory attribute table
  x86/efi: Drop kexec quirk for the EFI memory attributes table

 arch/x86/include/asm/efi.h     |  5 ++---
 arch/x86/kernel/setup.c        | 11 ++---------
 arch/x86/platform/efi/efi.c    | 15 +++++++++++++++
 arch/x86/platform/efi/quirks.c |  4 ----
 drivers/firmware/efi/efi.c     |  2 +-
 drivers/firmware/efi/memattr.c | 17 ++++++++++-------
 6 files changed, 30 insertions(+), 24 deletions(-)


base-commit: 217c0a5c177a3d4f7c8497950cbf5c36756e8bbb
-- 
2.53.0.1018.g2bb0e51243-goog

[PATCH 1/5] efi/memattr: Fix thinko in table size sanity check

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

While it is true that each PE/COFF runtime driver in memory can
generally be split into 3 different regions (the header, the code/rodata
region and the data/bss region), each with different permissions, it
does not mean that 3x the size of the memory map is a suitable upper
bound. This is due to the fact that all runtime drivers could be
coalesced into a single EFI runtime code region by the firmware, and if
the firmware does a good job of keeping the fragmentation down, it is
conceivable that the memory attributes table has more entries than the
EFI memory map itself.

So instead, base the sanity check on whether the descriptor size matches
the EFI memory map's descriptor size (which is not mandated by the spec
but extremely unlikely to differ in practice), and whether the size of
the whole table does not exceed 2 MiB.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/memattr.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
index e727cc5909cb..8d4004149297 100644
--- a/drivers/firmware/efi/memattr.c
+++ b/drivers/firmware/efi/memattr.c
@@ -42,14 +42,17 @@ void __init efi_memattr_init(void)
 
 
 	/*
-	 * Sanity check: the Memory Attributes Table contains up to 3 entries
-	 * for each entry of type EfiRuntimeServicesCode in the EFI memory map.
-	 * So if the size of the table exceeds 3x the size of the entire EFI
-	 * memory map, there is clearly something wrong, and the table should
-	 * just be ignored altogether.
+	 * Sanity check: the Memory Attributes Table contains multiple entries
+	 * for each EFI runtime services code or data region in the EFI memory
+	 * map, each with the permission attributes that may be applied when
+	 * mapping the region.  There is no upper bound for the number of
+	 * entries, as it could conceivably contain more entries than the EFI
+	 * memory map itself. So pick an arbitrary limit of 2M for the size,
+	 * which translates to ~50k entries of 40 bytes in size. This prevents
+	 * a corrupted table from eating all system RAM.
 	 */
 	size = tbl->num_entries * tbl->desc_size;
-	if (size > 3 * efi.memmap.nr_map * efi.memmap.desc_size) {
+	if (tbl->desc_size != efi.memmap.desc_size || size > SZ_2M) {
 		pr_warn(FW_BUG "Corrupted EFI Memory Attributes Table detected! (version == %u, desc_size == %u, num_entries == %u)\n",
 			tbl->version, tbl->desc_size, tbl->num_entries);
 		goto unmap;
-- 
2.53.0.1018.g2bb0e51243-goog

Re: [PATCH 1/5] efi/memattr: Fix thinko in table size sanity check

[Gregory Price]

On Thu, Mar 26, 2026 at 02:26:57PM +0100, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> While it is true that each PE/COFF runtime driver in memory can
> generally be split into 3 different regions (the header, the code/rodata
> region and the data/bss region), each with different permissions, it
> does not mean that 3x the size of the memory map is a suitable upper
> bound. This is due to the fact that all runtime drivers could be
> coalesced into a single EFI runtime code region by the firmware, and if
> the firmware does a good job of keeping the fragmentation down, it is
> conceivable that the memory attributes table has more entries than the
> EFI memory map itself.
> 
> So instead, base the sanity check on whether the descriptor size matches
> the EFI memory map's descriptor size (which is not mandated by the spec
> but extremely unlikely to differ in practice), and whether the size of
> the whole table does not exceed 2 MiB.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

The 2MB limit is a bit odd to me - but then i don't see a legitimate
reason to need 50k+ entries here unless the system is doing something
absolutely nutty - it would mean a wildly fragmented system and most
likely an indicator of a bug rather than a legitimately intended
configuration.

Otherwise, this does seem like a better check regardless.

Reviewed-by: Gregory Price <gourry@gourry.net>

Re: [PATCH 1/5] efi/memattr: Fix thinko in table size sanity check

[Ard Biesheuvel]

On Sun, 29 Mar 2026, at 19:51, Gregory Price wrote:
> On Thu, Mar 26, 2026 at 02:26:57PM +0100, Ard Biesheuvel wrote:
>> From: Ard Biesheuvel <ardb@kernel.org>
>> 
>> While it is true that each PE/COFF runtime driver in memory can
>> generally be split into 3 different regions (the header, the code/rodata
>> region and the data/bss region), each with different permissions, it
>> does not mean that 3x the size of the memory map is a suitable upper
>> bound. This is due to the fact that all runtime drivers could be
>> coalesced into a single EFI runtime code region by the firmware, and if
>> the firmware does a good job of keeping the fragmentation down, it is
>> conceivable that the memory attributes table has more entries than the
>> EFI memory map itself.
>> 
>> So instead, base the sanity check on whether the descriptor size matches
>> the EFI memory map's descriptor size (which is not mandated by the spec
>> but extremely unlikely to differ in practice), and whether the size of
>> the whole table does not exceed 2 MiB.
>> 
>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> The 2MB limit is a bit odd to me - but then i don't see a legitimate
> reason to need 50k+ entries here unless the system is doing something
> absolutely nutty - it would mean a wildly fragmented system and most
> likely an indicator of a bug rather than a legitimately intended
> configuration.
>

The 2MB is completely arbitrary. We could use 1M or 32M for all I care. The original report was about a bogus table eating up all the memory.

> Otherwise, this does seem like a better check regardless.
>
> Reviewed-by: Gregory Price <gourry@gourry.net>

Thanks.

[PATCH 2/5] x86/efi: Gather initial memory reservation and table handling logic

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

Move the back-to-back calls to various EFI routines related to
processing of firmware tables and reserving the associated memory into a
helper function. This is tidier, and will avoid the need to add yet
another function call there in a subsequent patch.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/include/asm/efi.h  |  5 ++---
 arch/x86/kernel/setup.c     | 11 ++---------
 arch/x86/platform/efi/efi.c | 13 +++++++++++++
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 51b4cdbea061..3dcb137a49ed 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -387,11 +387,10 @@ static inline  bool efi_is_table_address(unsigned long phys_addr)
 {
 	return false;
 }
-static inline void efi_reserve_boot_services(void)
-{
-}
 #endif /* CONFIG_EFI */
 
+void efi_init_reservations(void);
+
 extern int __init efi_memmap_alloc(unsigned int num_entries,
 				   struct efi_memory_map_data *data);
 
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index eebcc9db1a1b..9f5f50bff16d 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1083,15 +1083,8 @@ void __init setup_arch(char **cmdline_p)
 	mem_encrypt_setup_arch();
 	cc_random_init();
 
-	efi_find_mirror();
-	efi_esrt_init();
-	efi_mokvar_table_init();
-
-	/*
-	 * The EFI specification says that boot service code won't be
-	 * called after ExitBootServices(). This is, in fact, a lie.
-	 */
-	efi_reserve_boot_services();
+	if (efi_enabled(EFI_BOOT))
+		efi_init_reservations();
 
 	/* preallocate 4k for mptable mpc */
 	e820__memblock_alloc_reserved_mpc_new();
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index d84c6020dda1..f32276bd8d4e 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -165,6 +165,19 @@ static void __init do_add_efi_memmap(void)
 	e820__update_table(e820_table);
 }
 
+void __init efi_init_reservations(void)
+{
+	efi_find_mirror();
+	efi_esrt_init();
+	efi_mokvar_table_init();
+
+	/*
+	 * The EFI specification says that boot service code won't be
+	 * called after ExitBootServices(). This is, in fact, a lie.
+	 */
+	efi_reserve_boot_services();
+}
+
 /*
  * Given add_efi_memmap defaults to 0 and there is no alternative
  * e820 mechanism for soft-reserved memory, import the full EFI memory
-- 
2.53.0.1018.g2bb0e51243-goog

Re: [PATCH 2/5] x86/efi: Gather initial memory reservation and table handling logic

[Gregory Price]

On Thu, Mar 26, 2026 at 02:26:58PM +0100, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> Move the back-to-back calls to various EFI routines related to
> processing of firmware tables and reserving the associated memory into a
> helper function. This is tidier, and will avoid the need to add yet
> another function call there in a subsequent patch.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Reviewed-by: Gregory Price <gourry@gourry.net>

[PATCH 3/5] x86/efi: Defer the call to efi_memattr_init()

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

efi_memattr_init() should call efi_mem_reserve() rather than
memblock_reserve() to preserve the memory contents of the region when
the EFI memory attributes table is loaded by the firmware.

However, efi_mem_reserve() can only be called later during the boot on
x86, due to the fact that it may need to memblock_alloc() memory for the
EFI memory map.

So move the call to efi_memattr_init() to a later stage when building
for x86_64.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/platform/efi/efi.c | 2 ++
 drivers/firmware/efi/efi.c  | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index f32276bd8d4e..107dcdf354ba 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -167,6 +167,8 @@ static void __init do_add_efi_memmap(void)
 
 void __init efi_init_reservations(void)
 {
+	if (IS_ENABLED(CONFIG_X86_64) && efi_enabled(EFI_MEMMAP))
+		efi_memattr_init();
 	efi_find_mirror();
 	efi_esrt_init();
 	efi_mokvar_table_init();
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index b2fb92a4bbd1..d28ed1ce9a33 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -761,7 +761,7 @@ int __init efi_config_parse_tables(const efi_config_table_t *config_tables,
 		}
 	}
 
-	if (!IS_ENABLED(CONFIG_X86_32) && efi_enabled(EFI_MEMMAP))
+	if (!IS_ENABLED(CONFIG_X86) && efi_enabled(EFI_MEMMAP))
 		efi_memattr_init();
 
 	efi_tpm_eventlog_init();
-- 
2.53.0.1018.g2bb0e51243-goog

[PATCH 4/5] efi: Use efi_mem_reserve() to reserve the memory attribute table

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

Use efi_mem_reserve() rather than memblock_reserve() to reserve the EFI
memory attributes table. This is needed on x86, where memblock_reserve()
is not sufficient for assets that may be placed in EFI boot services
data memory.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 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 8d4004149297..f9cfdec1fb51 100644
--- a/drivers/firmware/efi/memattr.c
+++ b/drivers/firmware/efi/memattr.c
@@ -59,7 +59,7 @@ void __init efi_memattr_init(void)
 	}
 
 	tbl_size = sizeof(*tbl) + size;
-	memblock_reserve(efi_mem_attr_table, tbl_size);
+	efi_mem_reserve(efi_mem_attr_table, tbl_size);
 	set_bit(EFI_MEM_ATTR, &efi.flags);
 
 unmap:
-- 
2.53.0.1018.g2bb0e51243-goog

[PATCH 5/5] x86/efi: Drop kexec quirk for the EFI memory attributes table

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

Now that the EFI memory attributes table is preserved properly, and the
quirk to detect corrupted tables has been updated not to result in false
positives when the number of EFI memory map entries is low compared to
the number of EFI memory attributes table entries, there is no longer a
need to ignore the latter when doing a kexec boot. So drop the
workaround.

This reverts commit

  64b45dd46e15 ("x86/efi: skip memattr table on kexec boot")

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/platform/efi/quirks.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 79f0818131e8..98641c621e6c 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -611,10 +611,6 @@ int __init efi_reuse_config(u64 tables, int nr_tables)
 		if (!efi_guidcmp(guid, SMBIOS_TABLE_GUID))
 			((efi_config_table_64_t *)p)->table = data->smbios;
 
-		/* Do not bother to play with mem attr table across kexec */
-		if (!efi_guidcmp(guid, EFI_MEMORY_ATTRIBUTES_TABLE_GUID))
-			((efi_config_table_64_t *)p)->table = EFI_INVALID_TABLE_ADDR;
-
 		p += sz;
 	}
 	early_memunmap(tablep, nr_tables * sz);
-- 
2.53.0.1018.g2bb0e51243-goog

Re: [PATCH 0/5] x86/efi: Re-enable memory attributes table for kexec

[Gregory Price]

On Thu, Mar 26, 2026 at 02:26:56PM +0100, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> The EFI memory attributes table augments the EFI memory map, and
> provides permission attributes for all runtime code and data regions
> that are otherwise mapped read-write-execute in their entirety.
> 
> Currently, this table is disregarded when doing kexec boot on x86, for
> two reasons:
> - the boot services data region that holds the table is not reserved
>   correctly, and may contain garbage at kexec time
> - a misguided sanity check on the size of the table is likely to trigger
>   on kexec, as the EFI memory map has been trimmed down by that time.
> 
> Fix both issues, so that the EFI memory attributes table can be taken
> into account again at kexec time. Note that this requires that the call
> to efi_memattr_init() is moved to a later point for x86.
> 

I appreciate this fixup, I'll try to give it a test in some known host
configuration switch scenarios I know don't work across multiple kexecs
and see if it is an improvement.

~Gregory

> Cc: Dave Young <dyoung@redhat.com>
> Cc: Gregory Price <gourry@gourry.net>
> Cc: Usama Arif <usamaarif642@gmail.com>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: Breno Leitao <leitao@debian.org>
> 
> Ard Biesheuvel (5):
>   efi/memattr: Fix thinko in table size sanity check
>   x86/efi: Gather initial memory reservation and table handling logic
>   x86/efi: Defer the call to efi_memattr_init()
>   efi: Use efi_mem_reserve() to reserve the memory attribute table
>   x86/efi: Drop kexec quirk for the EFI memory attributes table
> 
>  arch/x86/include/asm/efi.h     |  5 ++---
>  arch/x86/kernel/setup.c        | 11 ++---------
>  arch/x86/platform/efi/efi.c    | 15 +++++++++++++++
>  arch/x86/platform/efi/quirks.c |  4 ----
>  drivers/firmware/efi/efi.c     |  2 +-
>  drivers/firmware/efi/memattr.c | 17 ++++++++++-------
>  6 files changed, 30 insertions(+), 24 deletions(-)
> 
> 
> base-commit: 217c0a5c177a3d4f7c8497950cbf5c36756e8bbb
> -- 
> 2.53.0.1018.g2bb0e51243-goog
> 

Re: [PATCH 0/5] x86/efi: Re-enable memory attributes table for kexec

[Dave Young]

On Thu, 26 Mar 2026 at 21:27, Ard Biesheuvel <ardb+git@google.com> wrote:
>
> From: Ard Biesheuvel <ardb@kernel.org>
>
> The EFI memory attributes table augments the EFI memory map, and
> provides permission attributes for all runtime code and data regions
> that are otherwise mapped read-write-execute in their entirety.
>
> Currently, this table is disregarded when doing kexec boot on x86, for
> two reasons:
> - the boot services data region that holds the table is not reserved
>   correctly, and may contain garbage at kexec time
> - a misguided sanity check on the size of the table is likely to trigger
>   on kexec, as the EFI memory map has been trimmed down by that time.
>
> Fix both issues, so that the EFI memory attributes table can be taken
> into account again at kexec time. Note that this requires that the call
> to efi_memattr_init() is moved to a later point for x86.

I vaguely remember when we discussed this issue in old threads
efi_mem_reserve can not work for kexec_load syscall, but I think after
the commit a2498e5c453b it should just work.   So I think it looks
good, thanks Ard!

Thanks
Dave