linux-kernel - Re: [RFC] efi/tpm: add efi.tpm_log as a reserved region in 820_table

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9d3962f1-96b6-44a3-a7d3-10fbfbe06164@gmail.com>
Date: Thu, 12 Sep 2024 16:35:06 +0100
From: Usama Arif <usamaarif642@...il.com>
To: Ard Biesheuvel <ardb@...nel.org>, Breno Leitao <leitao@...ian.org>
Cc: linux-efi@...r.kernel.org, kexec@...ts.infradead.org,
 ebiederm@...ssion.com, bhe@...hat.com, vgoyal@...hat.com,
 tglx@...utronix.de, dave.hansen@...ux.intel.com, x86@...nel.org,
 linux-kernel@...r.kernel.org, rmikey@...a.com, gourry@...rry.net
Subject: Re: [RFC] efi/tpm: add efi.tpm_log as a reserved region in
 820_table_firmware



On 12/09/2024 16:21, Ard Biesheuvel wrote:
> On Thu, 12 Sept 2024 at 16:29, Breno Leitao <leitao@...ian.org> wrote:
>>
>> On Thu, Sep 12, 2024 at 03:10:43PM +0200, Ard Biesheuvel wrote:
>>> On Thu, 12 Sept 2024 at 15:03, Breno Leitao <leitao@...ian.org> wrote:
>>>> 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.
>>
>> Thanks for the explanation.
>>
>> So, if I understand what you meant here, the TPM event log memory range
>> shouldn't be listed as a memory region in EFI memory map (as passed by
>> the firmware to the OS).
>>
>> Hence, this is not a EFI firmware bug, but a OS/Kernel bug.
>>
>> Am I correct with the statements above?
>>
> 
> No, not entirely. But I also missed the face that this table is
> actually created by the EFI stub in Linux, not the firmware. It is
> *not* the same memory region that the TPM2 ACPI table describes, and
> so what the various specs say about that is entirely irrelevant.
> 
> The TPM event log configuration table is created by the EFI stub,
> which uses the TCG2::GetEventLog () protocol method to obtain it. This
> must be done by the EFI stub because these protocols will no longer be
> available once the OS boots. But the data is not used by the EFI stub,
> only by the OS, which is why it is passed in memory like this.
> 
> The memory in question is allocated as EFI_LOADER_DATA, and so we are
> relying on the OS to know that this memory is special, and needs to be
> preserved.
> 
> I think the solution here is to use a different memory type:
> 
> --- 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);
> 
>         if (status != EFI_SUCCESS) {
> 
> which will be treated appropriately by the existing EFI-to-E820
> conversion logic.

I have tested above diff, and it works! No memory corruption.

The region comes up as ACPI data:
[    0.000000] BIOS-e820: [mem 0x000000007fb6d000-0x000000007fb7efff] ACPI data                                                                                                                               

and kexec doesnt interfere with it.

Thanks,
Usama