linux-kernel - Re: [PATCH] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified

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: <102fa362-35c9-46d2-853c-472a5c4cd5d9@126.com>
Date: Sat, 5 Jul 2025 14:58:24 +0800
From: Ge Yang <yangge1116@....com>
To: Jarkko Sakkinen <jarkko@...nel.org>
Cc: ardb@...nel.org, sathyanarayanan.kuppuswamy@...ux.intel.com,
 ilias.apalodimas@...aro.org, jgg@...pe.ca, linux-efi@...r.kernel.org,
 linux-kernel@...r.kernel.org, stable@...r.kernel.org, liuzixing@...on.cn
Subject: Re: [PATCH] efi/tpm: Fix the issue where the CC platforms event log
 header can't be correctly identified



在 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@....com wrote:
>>>> From: Ge Yang <yangge1116@....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