linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified
@ 2025-07-07  3:14 yangge1116
  2025-07-08  1:19 ` Ard Biesheuvel
  2025-07-10 21:58 ` James Bottomley
  0 siblings, 2 replies; 6+ messages in thread
From: yangge1116 @ 2025-07-07  3:14 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 Specification 2.10, Section 38.4.1: For TDX, TPM PCR
0 maps to MRTD, so the log header uses TPM PCR 1 instead. To successfully
parse the TDX event log header, the check for a pcr_idx value of 0
must be skipped.

According to Table 6 in Section 10.2.1 of the TCG PC Client
Specification, the index field does not require the PCR index to be
fixed at zero. Therefore, skipping the check for a pcr_idx value of
0 for CC platforms is safe.

Link: https://uefi.org/specs/UEFI/2.10/38_Confidential_Computing.html#intel-trust-domain-extension
Link: https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05_v23_pub.pdf
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
---

V3:
- fix build error

V2:
- limit the fix for CC only suggested by Jarkko and Sathyanarayanan

 drivers/char/tpm/eventlog/tpm2.c   |  4 +++-
 drivers/firmware/efi/libstub/tpm.c | 13 +++++++++----
 drivers/firmware/efi/tpm.c         |  4 +++-
 include/linux/tpm_eventlog.h       | 14 +++++++++++---
 4 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/char/tpm/eventlog/tpm2.c b/drivers/char/tpm/eventlog/tpm2.c
index 37a0580..30ef47c 100644
--- a/drivers/char/tpm/eventlog/tpm2.c
+++ b/drivers/char/tpm/eventlog/tpm2.c
@@ -18,6 +18,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/tpm_eventlog.h>
+#include <linux/cc_platform.h>
 
 #include "../tpm.h"
 #include "common.h"
@@ -36,7 +37,8 @@
 static size_t calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 				   struct tcg_pcr_event *event_header)
 {
-	return __calc_tpm2_event_size(event, event_header, false);
+	return __calc_tpm2_event_size(event, event_header, false,
+			cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT));
 }
 
 static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos)
diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c
index a5c6c4f..9728060 100644
--- a/drivers/firmware/efi/libstub/tpm.c
+++ b/drivers/firmware/efi/libstub/tpm.c
@@ -50,7 +50,8 @@ void efi_enable_reset_attack_mitigation(void)
 static void efi_retrieve_tcg2_eventlog(int version, efi_physical_addr_t log_location,
 				       efi_physical_addr_t log_last_entry,
 				       efi_bool_t truncated,
-				       struct efi_tcg2_final_events_table *final_events_table)
+				       struct efi_tcg2_final_events_table *final_events_table,
+				       bool is_cc_event)
 {
 	efi_guid_t linux_eventlog_guid = LINUX_EFI_TPM_EVENT_LOG_GUID;
 	efi_status_t status;
@@ -87,7 +88,8 @@ static void efi_retrieve_tcg2_eventlog(int version, efi_physical_addr_t log_loca
 			last_entry_size =
 				__calc_tpm2_event_size((void *)last_entry_addr,
 						    (void *)(long)log_location,
-						    false);
+						    false,
+						    is_cc_event);
 		} else {
 			last_entry_size = sizeof(struct tcpa_event) +
 			   ((struct tcpa_event *) last_entry_addr)->event_size;
@@ -123,7 +125,8 @@ static void efi_retrieve_tcg2_eventlog(int version, efi_physical_addr_t log_loca
 			header = data + offset + final_events_size;
 			event_size = __calc_tpm2_event_size(header,
 						   (void *)(long)log_location,
-						   false);
+						   false,
+						   is_cc_event);
 			/* If calc fails this is a malformed log */
 			if (!event_size)
 				break;
@@ -157,6 +160,7 @@ void efi_retrieve_eventlog(void)
 	efi_tcg2_protocol_t *tpm2 = NULL;
 	efi_bool_t truncated;
 	efi_status_t status;
+	bool is_cc_event = false;
 
 	status = efi_bs_call(locate_protocol, &tpm2_guid, NULL, (void **)&tpm2);
 	if (status == EFI_SUCCESS) {
@@ -186,11 +190,12 @@ void efi_retrieve_eventlog(void)
 
 		final_events_table =
 			get_efi_config_table(EFI_CC_FINAL_EVENTS_TABLE_GUID);
+		is_cc_event = true;
 	}
 
 	if (status != EFI_SUCCESS || !log_location)
 		return;
 
 	efi_retrieve_tcg2_eventlog(version, log_location, log_last_entry,
-				   truncated, final_events_table);
+				   truncated, final_events_table, is_cc_event);
 }
diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
index cdd4310..ca8535d 100644
--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -12,6 +12,7 @@
 #include <linux/init.h>
 #include <linux/memblock.h>
 #include <linux/tpm_eventlog.h>
+#include <linux/cc_platform.h>
 
 int efi_tpm_final_log_size;
 EXPORT_SYMBOL(efi_tpm_final_log_size);
@@ -23,7 +24,8 @@ static int __init tpm2_calc_event_log_size(void *data, int count, void *size_inf
 
 	while (count > 0) {
 		header = data + size;
-		event_size = __calc_tpm2_event_size(header, size_info, true);
+		event_size = __calc_tpm2_event_size(header, size_info, true,
+				     cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT));
 		if (event_size == 0)
 			return -1;
 		size += event_size;
diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index 891368e..b3380c9 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -143,6 +143,7 @@ struct tcg_algorithm_info {
  * @event:        Pointer to the event whose size should be calculated
  * @event_header: Pointer to the initial event containing the digest lengths
  * @do_mapping:   Whether or not the event needs to be mapped
+ * @is_cc_event:  Whether or not the event is from a CC platform
  *
  * The TPM2 event log format can contain multiple digests corresponding to
  * separate PCR banks, and also contains a variable length of the data that
@@ -159,7 +160,8 @@ struct tcg_algorithm_info {
 
 static __always_inline u32 __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 					 struct tcg_pcr_event *event_header,
-					 bool do_mapping)
+					 bool do_mapping,
+					 bool is_cc_event)
 {
 	struct tcg_efi_specid_event_head *efispecid;
 	struct tcg_event_field *event_field;
@@ -201,8 +203,14 @@ static __always_inline u32 __calc_tpm2_event_size(struct tcg_pcr_event2_head *ev
 	count = event->count;
 	event_type = event->event_type;
 
-	/* Verify that it's the log header */
-	if (event_header->pcr_idx != 0 ||
+	/*
+	 * Verify that it's the log header. According to the TCG PC Client
+	 * Specification, when identifying a log header, the check for a
+	 * pcr_idx value of 0 is not required. For CC platforms, skipping
+	 * this check during log header is necessary; otherwise, the CC
+	 * platform's log header may fail to be recognized.
+	 */
+	if ((!is_cc_event && event_header->pcr_idx != 0) ||
 	    event_header->event_type != NO_ACTION ||
 	    memcmp(event_header->digest, zero_digest, sizeof(zero_digest))) {
 		size = 0;
-- 
2.7.4


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

* Re: [PATCH V3] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified
  2025-07-07  3:14 [PATCH V3] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified yangge1116
@ 2025-07-08  1:19 ` Ard Biesheuvel
  2025-07-08  7:39   ` Ge Yang
  2025-07-10 21:58 ` James Bottomley
  1 sibling, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2025-07-08  1:19 UTC (permalink / raw)
  To: yangge1116
  Cc: jarkko, sathyanarayanan.kuppuswamy, ilias.apalodimas, jgg,
	linux-efi, linux-kernel, stable, liuzixing

On Mon, 7 Jul 2025 at 13:15, <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 Specification 2.10, Section 38.4.1: For TDX, TPM PCR
> 0 maps to MRTD, so the log header uses TPM PCR 1 instead. To successfully
> parse the TDX event log header, the check for a pcr_idx value of 0
> must be skipped.
>
> According to Table 6 in Section 10.2.1 of the TCG PC Client
> Specification, the index field does not require the PCR index to be
> fixed at zero. Therefore, skipping the check for a pcr_idx value of
> 0 for CC platforms is safe.
>
> Link: https://uefi.org/specs/UEFI/2.10/38_Confidential_Computing.html#intel-trust-domain-extension
> Link: https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05_v23_pub.pdf
> 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
> ---
>
> V3:
> - fix build error
>
> V2:
> - limit the fix for CC only suggested by Jarkko and Sathyanarayanan
>
>  drivers/char/tpm/eventlog/tpm2.c   |  4 +++-
>  drivers/firmware/efi/libstub/tpm.c | 13 +++++++++----
>  drivers/firmware/efi/tpm.c         |  4 +++-
>  include/linux/tpm_eventlog.h       | 14 +++++++++++---
>  4 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/char/tpm/eventlog/tpm2.c b/drivers/char/tpm/eventlog/tpm2.c
> index 37a0580..30ef47c 100644
> --- a/drivers/char/tpm/eventlog/tpm2.c
> +++ b/drivers/char/tpm/eventlog/tpm2.c
> @@ -18,6 +18,7 @@
>  #include <linux/module.h>
>  #include <linux/slab.h>
>  #include <linux/tpm_eventlog.h>
> +#include <linux/cc_platform.h>
>
>  #include "../tpm.h"
>  #include "common.h"
> @@ -36,7 +37,8 @@
>  static size_t calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
>                                    struct tcg_pcr_event *event_header)
>  {
> -       return __calc_tpm2_event_size(event, event_header, false);
> +       return __calc_tpm2_event_size(event, event_header, false,
> +                       cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT));


This conflates TDX attestation with guest state encryption.

I think there could be meaningful ways for a confidential guest to use
[emulated] TPM attestation rather than CC specific attestation, so I
don't think it is a good idea to assume that the fact that we are
running on a CoCo guest always implies that all the TCG data
structures are of the CC variety.


>  }
>
>  static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos)
> diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c
> index a5c6c4f..9728060 100644
> --- a/drivers/firmware/efi/libstub/tpm.c
> +++ b/drivers/firmware/efi/libstub/tpm.c
> @@ -50,7 +50,8 @@ void efi_enable_reset_attack_mitigation(void)
>  static void efi_retrieve_tcg2_eventlog(int version, efi_physical_addr_t log_location,
>                                        efi_physical_addr_t log_last_entry,
>                                        efi_bool_t truncated,
> -                                      struct efi_tcg2_final_events_table *final_events_table)
> +                                      struct efi_tcg2_final_events_table *final_events_table,
> +                                      bool is_cc_event)
>  {
>         efi_guid_t linux_eventlog_guid = LINUX_EFI_TPM_EVENT_LOG_GUID;
>         efi_status_t status;
> @@ -87,7 +88,8 @@ static void efi_retrieve_tcg2_eventlog(int version, efi_physical_addr_t log_loca
>                         last_entry_size =
>                                 __calc_tpm2_event_size((void *)last_entry_addr,
>                                                     (void *)(long)log_location,
> -                                                   false);
> +                                                   false,
> +                                                   is_cc_event);
>                 } else {
>                         last_entry_size = sizeof(struct tcpa_event) +
>                            ((struct tcpa_event *) last_entry_addr)->event_size;
> @@ -123,7 +125,8 @@ static void efi_retrieve_tcg2_eventlog(int version, efi_physical_addr_t log_loca
>                         header = data + offset + final_events_size;
>                         event_size = __calc_tpm2_event_size(header,
>                                                    (void *)(long)log_location,
> -                                                  false);
> +                                                  false,
> +                                                  is_cc_event);
>                         /* If calc fails this is a malformed log */
>                         if (!event_size)
>                                 break;
> @@ -157,6 +160,7 @@ void efi_retrieve_eventlog(void)
>         efi_tcg2_protocol_t *tpm2 = NULL;
>         efi_bool_t truncated;
>         efi_status_t status;
> +       bool is_cc_event = false;
>
>         status = efi_bs_call(locate_protocol, &tpm2_guid, NULL, (void **)&tpm2);
>         if (status == EFI_SUCCESS) {
> @@ -186,11 +190,12 @@ void efi_retrieve_eventlog(void)
>
>                 final_events_table =
>                         get_efi_config_table(EFI_CC_FINAL_EVENTS_TABLE_GUID);
> +               is_cc_event = true;
>         }
>
>         if (status != EFI_SUCCESS || !log_location)
>                 return;
>
>         efi_retrieve_tcg2_eventlog(version, log_location, log_last_entry,
> -                                  truncated, final_events_table);
> +                                  truncated, final_events_table, is_cc_event);
>  }
> diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
> index cdd4310..ca8535d 100644
> --- a/drivers/firmware/efi/tpm.c
> +++ b/drivers/firmware/efi/tpm.c
> @@ -12,6 +12,7 @@
>  #include <linux/init.h>
>  #include <linux/memblock.h>
>  #include <linux/tpm_eventlog.h>
> +#include <linux/cc_platform.h>
>
>  int efi_tpm_final_log_size;
>  EXPORT_SYMBOL(efi_tpm_final_log_size);
> @@ -23,7 +24,8 @@ static int __init tpm2_calc_event_log_size(void *data, int count, void *size_inf
>
>         while (count > 0) {
>                 header = data + size;
> -               event_size = __calc_tpm2_event_size(header, size_info, true);
> +               event_size = __calc_tpm2_event_size(header, size_info, true,
> +                                    cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT));
>                 if (event_size == 0)
>                         return -1;
>                 size += event_size;
> diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
> index 891368e..b3380c9 100644
> --- a/include/linux/tpm_eventlog.h
> +++ b/include/linux/tpm_eventlog.h
> @@ -143,6 +143,7 @@ struct tcg_algorithm_info {
>   * @event:        Pointer to the event whose size should be calculated
>   * @event_header: Pointer to the initial event containing the digest lengths
>   * @do_mapping:   Whether or not the event needs to be mapped
> + * @is_cc_event:  Whether or not the event is from a CC platform
>   *
>   * The TPM2 event log format can contain multiple digests corresponding to
>   * separate PCR banks, and also contains a variable length of the data that
> @@ -159,7 +160,8 @@ struct tcg_algorithm_info {
>
>  static __always_inline u32 __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
>                                          struct tcg_pcr_event *event_header,
> -                                        bool do_mapping)
> +                                        bool do_mapping,
> +                                        bool is_cc_event)
>  {
>         struct tcg_efi_specid_event_head *efispecid;
>         struct tcg_event_field *event_field;
> @@ -201,8 +203,14 @@ static __always_inline u32 __calc_tpm2_event_size(struct tcg_pcr_event2_head *ev
>         count = event->count;
>         event_type = event->event_type;
>
> -       /* Verify that it's the log header */
> -       if (event_header->pcr_idx != 0 ||
> +       /*
> +        * Verify that it's the log header. According to the TCG PC Client
> +        * Specification, when identifying a log header, the check for a
> +        * pcr_idx value of 0 is not required. For CC platforms, skipping
> +        * this check during log header is necessary; otherwise, the CC
> +        * platform's log header may fail to be recognized.
> +        */
> +       if ((!is_cc_event && event_header->pcr_idx != 0) ||
>             event_header->event_type != NO_ACTION ||
>             memcmp(event_header->digest, zero_digest, sizeof(zero_digest))) {
>                 size = 0;
> --
> 2.7.4
>

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

* Re: [PATCH V3] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified
  2025-07-08  1:19 ` Ard Biesheuvel
@ 2025-07-08  7:39   ` Ge Yang
  0 siblings, 0 replies; 6+ messages in thread
From: Ge Yang @ 2025-07-08  7:39 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: jarkko, sathyanarayanan.kuppuswamy, ilias.apalodimas, jgg,
	linux-efi, linux-kernel, stable, liuzixing



在 2025/7/8 9:19, Ard Biesheuvel 写道:
> On Mon, 7 Jul 2025 at 13:15, <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 Specification 2.10, Section 38.4.1: For TDX, TPM PCR
>> 0 maps to MRTD, so the log header uses TPM PCR 1 instead. To successfully
>> parse the TDX event log header, the check for a pcr_idx value of 0
>> must be skipped.
>>
>> According to Table 6 in Section 10.2.1 of the TCG PC Client
>> Specification, the index field does not require the PCR index to be
>> fixed at zero. Therefore, skipping the check for a pcr_idx value of
>> 0 for CC platforms is safe.
>>
>> Link: https://uefi.org/specs/UEFI/2.10/38_Confidential_Computing.html#intel-trust-domain-extension
>> Link: https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05_v23_pub.pdf
>> 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
>> ---
>>
>> V3:
>> - fix build error
>>
>> V2:
>> - limit the fix for CC only suggested by Jarkko and Sathyanarayanan
>>
>>   drivers/char/tpm/eventlog/tpm2.c   |  4 +++-
>>   drivers/firmware/efi/libstub/tpm.c | 13 +++++++++----
>>   drivers/firmware/efi/tpm.c         |  4 +++-
>>   include/linux/tpm_eventlog.h       | 14 +++++++++++---
>>   4 files changed, 26 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/char/tpm/eventlog/tpm2.c b/drivers/char/tpm/eventlog/tpm2.c
>> index 37a0580..30ef47c 100644
>> --- a/drivers/char/tpm/eventlog/tpm2.c
>> +++ b/drivers/char/tpm/eventlog/tpm2.c
>> @@ -18,6 +18,7 @@
>>   #include <linux/module.h>
>>   #include <linux/slab.h>
>>   #include <linux/tpm_eventlog.h>
>> +#include <linux/cc_platform.h>
>>
>>   #include "../tpm.h"
>>   #include "common.h"
>> @@ -36,7 +37,8 @@
>>   static size_t calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
>>                                     struct tcg_pcr_event *event_header)
>>   {
>> -       return __calc_tpm2_event_size(event, event_header, false);
>> +       return __calc_tpm2_event_size(event, event_header, false,
>> +                       cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT));
> 
> 
> This conflates TDX attestation with guest state encryption.
> 
> I think there could be meaningful ways for a confidential guest to use
> [emulated] TPM attestation rather than CC specific attestation, so I
> don't think it is a good idea to assume that the fact that we are
> running on a CoCo guest always implies that all the TCG data
> structures are of the CC variety.
> 

Okay, thank you. I'll adjust the patch again.

> 
>>   }
>>
>>   static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos)
>> diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c
>> index a5c6c4f..9728060 100644
>> --- a/drivers/firmware/efi/libstub/tpm.c
>> +++ b/drivers/firmware/efi/libstub/tpm.c
>> @@ -50,7 +50,8 @@ void efi_enable_reset_attack_mitigation(void)
>>   static void efi_retrieve_tcg2_eventlog(int version, efi_physical_addr_t log_location,
>>                                         efi_physical_addr_t log_last_entry,
>>                                         efi_bool_t truncated,
>> -                                      struct efi_tcg2_final_events_table *final_events_table)
>> +                                      struct efi_tcg2_final_events_table *final_events_table,
>> +                                      bool is_cc_event)
>>   {
>>          efi_guid_t linux_eventlog_guid = LINUX_EFI_TPM_EVENT_LOG_GUID;
>>          efi_status_t status;
>> @@ -87,7 +88,8 @@ static void efi_retrieve_tcg2_eventlog(int version, efi_physical_addr_t log_loca
>>                          last_entry_size =
>>                                  __calc_tpm2_event_size((void *)last_entry_addr,
>>                                                      (void *)(long)log_location,
>> -                                                   false);
>> +                                                   false,
>> +                                                   is_cc_event);
>>                  } else {
>>                          last_entry_size = sizeof(struct tcpa_event) +
>>                             ((struct tcpa_event *) last_entry_addr)->event_size;
>> @@ -123,7 +125,8 @@ static void efi_retrieve_tcg2_eventlog(int version, efi_physical_addr_t log_loca
>>                          header = data + offset + final_events_size;
>>                          event_size = __calc_tpm2_event_size(header,
>>                                                     (void *)(long)log_location,
>> -                                                  false);
>> +                                                  false,
>> +                                                  is_cc_event);
>>                          /* If calc fails this is a malformed log */
>>                          if (!event_size)
>>                                  break;
>> @@ -157,6 +160,7 @@ void efi_retrieve_eventlog(void)
>>          efi_tcg2_protocol_t *tpm2 = NULL;
>>          efi_bool_t truncated;
>>          efi_status_t status;
>> +       bool is_cc_event = false;
>>
>>          status = efi_bs_call(locate_protocol, &tpm2_guid, NULL, (void **)&tpm2);
>>          if (status == EFI_SUCCESS) {
>> @@ -186,11 +190,12 @@ void efi_retrieve_eventlog(void)
>>
>>                  final_events_table =
>>                          get_efi_config_table(EFI_CC_FINAL_EVENTS_TABLE_GUID);
>> +               is_cc_event = true;
>>          }
>>
>>          if (status != EFI_SUCCESS || !log_location)
>>                  return;
>>
>>          efi_retrieve_tcg2_eventlog(version, log_location, log_last_entry,
>> -                                  truncated, final_events_table);
>> +                                  truncated, final_events_table, is_cc_event);
>>   }
>> diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
>> index cdd4310..ca8535d 100644
>> --- a/drivers/firmware/efi/tpm.c
>> +++ b/drivers/firmware/efi/tpm.c
>> @@ -12,6 +12,7 @@
>>   #include <linux/init.h>
>>   #include <linux/memblock.h>
>>   #include <linux/tpm_eventlog.h>
>> +#include <linux/cc_platform.h>
>>
>>   int efi_tpm_final_log_size;
>>   EXPORT_SYMBOL(efi_tpm_final_log_size);
>> @@ -23,7 +24,8 @@ static int __init tpm2_calc_event_log_size(void *data, int count, void *size_inf
>>
>>          while (count > 0) {
>>                  header = data + size;
>> -               event_size = __calc_tpm2_event_size(header, size_info, true);
>> +               event_size = __calc_tpm2_event_size(header, size_info, true,
>> +                                    cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT));
>>                  if (event_size == 0)
>>                          return -1;
>>                  size += event_size;
>> diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
>> index 891368e..b3380c9 100644
>> --- a/include/linux/tpm_eventlog.h
>> +++ b/include/linux/tpm_eventlog.h
>> @@ -143,6 +143,7 @@ struct tcg_algorithm_info {
>>    * @event:        Pointer to the event whose size should be calculated
>>    * @event_header: Pointer to the initial event containing the digest lengths
>>    * @do_mapping:   Whether or not the event needs to be mapped
>> + * @is_cc_event:  Whether or not the event is from a CC platform
>>    *
>>    * The TPM2 event log format can contain multiple digests corresponding to
>>    * separate PCR banks, and also contains a variable length of the data that
>> @@ -159,7 +160,8 @@ struct tcg_algorithm_info {
>>
>>   static __always_inline u32 __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
>>                                           struct tcg_pcr_event *event_header,
>> -                                        bool do_mapping)
>> +                                        bool do_mapping,
>> +                                        bool is_cc_event)
>>   {
>>          struct tcg_efi_specid_event_head *efispecid;
>>          struct tcg_event_field *event_field;
>> @@ -201,8 +203,14 @@ static __always_inline u32 __calc_tpm2_event_size(struct tcg_pcr_event2_head *ev
>>          count = event->count;
>>          event_type = event->event_type;
>>
>> -       /* Verify that it's the log header */
>> -       if (event_header->pcr_idx != 0 ||
>> +       /*
>> +        * Verify that it's the log header. According to the TCG PC Client
>> +        * Specification, when identifying a log header, the check for a
>> +        * pcr_idx value of 0 is not required. For CC platforms, skipping
>> +        * this check during log header is necessary; otherwise, the CC
>> +        * platform's log header may fail to be recognized.
>> +        */
>> +       if ((!is_cc_event && event_header->pcr_idx != 0) ||
>>              event_header->event_type != NO_ACTION ||
>>              memcmp(event_header->digest, zero_digest, sizeof(zero_digest))) {
>>                  size = 0;
>> --
>> 2.7.4
>>


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

* Re: [PATCH V3] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified
  2025-07-07  3:14 [PATCH V3] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified yangge1116
  2025-07-08  1:19 ` Ard Biesheuvel
@ 2025-07-10 21:58 ` James Bottomley
  2025-07-11  1:23   ` Ge Yang
  1 sibling, 1 reply; 6+ messages in thread
From: James Bottomley @ 2025-07-10 21:58 UTC (permalink / raw)
  To: yangge1116, ardb
  Cc: jarkko, sathyanarayanan.kuppuswamy, ilias.apalodimas, jgg,
	linux-efi, linux-kernel, stable, liuzixing

On Mon, 2025-07-07 at 11:14 +0800, yangge1116@126.com wrote:
> 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 Specification 2.10, Section 38.4.1: For TDX, TPM
> PCR 0 maps to MRTD, so the log header uses TPM PCR 1 instead. To
> successfully parse the TDX event log header, the check for a pcr_idx
> value of 0 must be skipped.

I think someone has misread the spec.  EV_NO_ACTION events produce no
PCR extension.  So the PCR value zero is conventional (and required by
the TCG) since nothing gets logged.  Therefore even if you're
technically using PCR0 for something else EV_NO_ACTION events should
still have the conventional PCR = 0 value to conform to the TCG spec. 
I assume it's too late to correct this in the implementation?

>  __calc_tpm2_event_size(struct tcg_pcr_event2_head *ev
>  	count = event->count;
>  	event_type = event->event_type;
>  
> -	/* Verify that it's the log header */
> -	if (event_header->pcr_idx != 0 ||
> +	/*
> +	 * Verify that it's the log header. According to the TCG PC
> Client
> +	 * Specification, when identifying a log header, the check
> for a
> +	 * pcr_idx value of 0 is not required. For CC platforms,
> skipping
> +	 * this check during log header is necessary; otherwise, the
> CC
> +	 * platform's log header may fail to be recognized.
> +	 */
> +	if ((!is_cc_event && event_header->pcr_idx != 0) ||
>  	    event_header->event_type != NO_ACTION ||
>  	    memcmp(event_header->digest, zero_digest,
> sizeof(zero_digest))) {
>  		size = 0;

The above is just a heuristic to recognize an EV_NO_ACTION event as
zero size.  All the TCG specs require that EV_NO_ACTION have pcr 0 in
the event, but if the heuristic is wrong because of Intel/CC spec
violations which can't be fixed, then we should update the heuristic
... so I don't think you need to thread the is_cc_event.

Regards,

James

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

* Re: [PATCH V3] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified
  2025-07-10 21:58 ` James Bottomley
@ 2025-07-11  1:23   ` Ge Yang
  2025-07-11 12:32     ` James Bottomley
  0 siblings, 1 reply; 6+ messages in thread
From: Ge Yang @ 2025-07-11  1:23 UTC (permalink / raw)
  To: James Bottomley, ardb
  Cc: jarkko, sathyanarayanan.kuppuswamy, ilias.apalodimas, jgg,
	linux-efi, linux-kernel, stable, liuzixing



在 2025/7/11 5:58, James Bottomley 写道:
> On Mon, 2025-07-07 at 11:14 +0800, yangge1116@126.com wrote:
>> 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 Specification 2.10, Section 38.4.1: For TDX, TPM
>> PCR 0 maps to MRTD, so the log header uses TPM PCR 1 instead. To
>> successfully parse the TDX event log header, the check for a pcr_idx
>> value of 0 must be skipped.
> 
> I think someone has misread the spec.  EV_NO_ACTION events produce no
> PCR extension.  So the PCR value zero is conventional (and required by
> the TCG) since nothing gets logged.  Therefore even if you're
> technically using PCR0 for something else EV_NO_ACTION events should
> still have the conventional PCR = 0 value to conform to the TCG spec.
> I assume it's too late to correct this in the implementation?
> 

According to Table 14 in Section 10.4.1 of the TCG PC Client 
Specification, for EV_NO_ACTION events, the PCR (Platform Configuration 
Register) value can be 0 or other values, such as 6.

Link: 
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05_v23_pub.pdf

>>   __calc_tpm2_event_size(struct tcg_pcr_event2_head *ev
>>   	count = event->count;
>>   	event_type = event->event_type;
>>   
>> -	/* Verify that it's the log header */
>> -	if (event_header->pcr_idx != 0 ||
>> +	/*
>> +	 * Verify that it's the log header. According to the TCG PC
>> Client
>> +	 * Specification, when identifying a log header, the check
>> for a
>> +	 * pcr_idx value of 0 is not required. For CC platforms,
>> skipping
>> +	 * this check during log header is necessary; otherwise, the
>> CC
>> +	 * platform's log header may fail to be recognized.
>> +	 */
>> +	if ((!is_cc_event && event_header->pcr_idx != 0) ||
>>   	    event_header->event_type != NO_ACTION ||
>>   	    memcmp(event_header->digest, zero_digest,
>> sizeof(zero_digest))) {
>>   		size = 0;
> 
> The above is just a heuristic to recognize an EV_NO_ACTION event as
> zero size.  All the TCG specs require that EV_NO_ACTION have pcr 0 in
> the event, but if the heuristic is wrong because of Intel/CC spec
> violations which can't be fixed, then we should update the heuristic
> ... so I don't think you need to thread the is_cc_event.

It seems that the TCG specifications do not stipulate that the 
EV_NO_ACTION event must have PCR 0. In addition, adding is_cc_event can 
maximize the reuse of TPM code for CC platforms without disrupting TPM 
functionality; otherwise, new functions would need to be added for CC 
platforms.

Please review the latest patch. Patch link: 
https://lore.kernel.org/lkml/1751961289-29673-1-git-send-email-yangge1116@126.com/

> 
> Regards,
> 
> James


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

* Re: [PATCH V3] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified
  2025-07-11  1:23   ` Ge Yang
@ 2025-07-11 12:32     ` James Bottomley
  0 siblings, 0 replies; 6+ messages in thread
From: James Bottomley @ 2025-07-11 12:32 UTC (permalink / raw)
  To: Ge Yang, ardb
  Cc: jarkko, sathyanarayanan.kuppuswamy, ilias.apalodimas, jgg,
	linux-efi, linux-kernel, stable, liuzixing

On Fri, 2025-07-11 at 09:23 +0800, Ge Yang wrote:
> 在 2025/7/11 5:58, James Bottomley 写道:
[...]
> > I think someone has misread the spec.  EV_NO_ACTION events produce
> > no PCR extension.  So the PCR value zero is conventional (and
> > required by the TCG) since nothing gets logged.  Therefore even if
> > you're technically using PCR0 for something else EV_NO_ACTION
> > events should still have the conventional PCR = 0 value to conform
> > to the TCG spec. I assume it's too late to correct this in the
> > implementation?
> > 
> 
> According to Table 14 in Section 10.4.1 of the TCG PC Client 
> Specification, for EV_NO_ACTION events, the PCR (Platform
> Configuration Register) value can be 0 or other values, such as 6.
> 
> Link: 
> https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05_v23_pub.pdf

You're selectively quoting one part of the spec out of context. 
Section 10.4.5 (which is the normative one about EV_NO_ACTION events)
says:

   1. All EV_NO_ACTION events SHALL set TCG_PCR_EVENT2.pcrIndex = 0,
   unless otherwise specified.

And the only update to this is in 10.4.5.4 where it says a vendor
specific EV_NO_ACTION event may set pcrIndex to 6.  Since the log
header is not vendor specific, it must have pcr = 0.

Regards,

James




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

end of thread, other threads:[~2025-07-11 12:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-07  3:14 [PATCH V3] efi/tpm: Fix the issue where the CC platforms event log header can't be correctly identified yangge1116
2025-07-08  1:19 ` Ard Biesheuvel
2025-07-08  7:39   ` Ge Yang
2025-07-10 21:58 ` James Bottomley
2025-07-11  1:23   ` Ge Yang
2025-07-11 12:32     ` James Bottomley

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).