On Fri, 25 Oct 2024 at 07:09, Jiri Slaby <jirislaby@xxxxxxxxxx> wrote: > > On 25. 10. 24, 7:07, Jiri Slaby wrote: > > On 24. 10. 24, 18:20, Jiri Slaby wrote: > >> On 12. 09. 24, 17:52, Ard Biesheuvel wrote: > >>> From: Ard Biesheuvel <ardb@xxxxxxxxxx> > >>> > >>> The TPM event log table is a Linux specific construct, where the data > >>> produced by the GetEventLog() boot service is cached in memory, and > >>> passed on to the OS using a EFI configuration table. > >>> > >>> The use of EFI_LOADER_DATA here results in the region being left > >>> unreserved in the E820 memory map constructed by the EFI stub, and this > >>> is the memory description that is passed on to the incoming kernel by > >>> kexec, which is therefore unaware that the region should be reserved. > >>> > >>> Even though the utility of the TPM2 event log after a kexec is > >>> questionable, any corruption might send the parsing code off into the > >>> weeds and crash the kernel. So let's use EFI_ACPI_RECLAIM_MEMORY > >>> instead, which is always treated as reserved by the E820 conversion > >>> logic. > >>> > >>> Cc: <stable@xxxxxxxxxxxxxxx> > >>> Reported-by: Breno Leitao <leitao@xxxxxxxxxx> > >>> Tested-by: Usama Arif <usamaarif642@xxxxxxxxx> > >>> Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx> > >>> --- > >>> drivers/firmware/efi/libstub/tpm.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/ > >>> efi/libstub/tpm.c > >>> index df3182f2e63a..1fd6823248ab 100644 > >>> --- a/drivers/firmware/efi/libstub/tpm.c > >>> +++ b/drivers/firmware/efi/libstub/tpm.c > >>> @@ -96,7 +96,7 @@ static void efi_retrieve_tcg2_eventlog(int version, > >>> efi_physical_addr_t log_loca > >>> } > >>> /* Allocate space for the logs and copy them. */ > >>> - status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, > >>> + status = efi_bs_call(allocate_pool, EFI_ACPI_RECLAIM_MEMORY, > >>> sizeof(*log_tbl) + log_size, (void **)&log_tbl); > >> > >> Hi, > >> > >> this, for some reason, corrupts system configuration table. On good > >> boots, memattr points to 0x77535018, on bad boots (this commit > >> applied), it points to 0x77526018. > >> > >> And the good content at 0x77526018: > >> tab=0x77526018 size=16+45*48=0x0000000000000880 > >> > >> bad content at 0x77535018: > >> tab=0x77535018 size=16+2*1705353216=0x00000000cb4b4010 > >> > >> This happens only on cold boots. Subsequent boots (having the commit > >> or not) are all fine. > >> > >> Any ideas? > > > > ==== > > EFI_ACPI_RECLAIM_MEMORY > > > > This memory is to be preserved by the UEFI OS loader and OS until ACPI > > is enabled. Once ACPI is enabled, the memory in this range is available > > for general use. > > ==== > > > > BTW doesn't the above mean it is released by the time TPM actually reads > > it? > > > > Isn't the proper fix to actually memblock_reserve() that TPM portion. > > The same as memattr in efi_memattr_init()? > > And this is actually done in efi_tpm_eventlog_init(). > EFI_ACPI_RECLAIM_MEMORY may be reclaimed by the OS, but we never actually do that in Linux. To me, it seems like the use of EFI_ACPI_RECLAIM_MEMORY in this case simply tickles a bug in the firmware that causes it to corrupt the memory attributes table. The fact that cold boot behaves differently is a strong indicator here. I didn't see the results of the memory attribute table dumps on the bugzilla thread, but dumping this table from EFI is not very useful because it will get regenerated/updated at ExitBootServices() time. Unfortunately, that also takes away the console so capturing the state of that table before the EFI stub boots the kernel is not an easy thing to do. Is the memattr table completely corrupted? It also has a version field, and only versions 1 and 2 are defined so we might use that to detect corruption.
