linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified
@ 2025-07-03  2:38 yangge1116
  2025-07-04  1:50 ` Jarkko Sakkinen
  0 siblings, 1 reply; 7+ messages in thread
From: yangge1116 @ 2025-07-03  2:38 UTC (permalink / raw)
  To: ardb
  Cc: jarkko, sathyanarayanan.kuppuswamy, ilias.apalodimas, jgg,
	linux-efi, linux-kernel, stable, liuzixing, Ge Yang

From: Ge Yang <yangge1116@126.com>

Since commit d228814b1913 ("efi/libstub: Add get_event_log() support
for CC platforms") reuses TPM2 support code for the CC platforms, when
launching a TDX virtual machine with coco measurement enabled, the
following error log is generated:

[Firmware Bug]: Failed to parse event in TPM Final Events Log

Call Trace:
efi_config_parse_tables()
  efi_tpm_eventlog_init()
    tpm2_calc_event_log_size()
      __calc_tpm2_event_size()

The pcr_idx value in the Intel TDX log header is 1, causing the
function __calc_tpm2_event_size() to fail to recognize the log header,
ultimately leading to the "Failed to parse event in TPM Final Events
Log" error.

According to UEFI Spec 2.10 Section 38.4.1: For Tdx, TPM PCR 0 maps to
MRTD, so the log header uses TPM PCR 1. To successfully parse the TDX
event log header, the check for a pcr_idx value of 0 has been removed
here, and it appears that this will not affect other functionalities.

Link: https://uefi.org/specs/UEFI/2.10/38_Confidential_Computing.html#intel-trust-domain-extension
Fixes: d228814b1913 ("efi/libstub: Add get_event_log() support for CC platforms")
Signed-off-by: Ge Yang <yangge1116@126.com>
Cc: stable@vger.kernel.org
---
 include/linux/tpm_eventlog.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index 891368e..05c0ae5 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -202,8 +202,7 @@ static __always_inline u32 __calc_tpm2_event_size(struct tcg_pcr_event2_head *ev
 	event_type = event->event_type;
 
 	/* Verify that it's the log header */
-	if (event_header->pcr_idx != 0 ||
-	    event_header->event_type != NO_ACTION ||
+	if (event_header->event_type != NO_ACTION ||
 	    memcmp(event_header->digest, zero_digest, sizeof(zero_digest))) {
 		size = 0;
 		goto out;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified
  2025-07-03  2:38 [PATCH] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified yangge1116
@ 2025-07-04  1:50 ` Jarkko Sakkinen
  2025-07-04  2:53   ` Ge Yang
  0 siblings, 1 reply; 7+ messages in thread
From: Jarkko Sakkinen @ 2025-07-04  1:50 UTC (permalink / raw)
  To: yangge1116
  Cc: ardb, sathyanarayanan.kuppuswamy, ilias.apalodimas, jgg,
	linux-efi, linux-kernel, stable, liuzixing

On Thu, Jul 03, 2025 at 10:38:37AM +0800, yangge1116@126.com wrote:
> From: Ge Yang <yangge1116@126.com>
> 
> Since commit d228814b1913 ("efi/libstub: Add get_event_log() support
> for CC platforms") reuses TPM2 support code for the CC platforms, when
> launching a TDX virtual machine with coco measurement enabled, the
> following error log is generated:
> 
> [Firmware Bug]: Failed to parse event in TPM Final Events Log
> 
> Call Trace:
> efi_config_parse_tables()
>   efi_tpm_eventlog_init()
>     tpm2_calc_event_log_size()
>       __calc_tpm2_event_size()
> 
> The pcr_idx value in the Intel TDX log header is 1, causing the
> function __calc_tpm2_event_size() to fail to recognize the log header,
> ultimately leading to the "Failed to parse event in TPM Final Events
> Log" error.
> 
> According to UEFI Spec 2.10 Section 38.4.1: For Tdx, TPM PCR 0 maps to
> MRTD, so the log header uses TPM PCR 1. To successfully parse the TDX
> event log header, the check for a pcr_idx value of 0 has been removed
> here, and it appears that this will not affect other functionalities.

I'm not familiar with the original change but with a quick check it did
not change __calc_tpm2_event_size(). Your change is changing semantics
to two types of callers:

1. Those that caused the bug.
2. Those that nothing to do with this bug.

I'm not seeing anything explaining that your change is guaranteed not to
have any consequences to "innocent" callers, which have no relation to
the bug.

> 
> Link: https://uefi.org/specs/UEFI/2.10/38_Confidential_Computing.html#intel-trust-domain-extension
> Fixes: d228814b1913 ("efi/libstub: Add get_event_log() support for CC platforms")
> Signed-off-by: Ge Yang <yangge1116@126.com>
> Cc: stable@vger.kernel.org
> ---
>  include/linux/tpm_eventlog.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
> index 891368e..05c0ae5 100644
> --- a/include/linux/tpm_eventlog.h
> +++ b/include/linux/tpm_eventlog.h
> @@ -202,8 +202,7 @@ static __always_inline u32 __calc_tpm2_event_size(struct tcg_pcr_event2_head *ev
>  	event_type = event->event_type;
>  
>  	/* Verify that it's the log header */
> -	if (event_header->pcr_idx != 0 ||
> -	    event_header->event_type != NO_ACTION ||
> +	if (event_header->event_type != NO_ACTION ||
>  	    memcmp(event_header->digest, zero_digest, sizeof(zero_digest))) {
>  		size = 0;
>  		goto out;
> -- 
> 2.7.4
> 

BR, Jarkko

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified
  2025-07-04  1:50 ` Jarkko Sakkinen
