Re: [RFC] efi/tpm: add efi.tpm_log as a reserved region in 820_table_firmware

[Ard Biesheuvel]

On Thu, 12 Sept 2024 at 15:03, Breno Leitao <leitao@xxxxxxxxxx> wrote:
>
> Hello Ard,
>
> On Thu, Sep 12, 2024 at 12:51:57PM +0200, Ard Biesheuvel wrote:
> > I don't see how this could be an EFI bug, given that it does not deal
> > with E820 tables at all.
>
> I want to back up a little bit and make sure I am following the
> discussion.
>
> From what I understand from previous discussion, we have an EFI bug as
> the root cause of this issue.
>
> This happens because the EFI does NOT mark the EFI TPM event log memory
> region as reserved (EFI_RESERVED_TYPE).

Why do you think EFI should use EFI_RESERVED_TYPE in this case?

The EFI spec is very clear that EFI_RESERVED_TYPE really shouldn't be
used for anything by EFI itself. It is quite common for EFI
configuration tables to be passed as EfiRuntimeServicesData (SMBIOS),
EfiBootServicesData (ESRT) or EFiAcpiReclaim (ACPI tables).

Reserved memory is mostly for memory that even the firmware does not
know what it is for, i.e., particular platform specific uses.

In general, it is up to the OS to ensure that EFI configuration tables
that it cares about should be reserved in the correct way.

> Not having an entry for the
> event table memory in EFI memory mapped, then libstub will ignore it
> completely (the TPM event log memory range) and not populate e820 table
> with it.
>
> Once the e820 table does not have the memory range for TPM event log,
> then the kernel is free to overwrite that memory region, causing
> corruptions all across the board.
>

We shouldn't be relying on the E820 table for this.

> From what I understand from the thread discussion, there are three ways
> to "solve" it:
>
> 1) Fix the EFI to pass the TPM event log memory as reserved.
>
> 2) Workaround it in libstub, and considering the TPM event log memory
> range when populating the e820 table. (As proposed in
> https://lore.kernel.org/all/2542182d-aa79-4705-91b6-fa593bacffa6@xxxxxxxxx/)
>
> 3) Workaround in later in the kernel, as proposed in
> https://lore.kernel.org/all/20240911104109.1831501-1-usamaarif642@xxxxxxxxx/
>

Does the below help at all?

--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -60,7 +60,7 @@ int __init efi_tpm_eventlog_init(void)
        }

        tbl_size = sizeof(*log_tbl) + log_tbl->size;
-       memblock_reserve(efi.tpm_log, tbl_size);
+       efi_mem_reserve(efi.tpm_log, tbl_size);

        if (efi.tpm_final_log == EFI_INVALID_TABLE_ADDR) {
                pr_info("TPM Final Events table not present\n");