From: Usama Arif <usamaarif642@gmail.com>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: linux-efi@vger.kernel.org, kexec@lists.infradead.org,
ebiederm@xmission.com, bhe@redhat.com, vgoyal@redhat.com,
tglx@linutronix.de, dave.hansen@linux.intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, leitao@debian.org, rmikey@meta.com,
gourry@gourry.net
Subject: Re: [RFC] efi/tpm: add efi.tpm_log as a reserved region in 820_table_firmware
Date: Thu, 12 Sep 2024 12:17:06 +0100 [thread overview]
Message-ID: <542e0a32-5dfd-4894-b08b-f4cdc49705bc@gmail.com> (raw)
In-Reply-To: <CAMj1kXGi+N6AukJt6EGQTao=-1Ud_=bzwPvdjEzhmzEraFU98w@mail.gmail.com>
On 12/09/2024 11:51, Ard Biesheuvel wrote:
> On Thu, 12 Sept 2024 at 12:23, Usama Arif <usamaarif642@gmail.com> wrote:
>>
>>
>>
>> On 11/09/2024 12:51, Ard Biesheuvel wrote:
>>> On Wed, 11 Sept 2024 at 12:41, Usama Arif <usamaarif642@gmail.com> wrote:
>>>>
>>>> Looking at the TPM spec [1]
>>>>
>>>> If the ACPI TPM2 table contains the address and size of the Platform
>>>> Firmware TCG log, firmware “pins” the memory associated with the
>>>> Platform FirmwareTCG log, and reports this memory as “Reserved” memory
>>>> via the INT 15h/E820 interface.
>>>>
>>>> It looks like the firmware should pass this as reserved in e820 memory
>>>> map. However, it doesn't seem to. The firmware being tested on is:
>>>> dmidecode -s bios-version
>>>> edk2-20240214-2.el9
>>>>
>>>> When this area is not reserved, it comes up as usable in
>>>> /sys/firmware/memmap. This means that kexec, which uses that memmap
>>>> to find usable memory regions, can select the region where efi.tpm_log
>>>> is and overwrite it and relocate_kernel.
>>>>
>>>> Having a fix in firmware can be difficult to get through. As a secondary
>>>> fix, this patch marks that region as reserved in e820_table_firmware if it
>>>> is currently E820_TYPE_RAM so that kexec doesn't use it for kernel segments.
>>>>
>>>> [1] https://trustedcomputinggroup.org/wp-content/uploads/PC-ClientPlatform_Profile_for_TPM_2p0_Systems_v49_161114_public-review.pdf
>>>>
>>>> Signed-off-by: Usama Arif <usamaarif642@gmail.com>
>>>
>>> I would expect the EFI memory map to E820 conversion implemented in
>>> the EFI stub to take care of this.
>>>
>>
>> So I have been making a prototype with EFI stub, and the unfinished version is looking like a
>> horrible hack.
>>
>> The only way to do this in libstub is to pass log_tbl all the way from efi_retrieve_tcg2_eventlog
>> to efi_stub_entry and from there to setup_e820.
>> While going through the efi memory map and converting it to e820 table in setup_e820, you have to check
>> if log_tbl falls in any of the ranges and if the range is E820_TYPE_RAM. If that condition is satisfied,
>> then you have to split that range into 3. i.e. the E820_TYPE_RAM range before tpm_log, the tpm_log
>> E820_TYPE_RESERVED range, and the E820_TYPE_RAM range after. There are no helper functions, so this
>> splitting involves playing with a lot of pointers, and it looks quite ugly. I believe doing this
>> way is more likely to introduce bugs.
>>
>> If we are having to compensate for an EFI bug, would it make sense to do it in the way done
>> in RFC and do it in kernel rather than libstub? It is simple and very likely to be bug free.
>>
>
> I don't see how this could be an EFI bug, given that it does not deal
> with E820 tables at all.
EFI passes memory descriptors to libstub, libstub converts it to e820.
I believe the right behaviour should be that EFI creates an EFI_RESERVED_TYPE region for
that TPM log memory. Then libstub would automatically convert that EFI_RESERVED_TYPE
to E820_TYPE_RESERVED in setup_e820 [1].
[1] https://elixir.bootlin.com/linux/v6.10.9/source/drivers/firmware/efi/libstub/x86-stub.c#L573
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
WARNING: multiple messages have this Message-ID (diff)
From: Usama Arif <usamaarif642@gmail.com>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: linux-efi@vger.kernel.org, kexec@lists.infradead.org,
ebiederm@xmission.com, bhe@redhat.com, vgoyal@redhat.com,
tglx@linutronix.de, dave.hansen@linux.intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, leitao@debian.org, rmikey@meta.com,
gourry@gourry.net
Subject: Re: [RFC] efi/tpm: add efi.tpm_log as a reserved region in 820_table_firmware
Date: Thu, 12 Sep 2024 12:17:06 +0100 [thread overview]
Message-ID: <542e0a32-5dfd-4894-b08b-f4cdc49705bc@gmail.com> (raw)
In-Reply-To: <CAMj1kXGi+N6AukJt6EGQTao=-1Ud_=bzwPvdjEzhmzEraFU98w@mail.gmail.com>
On 12/09/2024 11:51, Ard Biesheuvel wrote:
> On Thu, 12 Sept 2024 at 12:23, Usama Arif <usamaarif642@gmail.com> wrote:
>>
>>
>>
>> On 11/09/2024 12:51, Ard Biesheuvel wrote:
>>> On Wed, 11 Sept 2024 at 12:41, Usama Arif <usamaarif642@gmail.com> wrote:
>>>>
>>>> Looking at the TPM spec [1]
>>>>
>>>> If the ACPI TPM2 table contains the address and size of the Platform
>>>> Firmware TCG log, firmware “pins” the memory associated with the
>>>> Platform FirmwareTCG log, and reports this memory as “Reserved” memory
>>>> via the INT 15h/E820 interface.
>>>>
>>>> It looks like the firmware should pass this as reserved in e820 memory
>>>> map. However, it doesn't seem to. The firmware being tested on is:
>>>> dmidecode -s bios-version
>>>> edk2-20240214-2.el9
>>>>
>>>> When this area is not reserved, it comes up as usable in
>>>> /sys/firmware/memmap. This means that kexec, which uses that memmap
>>>> to find usable memory regions, can select the region where efi.tpm_log
>>>> is and overwrite it and relocate_kernel.
>>>>
>>>> Having a fix in firmware can be difficult to get through. As a secondary
>>>> fix, this patch marks that region as reserved in e820_table_firmware if it
>>>> is currently E820_TYPE_RAM so that kexec doesn't use it for kernel segments.
>>>>
>>>> [1] https://trustedcomputinggroup.org/wp-content/uploads/PC-ClientPlatform_Profile_for_TPM_2p0_Systems_v49_161114_public-review.pdf
>>>>
>>>> Signed-off-by: Usama Arif <usamaarif642@gmail.com>
>>>
>>> I would expect the EFI memory map to E820 conversion implemented in
>>> the EFI stub to take care of this.
>>>
>>
>> So I have been making a prototype with EFI stub, and the unfinished version is looking like a
>> horrible hack.
>>
>> The only way to do this in libstub is to pass log_tbl all the way from efi_retrieve_tcg2_eventlog
>> to efi_stub_entry and from there to setup_e820.
>> While going through the efi memory map and converting it to e820 table in setup_e820, you have to check
>> if log_tbl falls in any of the ranges and if the range is E820_TYPE_RAM. If that condition is satisfied,
>> then you have to split that range into 3. i.e. the E820_TYPE_RAM range before tpm_log, the tpm_log
>> E820_TYPE_RESERVED range, and the E820_TYPE_RAM range after. There are no helper functions, so this
>> splitting involves playing with a lot of pointers, and it looks quite ugly. I believe doing this
>> way is more likely to introduce bugs.
>>
>> If we are having to compensate for an EFI bug, would it make sense to do it in the way done
>> in RFC and do it in kernel rather than libstub? It is simple and very likely to be bug free.
>>
>
> I don't see how this could be an EFI bug, given that it does not deal
> with E820 tables at all.
EFI passes memory descriptors to libstub, libstub converts it to e820.
I believe the right behaviour should be that EFI creates an EFI_RESERVED_TYPE region for
that TPM log memory. Then libstub would automatically convert that EFI_RESERVED_TYPE
to E820_TYPE_RESERVED in setup_e820 [1].
[1] https://elixir.bootlin.com/linux/v6.10.9/source/drivers/firmware/efi/libstub/x86-stub.c#L573
next prev parent reply other threads:[~2024-09-12 12:31 UTC|newest]
Thread overview: 75+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-11 10:41 [RFC] efi/tpm: add efi.tpm_log as a reserved region in 820_table_firmware Usama Arif
2024-09-11 10:41 ` Usama Arif
2024-09-11 11:51 ` Ard Biesheuvel
2024-09-11 11:51 ` Ard Biesheuvel
2024-09-11 14:34 ` Usama Arif
2024-09-11 14:34 ` Usama Arif
2024-09-12 10:23 ` Usama Arif
2024-09-12 10:23 ` Usama Arif
2024-09-12 10:51 ` Ard Biesheuvel
2024-09-12 10:51 ` Ard Biesheuvel
2024-09-12 11:17 ` Usama Arif [this message]
2024-09-12 11:17 ` Usama Arif
2024-09-12 13:03 ` Breno Leitao
2024-09-12 13:03 ` Breno Leitao
2024-09-12 13:10 ` Ard Biesheuvel
2024-09-12 13:10 ` Ard Biesheuvel
2024-09-12 13:54 ` Usama Arif
2024-09-12 13:54 ` Usama Arif
2024-09-12 14:05 ` Ard Biesheuvel
2024-09-12 14:05 ` Ard Biesheuvel
2024-09-12 14:14 ` Ard Biesheuvel
2024-09-12 14:14 ` Ard Biesheuvel
2024-09-13 10:56 ` Dave Young
2024-09-13 10:56 ` Dave Young
2024-09-13 11:06 ` Usama Arif
2024-09-13 11:06 ` Usama Arif
2024-09-13 11:13 ` Dave Young
2024-09-13 11:13 ` Dave Young
2024-09-13 11:49 ` Dave Young
2024-09-13 11:49 ` Dave Young
2024-09-13 11:56 ` Usama Arif
2024-09-13 11:56 ` Usama Arif
2024-09-13 12:52 ` Dave Young
2024-09-13 12:52 ` Dave Young
2024-09-14 6:46 ` Dave Young
2024-09-14 6:46 ` Dave Young
2024-09-14 8:31 ` Ard Biesheuvel
2024-09-14 8:31 ` Ard Biesheuvel
2024-09-14 9:24 ` Dave Young
2024-09-14 9:24 ` Dave Young
2024-09-14 9:46 ` Dave Young
2024-09-14 9:46 ` Dave Young
2024-09-12 14:29 ` Breno Leitao
2024-09-12 14:29 ` Breno Leitao
2024-09-12 15:21 ` Ard Biesheuvel
2024-09-12 15:21 ` Ard Biesheuvel
2024-09-12 15:35 ` Usama Arif
2024-09-12 15:35 ` Usama Arif
2024-09-12 15:45 ` Ard Biesheuvel
2024-09-12 15:45 ` Ard Biesheuvel
2024-09-12 16:22 ` James Bottomley
2024-09-12 16:22 ` James Bottomley
2024-09-13 11:57 ` Breno Leitao
2024-09-13 11:57 ` Breno Leitao
2024-09-13 12:07 ` James Bottomley
2024-09-13 12:07 ` James Bottomley
2024-09-16 20:20 ` Eric W. Biederman
2024-09-16 20:20 ` Eric W. Biederman
2024-09-17 6:45 ` Ard Biesheuvel
2024-09-17 6:45 ` Ard Biesheuvel
2024-09-17 15:24 ` Eric W. Biederman
2024-09-17 15:24 ` Eric W. Biederman
2024-09-17 15:35 ` Ard Biesheuvel
2024-09-17 15:35 ` Ard Biesheuvel
2024-09-18 3:13 ` Eric W. Biederman
2024-09-18 3:13 ` Eric W. Biederman
2024-09-18 7:36 ` Ard Biesheuvel
2024-09-18 7:36 ` Ard Biesheuvel
2024-10-09 9:10 ` Jonathan McDowell
2024-10-09 9:10 ` Jonathan McDowell
2024-10-09 10:46 ` Breno Leitao
2024-10-09 10:46 ` Breno Leitao
2024-10-09 14:05 ` Jonathan McDowell
2024-10-09 14:05 ` Jonathan McDowell
2024-09-14 8:44 ` kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=542e0a32-5dfd-4894-b08b-f4cdc49705bc@gmail.com \
--to=usamaarif642@gmail.com \
--cc=ardb@kernel.org \
--cc=bhe@redhat.com \
--cc=dave.hansen@linux.intel.com \
--cc=ebiederm@xmission.com \
--cc=gourry@gourry.net \
--cc=kexec@lists.infradead.org \
--cc=leitao@debian.org \
--cc=linux-efi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rmikey@meta.com \
--cc=tglx@linutronix.de \
--cc=vgoyal@redhat.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.