@ 2025-07-04  2:53   ` Ge Yang
  2025-07-04 15:13     ` Jarkko Sakkinen
  2025-07-04 22:27     ` Sathyanarayanan Kuppuswamy
  0 siblings, 2 replies; 7+ messages in thread
From: Ge Yang @ 2025-07-04  2:53 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: ardb, sathyanarayanan.kuppuswamy, ilias.apalodimas, jgg,
	linux-efi, linux-kernel, stable, liuzixing



在 2025/7/4 9:50, Jarkko Sakkinen 写道:
> On Thu, Jul 03, 2025 at 10:38:37AM +0800, yangge1116@126.com wrote:
>> From: Ge Yang <yangge1116@126.com>
>>
>> Since commit d228814b1913 ("efi/libstub: Add get_event_log() support
>> for CC platforms") reuses TPM2 support code for the CC platforms, when
>> launching a TDX virtual machine with coco measurement enabled, the
>> following error log is generated:
>>
>> [Firmware Bug]: Failed to parse event in TPM Final Events Log
>>
>> Call Trace:
>> efi_config_parse_tables()
>>    efi_tpm_eventlog_init()
>>      tpm2_calc_event_log_size()
>>        __calc_tpm2_event_size()
>>
>> The pcr_idx value in the Intel TDX log header is 1, causing the
>> function __calc_tpm2_event_size() to fail to recognize the log header,
>> ultimately leading to the "Failed to parse event in TPM Final Events
>> Log" error.
>>
>> According to UEFI Spec 2.10 Section 38.4.1: For Tdx, TPM PCR 0 maps to
>> MRTD, so the log header uses TPM PCR 1. To successfully parse the TDX
>> event log header, the check for a pcr_idx value of 0 has been removed
>> here, and it appears that this will not affect other functionalities.
> 
> I'm not familiar with the original change but with a quick check it did
> not change __calc_tpm2_event_size(). Your change is changing semantics
> to two types of callers:
> 
> 1. Those that caused the bug.
> 2. Those that nothing to do with this bug.
> 
> I'm not seeing anything explaining that your change is guaranteed not to
> have any consequences to "innocent" callers, which have no relation to
> the bug.
> 

Thank you for your response.

According to Section 10.2.1, Table 6 (TCG_PCClientPCREvent Structure) in 
the TCG PC Client Platform Firmware Profile Specification, determining 
whether an event is an event log header does not require checking the 
pcrIndex field. The identification can be made based on other fields 
alone. Therefore, removing the pcrIndex check here is considered safe
for "innocent" callers.

Reference Link: 
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05_v23_pub.pdf
>>
>> Link: https://uefi.org/specs/UEFI/2.10/38_Confidential_Computing.html#intel-trust-domain-extension
>> Fixes: d228814b1913 ("efi/libstub: Add get_event_log() support for CC platforms")
>> Signed-off-by: Ge Yang <yangge1116@126.com>
>> Cc: stable@vger.kernel.org
>> ---
>>   include/linux/tpm_eventlog.h | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
>> index 891368e..05c0ae5 100644
>> --- a/include/linux/tpm_eventlog.h
>> +++ b/include/linux/tpm_eventlog.h
>> @@ -202,8 +202,7 @@ static __always_inline u32 __calc_tpm2_event_size(struct tcg_pcr_event2_head *ev
>>   	event_type = event->event_type;
>>   
>>   	/* Verify that it's the log header */
>> -	if (event_header->pcr_idx != 0 ||
>> -	    event_header->event_type != NO_ACTION ||
>> +	if (event_header->event_type != NO_ACTION ||
>>   	    memcmp(event_header->digest, zero_digest, sizeof(zero_digest))) {
>>   		size = 0;
>>   		goto out;
>> -- 
>> 2.7.4
>>
> 
> BR, Jarkko


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified
  2025-07-04  2:53   ` Ge Yang
