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

Thanks.