The commit in [1] introduced a check to see if EFI memory attributes table was corrupted. It assumed that efi.memmap.nr_map remains constant, but it changes during late boot. Hence, the check is valid during cold boot, but not in the subsequent kexec boot. This is best explained with an exampled. At cold boot, for a test machine: efi.memmap.nr_map=91, memory_attributes_table->num_entries=48, desc_size = 48 Hence, the check introduced in [1] where 3x the size of the entire EFI memory map is a reasonable upper bound for the size of this table is valid. In late boot __efi_enter_virtual_mode calls 2 functions that updates efi.memmap.nr_map: - efi_map_regions which reduces the `count` of map entries (for e.g. if should_map_region returns false) and which is reflected in efi.memmap by __efi_memmap_init. At this point efi.memmap.nr_map becomes 46 in the test machine. - efi_free_boot_services which also reduces the number of memory regions available (for e.g. if md->type or md->attribute is not the right value). At this point efi.memmap.nr_map becomes 9 in the test machine. Hence when you kexec into a new kernel and pass efi.memmap, the paramaters that are compared are: efi.memmap.nr_map=9, memory_attributes_table->num_entries=48, desc_size = 48 where the check in [1] is no longer valid with such a low efi.memmap.nr_map as it was reduced due to efi_map_regions and efi_free_boot_services. A more appropriate check is to see if the description size reported by efi and memory attributes table is the same. [1] https://lore.kernel.org/all/20241031175822.2952471-2-ardb+git@xxxxxxxxxx/ Fixes: 8fbe4c49c0cc ("efi/memattr: Ignore table if the size is clearly bogus") Reported-by: Breno Leitao <leitao@xxxxxxxxxx> Signed-off-by: Usama Arif <usamaarif642@xxxxxxxxx> --- drivers/firmware/efi/memattr.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c index c38b1a335590..d3bc161361fb 100644 --- a/drivers/firmware/efi/memattr.c +++ b/drivers/firmware/efi/memattr.c @@ -40,21 +40,17 @@ int __init efi_memattr_init(void) 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 desc_size and + * efi.memmap.desc_size should match. */ - size = tbl->num_entries * tbl->desc_size; - if (size > 3 * efi.memmap.nr_map * efi.memmap.desc_size) { - 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); + if (efi.memmap.desc_size != tbl->desc_size) { + pr_warn(FW_BUG "Corrupted EFI Memory Attributes Table detected! (version == %u, table desc_size == %u, efi.memmap.desc_size == %lu, table num_entries == %u)\n", + tbl->version, tbl->desc_size, efi.memmap.desc_size, tbl->num_entries); goto unmap; } + size = tbl->num_entries * tbl->desc_size; tbl_size = sizeof(*tbl) + size; memblock_reserve(efi_mem_attr_table, tbl_size); set_bit(EFI_MEM_ATTR, &efi.flags); -- 2.43.5
