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

[PATCH v2 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.

Changes since v1:
- Sanity check the descriptor size and entry count individually: this
  prevents integer overflow, and avoid noisy diagnostics when using
  kexec_load(), which reconstructs the EFI memory map using a different
  descriptor size.
- Apply Gregory's Rb to patch #1 (but not patch #1)

I'd like to take this through the EFI tree as a whole, or take the
first patch now (which is a fix) and leave the rest for -tip during the
next cycle. (Patches #3 and #4 should be queued together in any case)

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 | 39 +++++++++++++++-----
 6 files changed, 49 insertions(+), 27 deletions(-)


base-commit: d0c3bcd5b8976159d835a897254048e078f447e6
-- 
2.53.0.1118.gaef5881109-goog

[PATCH v2 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 closely enough (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 64k entries.

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

diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
index e727cc5909cb..b83f1c5a9164 100644
--- a/drivers/firmware/efi/memattr.c
+++ b/drivers/firmware/efi/memattr.c
@@ -22,7 +22,6 @@ unsigned long __ro_after_init efi_mem_attr_table = EFI_INVALID_TABLE_ADDR;
 void __init efi_memattr_init(void)
 {
 	efi_memory_attributes_table_t *tbl;
-	unsigned long size;
 
 	if (efi_mem_attr_table == EFI_INVALID_TABLE_ADDR)
 		return;
@@ -40,22 +39,42 @@ void __init efi_memattr_init(void)
 		goto unmap;
 	}
 
+	/*
+	 * The EFI memory attributes table descriptors might potentially be
+	 * smaller than those used by the EFI memory map, as long as they can
+	 * fit a efi_memory_desc_t. However, a larger descriptor size makes no
+	 * sense, and might be an indication that the table is corrupted.
+	 *
+	 * The only exception is kexec_load(), where the EFI memory map is
+	 * reconstructed by user space, and may use a smaller descriptor size
+	 * than the original. Given that, ignoring this companion table is
+	 * still the right thing to do here, but don't complain too loudly when
+	 * this happens.
+	 */
+	if (tbl->desc_size < sizeof(efi_memory_desc_t) ||
+	    tbl->desc_size > efi.memmap.desc_size) {
+		pr_warn("Unexpected EFI Memory Attributes descriptor size %d (expected: %ld)\n",
+			tbl->desc_size, efi.memmap.desc_size);
+		goto unmap;
+	}
 
 	/*
-	 * 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 64k, which is
+	 * ludicrously high. 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->num_entries > SZ_64K) {
 		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;
 	}
 
-	tbl_size = sizeof(*tbl) + size;
+	tbl_size = sizeof(*tbl) + tbl->num_entries * tbl->desc_size;
 	memblock_reserve(efi_mem_attr_table, tbl_size);
 	set_bit(EFI_MEM_ATTR, &efi.flags);
 
-- 
2.53.0.1118.gaef5881109-goog

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

[Breno Leitao]

On Wed, Apr 01, 2026 at 02:23:53PM +0200, Ard Biesheuvel wrote:
> +	if (tbl->desc_size < sizeof(efi_memory_desc_t) ||
> +	    tbl->desc_size > efi.memmap.desc_size) {
> +		pr_warn("Unexpected EFI Memory Attributes descriptor size %d (expected: %ld)\n",
> +			tbl->desc_size, efi.memmap.desc_size);

Should tbl->desc_size be printed as %u and %lu?  tbl->desc_size is u32
and.  The pr_warn a few lines below already uses %u for the same field:

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

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

[Ard Biesheuvel]

On Wed, 1 Apr 2026, at 14:45, Breno Leitao wrote:
> On Wed, Apr 01, 2026 at 02:23:53PM +0200, Ard Biesheuvel wrote:
>> +	if (tbl->desc_size < sizeof(efi_memory_desc_t) ||
>> +	    tbl->desc_size > efi.memmap.desc_size) {
>> +		pr_warn("Unexpected EFI Memory Attributes descriptor size %d (expected: %ld)\n",
>> +			tbl->desc_size, efi.memmap.desc_size);
>
> Should tbl->desc_size be printed as %u and %lu?  tbl->desc_size is u32
> and.  The pr_warn a few lines below already uses %u for the same field:
>

Yes, good point - will fix.

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

[Breno Leitao]

On Wed, Apr 01, 2026 at 04:31:27PM +0200, Ard Biesheuvel wrote:
> 
> On Wed, 1 Apr 2026, at 14:45, Breno Leitao wrote:
> > On Wed, Apr 01, 2026 at 02:23:53PM +0200, Ard Biesheuvel wrote:
> >> +	if (tbl->desc_size < sizeof(efi_memory_desc_t) ||
> >> +	    tbl->desc_size > efi.memmap.desc_size) {
> >> +		pr_warn("Unexpected EFI Memory Attributes descriptor size %d (expected: %ld)\n",
> >> +			tbl->desc_size, efi.memmap.desc_size);
> >
> > Should tbl->desc_size be printed as %u and %lu?  tbl->desc_size is u32
> > and.  The pr_warn a few lines below already uses %u for the same field:
> >
> 
> Yes, good point - will fix.

Thanks. With the fix, please add:

Reviewed-by: Breno Leitao <leitao@debian.org>

[PATCH v2 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.

Reviewed-by: Gregory Price <gourry@gourry.net>
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.1118.gaef5881109-goog

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

[Breno Leitao]

On Wed, Apr 01, 2026 at 02:23:54PM +0200, 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.
> 
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Reviewed-by: Breno Leitao <leitao@debian.org>

> +void efi_init_reservations(void);

Do you neeed __init here, as the declaration below? I know that "extern" is not
necessary, but isn't __init?

  extern int __init efi_memmap_alloc(unsigned int num_entries,
                                     struct efi_memory_map_data *data);

  extern int __init efi_memmap_install(struct efi_memory_map_data *data);
  extern int __init efi_memmap_split_count(efi_memory_desc_t *md,

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

[Ard Biesheuvel]

On Wed, 1 Apr 2026, at 16:49, Breno Leitao wrote:
> On Wed, Apr 01, 2026 at 02:23:54PM +0200, 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.
>>
>> Reviewed-by: Gregory Price <gourry@gourry.net>
>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> Reviewed-by: Breno Leitao <leitao@debian.org>
>

Thanks

>> +void efi_init_reservations(void);
>
> Do you neeed __init here, as the declaration below? I know that
> "extern" is not necessary, but isn't __init?
>

No, it's not. Initness is a property of the definition not the
declaration. It decides which ELF section the code is emitted into,
and a declaration does not produce any code.

The section attribute is permitted on declarations by most compilers,
and I can see how it may be informative, but it can also easily get
out of sync so I prefer to omit it.

[PATCH v2 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.1118.gaef5881109-goog

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

[Breno Leitao]

On Wed, Apr 01, 2026 at 02:23:55PM +0200, Ard Biesheuvel wrote:
> 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>

Reviewed-by: Breno Leitao <leitao@debian.org>

[PATCH v2 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 b83f1c5a9164..785b7fc687fe 100644
--- a/drivers/firmware/efi/memattr.c
+++ b/drivers/firmware/efi/memattr.c
@@ -75,7 +75,7 @@ void __init efi_memattr_init(void)
 	}
 
 	tbl_size = sizeof(*tbl) + tbl->num_entries * tbl->desc_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.1118.gaef5881109-goog

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

[Breno Leitao]

On Wed, Apr 01, 2026 at 02:23:56PM +0200, Ard Biesheuvel wrote:
> 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>

Reviewed-by: Breno Leitao <leitao@debian.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 b83f1c5a9164..785b7fc687fe 100644
> --- a/drivers/firmware/efi/memattr.c
> +++ b/drivers/firmware/efi/memattr.c
> @@ -75,7 +75,7 @@ void __init efi_memattr_init(void)
>  	}
>  
>  	tbl_size = sizeof(*tbl) + tbl->num_entries * tbl->desc_size;
> -	memblock_reserve(efi_mem_attr_table, tbl_size);

nit: memblock_reserve seems to be the only memblock usage in the file,
so after this patch, the #include <linux/memblock.h> becomes dead.

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

[Ard Biesheuvel]

On Wed, 1 Apr 2026, at 19:39, Breno Leitao wrote:
> On Wed, Apr 01, 2026 at 02:23:56PM +0200, Ard Biesheuvel wrote:
>> 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>
>
> Reviewed-by: Breno Leitao <leitao@debian.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 b83f1c5a9164..785b7fc687fe 100644
>> --- a/drivers/firmware/efi/memattr.c
>> +++ b/drivers/firmware/efi/memattr.c
>> @@ -75,7 +75,7 @@ void __init efi_memattr_init(void)
>>  	}
>>  
>>  	tbl_size = sizeof(*tbl) + tbl->num_entries * tbl->desc_size;
>> -	memblock_reserve(efi_mem_attr_table, tbl_size);
>
> nit: memblock_reserve seems to be the only memblock usage in the file,
> so after this patch, the #include <linux/memblock.h> becomes dead.

Thanks for spotting that. Will fix.

[PATCH v2 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.1118.gaef5881109-goog