@ 2025-07-04 15:13     ` Jarkko Sakkinen
  2025-07-05  6:58       ` Ge Yang
  2025-07-04 22:27     ` Sathyanarayanan Kuppuswamy
  1 sibling, 1 reply; 7+ messages in thread
From: Jarkko Sakkinen @ 2025-07-04 15:13 UTC (permalink / raw)
  To: Ge Yang
  Cc: ardb, sathyanarayanan.kuppuswamy, ilias.apalodimas, jgg,
	linux-efi, linux-kernel, stable, liuzixing

On Fri, Jul 04, 2025 at 10:53:54AM +0800, Ge Yang wrote:
> 
> 
> 在 2025/7/4 9:50, Jarkko Sakkinen 写道:
> > On Thu, Jul 03, 2025 at 10:38:37AM +0800, yangge1116@126.com wrote:
> > > From: Ge Yang <yangge1116@126.com>
> > > 
> > > Since commit d228814b1913 ("efi/libstub: Add get_event_log() support
> > > for CC platforms") reuses TPM2 support code for the CC platforms, when
> > > launching a TDX virtual machine with coco measurement enabled, the
> > > following error log is generated:
> > > 
> > > [Firmware Bug]: Failed to parse event in TPM Final Events Log
> > > 
> > > Call Trace:
> > > efi_config_parse_tables()
> > >    efi_tpm_eventlog_init()
> > >      tpm2_calc_event_log_size()
> > >        __calc_tpm2_event_size()
> > > 
> > > The pcr_idx value in the Intel TDX log header is 1, causing the
> > > function __calc_tpm2_event_size() to fail to recognize the log header,
> > > ultimately leading to the "Failed to parse event in TPM Final Events
> > > Log" error.
> > > 
> > > According to UEFI Spec 2.10 Section 38.4.1: For Tdx, TPM PCR 0 maps to
> > > MRTD, so the log header uses TPM PCR 1. To successfully parse the TDX
> > > event log header, the check for a pcr_idx value of 0 has been removed
> > > here, and it appears that this will not affect other functionalities.
> > 
> > I'm not familiar with the original change but with a quick check it did
> > not change __calc_tpm2_event_size(). Your change is changing semantics
> > to two types of callers:
> > 
> > 1. Those that caused the bug.
> > 2. Those that nothing to do with this bug.
> > 
> > I'm not seeing anything explaining that your change is guaranteed not to
> > have any consequences to "innocent" callers, which have no relation to
> > the bug.
> > 
> 
> Thank you for your response.
> 
> According to Section 10.2.1, Table 6 (TCG_PCClientPCREvent Structure) in the
> TCG PC Client Platform Firmware Profile Specification, determining whether
> an event is an event log header does not require checking the pcrIndex
> field. The identification can be made based on other fields alone.
> Therefore, removing the pcrIndex check here is considered safe
> for "innocent" callers.

Thanks for digging that out. Can you add something to the commit
message? That spec is common knowledge if you are "into the topic"
in the first palace so something along the lines of this would be
perfectly fine:

"The check can be safely removed, as ccording to table 6 at section
10.2.1 of TCG PC client specification the index field does not require
fixing the PCR index to zero."

But then: we still have that constraint there and we cannot predict the
side-effects of removing a constraint, even if it is incorrectly defined
constraint. For comparison, it's much less risky situation when adding
additional constraints, as possible side-effects in the worst case
scenarios can be even theoretically much lighter than in the opposite
situation.

For this reasons it would be perhaps better to limit the fix for the
CC only, and not change the semantics across the board.

BR, Jarkko

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified
  2025-07-04  2:53   ` Ge Yang
  2025-07-04 15:13     ` Jarkko Sakkinen
@ 2025-07-04 22:27     ` Sathyanarayanan Kuppuswamy
  2025-07-05  6:59       ` Ge Yang
  1 sibling, 1 reply; 7+ messages in thread
From: Sathyanarayanan Kuppuswamy @ 2025-07-04 22:27 UTC (permalink / raw)
  To: Ge Yang, Jarkko Sakkinen
  Cc: ardb, ilias.apalodimas, jgg, linux-efi, linux-kernel, stable,
	liuzixing


On 7/3/25 7:53 PM, Ge Yang wrote:
>
>
> 在 2025/7/4 9:50, Jarkko Sakkinen 写道:
>> On Thu, Jul 03, 2025 at 10:38:37AM +0800, yangge1116@126.com wrote:
>>> From: Ge Yang <yangge1116@126.com>
>>>
>>> Since commit d228814b1913 ("efi/libstub: Add get_event_log() support
>>> for CC platforms") reuses TPM2 support code for the CC platforms, when
>>> launching a TDX virtual machine with coco measurement enabled, the
>>> following error log is generated:
>>>
>>> [Firmware Bug]: Failed to parse event in TPM Final Events Log
>>>
>>> Call Trace:
>>> efi_config_parse_tables()
>>>    efi_tpm_eventlog_init()
>>>      tpm2_calc_event_log_size()
>>>        __calc_tpm2_event_size()
>>>
>>> The pcr_idx value in the Intel TDX log header is 1, causing the
>>> function __calc_tpm2_event_size() to fail to recognize the log header,
>>> ultimately leading to the "Failed to parse event in TPM Final Events
>>> Log" error.
>>>
>>> According to UEFI Spec 2.10 Section 38.4.1: For Tdx, TPM PCR 0 maps to
>>> MRTD, so the log header uses TPM PCR 1. To successfully parse the TDX
>>> event log header, the check for a pcr_idx value of 0 has been removed
>>> here, and it appears that this will not affect other functionalities.
>>
>> I'm not familiar with the original change but with a quick check it did
>> not change __calc_tpm2_event_size(). Your change is changing semantics
>> to two types of callers:
>>
>> 1. Those that caused the bug.
>> 2. Those that nothing to do with this bug.
>>
>> I'm not seeing anything explaining that your change is guaranteed not to
>> have any consequences to "innocent" callers, which have no relation to
>> the bug.
>>
>
> Thank you for your response.
>
> According to Section 10.2.1, Table 6 (TCG_PCClientPCREvent Structure) in the TCG PC Client Platform Firmware Profile Specification, determining whether an event is an event log header does not require checking the pcrIndex field. The identification can be made based on other fields alone. Therefore, removing the pcrIndex check here is considered safe
> for "innocent" callers.
>
> Reference Link: https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05_v23_pub.pdf


It looks like this check was originally added to handle a case which does not align with the TCG spec. So
removing it directly may have some impact to these older platform. Can we make this conditional to
CC platform?

commit 7dfc06a0f25b593a9f51992f540c0f80a57f3629
Author: Fabian Vogt <fvogt@suse.de>
Date:   Mon Jun 15 09:16:36 2020 +0200

     efi/tpm: Verify event log header before parsing

     It is possible that the first event in the event log is not actually a
     log header at all, but rather a normal event. This leads to the cast in
     __calc_tpm2_event_size being an invalid conversion, which means that
     the values read are effectively garbage. Depending on the first event's
     contents, this leads either to apparently normal behaviour, a crash or
     a freeze.

     While this behaviour of the firmware is not in accordance with the
     TCG Client EFI Specification, this happens on a Dell Precision 5510
     with the TPM enabled but hidden from the OS ("TPM On" disabled, state
     otherwise untouched). The EFI firmware claims that the TPM is present
     and active and that it supports the TCG 2.0 event log format.

     Fortunately, this can be worked around by simply checking the header
     of the first event and the event log header signature itself.

     Commit b4f1874c6216 ("tpm: check event log version before reading final
     events") addressed a similar issue also found on Dell models.

     Fixes: 6b0326190205 ("efi: Attempt to get the TCG2 event log in the boot stub")
     Signed-off-by: Fabian Vogt <fvogt@suse.de>
     Link: https://lore.kernel.org/r/1927248.evlx2EsYKh@linux-e202.suse.de
     Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1165773
     Signed-off-by: Ard Biesheuvel <ardb@kernel.org>


>>>
>>> Link: https://uefi.org/specs/UEFI/2.10/38_Confidential_Computing.html#intel-trust-domain-extension
>>> Fixes: d228814b1913 ("efi/libstub: Add get_event_log() support for CC platforms")
>>> Signed-off-by: Ge Yang <yangge1116@126.com>
>>> Cc: stable@vger.kernel.org
>>> ---
>>>   include/linux/tpm_eventlog.h | 3 +--
>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
>>> index 891368e..05c0ae5 100644
>>> --- a/include/linux/tpm_eventlog.h
>>> +++ b/include/linux/tpm_eventlog.h
>>> @@ -202,8 +202,7 @@ static __always_inline u32 __calc_tpm2_event_size(struct tcg_pcr_event2_head *ev
>>>       event_type = event->event_type;
>>>         /* Verify that it's the log header */
>>> -    if (event_header->pcr_idx != 0 ||
>>> -        event_header->event_type != NO_ACTION ||
>>> +    if (event_header->event_type != NO_ACTION ||
>>>           memcmp(event_header->digest, zero_digest, sizeof(zero_digest))) {
>>>           size = 0;
>>>           goto out;
>>> -- 
>>> 2.7.4
>>>
>>
>> BR, Jarkko
>
-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified
  2025-07-04 15:13     ` Jarkko Sakkinen
