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

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@xxxxxxxxxx>

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@xxxxxxxxxx>
---
 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