@ 2025-07-05  6:58       ` Ge Yang
  0 siblings, 0 replies; 7+ messages in thread
From: Ge Yang @ 2025-07-05  6:58 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: ardb, sathyanarayanan.kuppuswamy, ilias.apalodimas, jgg,
	linux-efi, linux-kernel, stable, liuzixing



在 2025/7/4 23:13, Jarkko Sakkinen 写道:
> On Fri, Jul 04, 2025 at 10:53:54AM +0800, Ge Yang wrote:
>>
>>
>> 在 2025/7/4 9:50, Jarkko Sakkinen 写道:
>>> On Thu, Jul 03, 2025 at 10:38:37AM +0800, yangge1116@126.com wrote:
>>>> From: Ge Yang <yangge1116@126.com>
>>>>
>>>> Since commit d228814b1913 ("efi/libstub: Add get_event_log() support
>>>> for CC platforms") reuses TPM2 support code for the CC platforms, when
>>>> launching a TDX virtual machine with coco measurement enabled, the
>>>> following error log is generated:
>>>>
>>>> [Firmware Bug]: Failed to parse event in TPM Final Events Log
>>>>
>>>> Call Trace:
>>>> efi_config_parse_tables()
>>>>     efi_tpm_eventlog_init()
>>>>       tpm2_calc_event_log_size()
>>>>         __calc_tpm2_event_size()
>>>>
>>>> The pcr_idx value in the Intel TDX log header is 1, causing the
>>>> function __calc_tpm2_event_size() to fail to recognize the log header,
>>>> ultimately leading to the "Failed to parse event in TPM Final Events
>>>> Log" error.
>>>>
>>>> According to UEFI Spec 2.10 Section 38.4.1: For Tdx, TPM PCR 0 maps to
>>>> MRTD, so the log header uses TPM PCR 1. To successfully parse the TDX
>>>> event log header, the check for a pcr_idx value of 0 has been removed
>>>> here, and it appears that this will not affect other functionalities.
>>>
>>> I'm not familiar with the original change but with a quick check it did
>>> not change __calc_tpm2_event_size(). Your change is changing semantics
>>> to two types of callers:
>>>
>>> 1. Those that caused the bug.
>>> 2. Those that nothing to do with this bug.
>>>
>>> I'm not seeing anything explaining that your change is guaranteed not to
>>> have any consequences to "innocent" callers, which have no relation to
>>> the bug.
>>>
>>
>> Thank you for your response.
>>
>> According to Section 10.2.1, Table 6 (TCG_PCClientPCREvent Structure) in the
>> TCG PC Client Platform Firmware Profile Specification, determining whether
>> an event is an event log header does not require checking the pcrIndex
>> field. The identification can be made based on other fields alone.
>> Therefore, removing the pcrIndex check here is considered safe
>> for "innocent" callers.
> 
> Thanks for digging that out. Can you add something to the commit
> message? That spec is common knowledge if you are "into the topic"
> in the first palace so something along the lines of this would be
> perfectly fine:
> 
> "The check can be safely removed, as ccording to table 6 at section
> 10.2.1 of TCG PC client specification the index field does not require
> fixing the PCR index to zero."
> 
Ok, thanks.

> But then: we still have that constraint there and we cannot predict the
> side-effects of removing a constraint, even if it is incorrectly defined
> constraint. For comparison, it's much less risky situation when adding
> additional constraints, as possible side-effects in the worst case
> scenarios can be even theoretically much lighter than in the opposite
> situation.
> 
> For this reasons it would be perhaps better to limit the fix for the
> CC only, and not change the semantics across the board.
> 
Ok, thanks.

I originally intended to add a 
cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT) check inside the 
__calc_tpm2_event_size() function to limit the fix to Confidential 
Computing (CC) platforms only. However, this function is also used 
during the EFI stub phase, where the cc_platform_has() function is not 
defined.

Call Trace:
efi_pe_entry()
   efi_stub_entry()
     efi_retrieve_eventlog()
       efi_retrieve_tcg2_eventlog()
         __calc_tpm2_event_size()

Therefore, I plan to modify __calc_tpm2_event_size() to accept an 
additional parameter indicating whether the event originates from a CC 
platform.


> BR, Jarkko


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified
  2025-07-04 22:27     ` Sathyanarayanan Kuppuswamy
@ 2025-07-05  6:59       ` Ge Yang
  0 siblings, 0 replies; 7+ messages in thread
From: Ge Yang @ 2025-07-05  6:59 UTC (permalink / raw)
  To: Sathyanarayanan Kuppuswamy, Jarkko Sakkinen
  Cc: ardb, ilias.apalodimas, jgg, linux-efi, linux-kernel, stable,
	liuzixing



在 2025/7/5 6:27, Sathyanarayanan Kuppuswamy 写道:
> 
> On 7/3/25 7:53 PM, Ge Yang wrote:
>>
>>
>> 在 2025/7/4 9:50, Jarkko Sakkinen 写道:
>>> On Thu, Jul 03, 2025 at 10:38:37AM +0800, yangge1116@126.com wrote:
>>>> From: Ge Yang <yangge1116@126.com>
>>>>
>>>> Since commit d228814b1913 ("efi/libstub: Add get_event_log() support
>>>> for CC platforms") reuses TPM2 support code for the CC platforms, when
>>>> launching a TDX virtual machine with coco measurement enabled, the
>>>> following error log is generated:
>>>>
>>>> [Firmware Bug]: Failed to parse event in TPM Final Events Log
>>>>
>>>> Call Trace:
>>>> efi_config_parse_tables()
>>>>    efi_tpm_eventlog_init()
>>>>      tpm2_calc_event_log_size()
>>>>        __calc_tpm2_event_size()
>>>>
>>>> The pcr_idx value in the Intel TDX log header is 1, causing the
>>>> function __calc_tpm2_event_size() to fail to recognize the log header,
>>>> ultimately leading to the "Failed to parse event in TPM Final Events
>>>> Log" error.
>>>>
>>>> According to UEFI Spec 2.10 Section 38.4.1: For Tdx, TPM PCR 0 maps to
>>>> MRTD, so the log header uses TPM PCR 1. To successfully parse the TDX
>>>> event log header, the check for a pcr_idx value of 0 has been removed
>>>> here, and it appears that this will not affect other functionalities.
>>>
>>> I'm not familiar with the original change but with a quick check it did
>>> not change __calc_tpm2_event_size(). Your change is changing semantics
>>> to two types of callers:
>>>
>>> 1. Those that caused the bug.
>>> 2. Those that nothing to do with this bug.
>>>
>>> I'm not seeing anything explaining that your change is guaranteed not to
>>> have any consequences to "innocent" callers, which have no relation to
>>> the bug.
>>>
>>
>> Thank you for your response.
>>
>> According to Section 10.2.1, Table 6 (TCG_PCClientPCREvent Structure) 
>> in the TCG PC Client Platform Firmware Profile Specification, 
>> determining whether an event is an event log header does not require 
>> checking the pcrIndex field. The identification can be made based on 
>> other fields alone. Therefore, removing the pcrIndex check here is 
>> considered safe
>> for "innocent" callers.
>>
>> Reference Link: https://trustedcomputinggroup.org/wp-content/uploads/ 
>> TCG_PCClient_PFP_r1p05_v23_pub.pdf
> 
> 
> It looks like this check was originally added to handle a case which 
> does not align with the TCG spec. So
> removing it directly may have some impact to these older platform. Can 
> we make this conditional to
> CC platform?

Ok, thanks.

I originally intended to add a 
cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT) check inside the 
__calc_tpm2_event_size() function to limit the fix to Confidential 
Computing (CC) platforms only. However, this function is also used 
during the EFI stub phase, where the cc_platform_has() function is not 
defined.

Call Trace:
efi_pe_entry()
   efi_stub_entry()
     efi_retrieve_eventlog()
       efi_retrieve_tcg2_eventlog()
         __calc_tpm2_event_size()

Therefore, I plan to modify __calc_tpm2_event_size() to accept an 
additional parameter indicating whether the event originates from a CC 
platform.

> 
> commit 7dfc06a0f25b593a9f51992f540c0f80a57f3629
> Author: Fabian Vogt <fvogt@suse.de>
> Date:   Mon Jun 15 09:16:36 2020 +0200
> 
>      efi/tpm: Verify event log header before parsing
> 
>      It is possible that the first event in the event log is not actually a
>      log header at all, but rather a normal event. This leads to the 
> cast in
>      __calc_tpm2_event_size being an invalid conversion, which means that
>      the values read are effectively garbage. Depending on the first 
> event's
>      contents, this leads either to apparently normal behaviour, a crash or
>      a freeze.
> 
>      While this behaviour of the firmware is not in accordance with the
>      TCG Client EFI Specification, this happens on a Dell Precision 5510
>      with the TPM enabled but hidden from the OS ("TPM On" disabled, state
>      otherwise untouched). The EFI firmware claims that the TPM is present
>      and active and that it supports the TCG 2.0 event log format.
> 
>      Fortunately, this can be worked around by simply checking the header
>      of the first event and the event log header signature itself.
> 
>      Commit b4f1874c6216 ("tpm: check event log version before reading 
> final
>      events") addressed a similar issue also found on Dell models.
> 
>      Fixes: 6b0326190205 ("efi: Attempt to get the TCG2 event log in the 
> boot stub")
>      Signed-off-by: Fabian Vogt <fvogt@suse.de>
>      Link: https://lore.kernel.org/r/1927248.evlx2EsYKh@linux-e202.suse.de
>      Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1165773
>      Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> 
> 
>>>>
>>>> Link: https://uefi.org/specs/ 
>>>> UEFI/2.10/38_Confidential_Computing.html#intel-trust-domain-extension
>>>> Fixes: d228814b1913 ("efi/libstub: Add get_event_log() support for 
>>>> CC platforms")
>>>> Signed-off-by: Ge Yang <yangge1116@126.com>
>>>> Cc: stable@vger.kernel.org
>>>> ---
>>>>   include/linux/tpm_eventlog.h | 3 +--
>>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/include/linux/tpm_eventlog.h b/include/linux/ 
>>>> tpm_eventlog.h
>>>> index 891368e..05c0ae5 100644
>>>> --- a/include/linux/tpm_eventlog.h
>>>> +++ b/include/linux/tpm_eventlog.h
>>>> @@ -202,8 +202,7 @@ static __always_inline u32 
>>>> __calc_tpm2_event_size(struct tcg_pcr_event2_head *ev
>>>>       event_type = event->event_type;
>>>>         /* Verify that it's the log header */
>>>> -    if (event_header->pcr_idx != 0 ||
>>>> -        event_header->event_type != NO_ACTION ||
>>>> +    if (event_header->event_type != NO_ACTION ||
>>>>           memcmp(event_header->digest, zero_digest, 
>>>> sizeof(zero_digest))) {
>>>>           size = 0;
>>>>           goto out;
>>>> -- 
>>>> 2.7.4
>>>>
>>>
>>> BR, Jarkko
>>


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-07-05  7:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-03  2:38 [PATCH] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified yangge1116
2025-07-04  1:50 ` Jarkko Sakkinen
2025-07-04  2:53   ` Ge Yang
2025-07-04 15:13     ` Jarkko Sakkinen
2025-07-05  6:58       ` Ge Yang
2025-07-04 22:27     ` Sathyanarayanan Kuppuswamy
2025-07-05  6:59       ` Ge Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).