[edk2-devel] [PATCH v1 0/3] TCG2 protocol clean up

[edk2-devel] [PATCH v1 0/3] TCG2 protocol clean up

[Stuart Yoder]

This patch series cleans up some issues found when building edk2-test with
a non-GCC compiler:
  -TPMT_HA struct had an error due to incorrect use of C flexible array member
  -compute struct member offsets using OFFSET_OF, which is not GCC specific
  -clean up of #pragma pack in one file

Patches are in github here:
https://github.com/stuyod01/edk2-test/tree/tcg2-cleanup

Stuart Yoder (3):
  uefi-sct/SctPkg: TCG2 Protocol: correct definition of TPMT_HA struct
  uefi-sct/SctPkg: TCG2 Protocol: use OFFSET_OF for computing offsets
  uefi-sct/SctPkg: TCG2 Protocol: #pragma pack cleanup

 uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h            |  3 +--
 uefi-sct/SctPkg/UEFI/Protocol/TCG2.h                                                         | 13 ++++++++++++-
 uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c |  7 +++----
 3 files changed, 16 insertions(+), 7 deletions(-)

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117807): https://edk2.groups.io/g/devel/message/117807
Mute This Topic: https://groups.io/mt/105546454/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v1 1/3] uefi-sct/SctPkg: TCG2 Protocol: correct definition of TPMT_HA struct

[Stuart Yoder]

The TPMT_HA struct defining event log hash algorithms was cut/pasted
from the TCG EFI Protocol specification which used a C struct
with a flexible array member as the last element.  This is incorrect
because TPMT_HA itself is used as an array element, and thus can't
be variable size.

Because the size of hash algorithms varies, this should have been
defined as a union of the sizes of supported hash algorithms.  This is
how is it done in the TPM Library specfication and in EDK2.

Signed-off-by: Stuart Yoder <stuart.yoder@arm.com>
---
 uefi-sct/SctPkg/UEFI/Protocol/TCG2.h | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/uefi-sct/SctPkg/UEFI/Protocol/TCG2.h b/uefi-sct/SctPkg/UEFI/Protocol/TCG2.h
index a83a84c33134..e42b8b347c05 100644
--- a/uefi-sct/SctPkg/UEFI/Protocol/TCG2.h
+++ b/uefi-sct/SctPkg/UEFI/Protocol/TCG2.h
@@ -51,6 +51,10 @@ Abstract:
 #define EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 0x00000002
 
 #define HASH_NUMBER 0x04
+#define SHA1_DIGEST_SIZE    20
+#define SHA256_DIGEST_SIZE  32
+#define SHA384_DIGEST_SIZE  48
+#define SHA512_DIGEST_SIZE  64
 
 typedef struct _EFI_TCG2_PROTOCOL EFI_TCG2_PROTOCOL;
 
@@ -117,9 +121,16 @@ typedef struct tdEFI_TCG2_EVENT {
   UINT8 Event[];
 } EFI_TCG2_EVENT;
 
+typedef union {
+  UINT8 sha1[SHA1_DIGEST_SIZE];
+  UINT8 sha256[SHA256_DIGEST_SIZE];
+  UINT8 sha384[SHA384_DIGEST_SIZE];
+  UINT8 sha512[SHA512_DIGEST_SIZE];
+} TPMU_HA;
+
 typedef struct {
   UINT16     hashAlg;
-  UINT8      digest[];
+  TPMU_HA    digest;
 } TPMT_HA;
 
 typedef struct tdTPML_DIGEST_VALUES {
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117808): https://edk2.groups.io/g/devel/message/117808
Mute This Topic: https://groups.io/mt/105546455/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/3] uefi-sct/SctPkg: TCG2 Protocol: correct definition of TPMT_HA struct

[Heinrich Schuchardt]

On 4/16/24 00:23, Stuart Yoder wrote:
> The TPMT_HA struct defining event log hash algorithms was cut/pasted
> from the TCG EFI Protocol specification which used a C struct
> with a flexible array member as the last element.  This is incorrect
> because TPMT_HA itself is used as an array element, and thus can't
> be variable size.
> 
> Because the size of hash algorithms varies, this should have been
> defined as a union of the sizes of supported hash algorithms.  This is
> how is it done in the TPM Library specfication and in EDK2.
> 
> Signed-off-by: Stuart Yoder <stuart.yoder@arm.com>
> ---
>   uefi-sct/SctPkg/UEFI/Protocol/TCG2.h | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/uefi-sct/SctPkg/UEFI/Protocol/TCG2.h b/uefi-sct/SctPkg/UEFI/Protocol/TCG2.h
> index a83a84c33134..e42b8b347c05 100644
> --- a/uefi-sct/SctPkg/UEFI/Protocol/TCG2.h
> +++ b/uefi-sct/SctPkg/UEFI/Protocol/TCG2.h
> @@ -51,6 +51,10 @@ Abstract:
>   #define EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 0x00000002
>   
>   #define HASH_NUMBER 0x04
> +#define SHA1_DIGEST_SIZE    20
> +#define SHA256_DIGEST_SIZE  32

We should follow the TCG EFI Protocol Specification and support ShangMi 
3 in the test too.

#define SM3_256_DIGEST_SIZE  32

> +#define SHA384_DIGEST_SIZE  48
> +#define SHA512_DIGEST_SIZE  64
>   
>   typedef struct _EFI_TCG2_PROTOCOL EFI_TCG2_PROTOCOL;
>   
> @@ -117,9 +121,16 @@ typedef struct tdEFI_TCG2_EVENT {
>     UINT8 Event[];
>   } EFI_TCG2_EVENT;
>   
> +typedef union {
> +  UINT8 sha1[SHA1_DIGEST_SIZE];
> +  UINT8 sha256[SHA256_DIGEST_SIZE];

UINT8 sm3_256[SM3_256_DIGEST_SIZE];

Best regards

Heinrich

> +  UINT8 sha384[SHA384_DIGEST_SIZE];
> +  UINT8 sha512[SHA512_DIGEST_SIZE];
> +} TPMU_HA;
> +
>   typedef struct {
>     UINT16     hashAlg;
> -  UINT8      digest[];
> +  TPMU_HA    digest;
>   } TPMT_HA;
>   
>   typedef struct tdTPML_DIGEST_VALUES {



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117850): https://edk2.groups.io/g/devel/message/117850
Mute This Topic: https://groups.io/mt/105546455/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v1 2/3] uefi-sct/SctPkg: TCG2 Protocol: use OFFSET_OF for computing offsets

[Stuart Yoder]

Use compiler-independent OFFSET_OF macro defined from Base.h instead
of the GCC specific __builtin_offsetof

Signed-off-by: Stuart Yoder <stuart.yoder@arm.com>
---
 uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c
index 0122458a89fe..cba1ec1b9e2c 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c
@@ -25,8 +25,7 @@ Abstract:
 --*/
 
 #include "TCG2ProtocolBBTest.h"
-
-#define offsetof(st, m) __builtin_offsetof(st, m)
+#include <Base.h>
 
 /**
  *  @brief Entrypoint for GetCapability() Function Test.
@@ -475,7 +474,7 @@ BBTestGetCapabilityConformanceTestCheckpoint4 (
 
   // set size of struct to be up to and including the ManufacturerID
   // (this acts like a client with a 1.0 version of the struct)
-  BootServiceCap.Size = offsetof(EFI_TCG2_BOOT_SERVICE_CAPABILITY, NumberOfPcrBanks);
+  BootServiceCap.Size = OFFSET_OF(EFI_TCG2_BOOT_SERVICE_CAPABILITY, NumberOfPcrBanks);
 
   Status = TCG2->GetCapability (
                            TCG2,
@@ -494,7 +493,7 @@ BBTestGetCapabilityConformanceTestCheckpoint4 (
   }
 
   // Verify returned Size equals the size of EFI_TCG2_BOOT_SERVICE_CAPABILITY up to and including the ManufacturerID field.
-  if (BootServiceCap.Size != offsetof(EFI_TCG2_BOOT_SERVICE_CAPABILITY, NumberOfPcrBanks)) {
+  if (BootServiceCap.Size != OFFSET_OF(EFI_TCG2_BOOT_SERVICE_CAPABILITY, NumberOfPcrBanks)) {
     StandardLib->RecordMessage (
                      StandardLib,
                      EFI_VERBOSE_LEVEL_DEFAULT,
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117809): https://edk2.groups.io/g/devel/message/117809
Mute This Topic: https://groups.io/mt/105546456/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v1 3/3] uefi-sct/SctPkg: TCG2 Protocol: #pragma pack cleanup

[Stuart Yoder]

Fix compiler warning by adding #pragma pack() to close a pragma
section.  Also delete extraneous #pragma pack(1).

Signed-off-by: Stuart Yoder <stuart.yoder@arm.com>
---
 uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
index deba13f21804..95307b7fa50f 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
@@ -79,7 +79,6 @@ typedef struct {
   UINT8  buffer[TEST_STRING_LEN];
 } TPM2B_MAX_BUFFER;
 
-#pragma pack(1)
 // TPM2B_DIGEST as defined in Table 73 of TPM Library Spec Part 2: Structures
 typedef struct {
   UINT16 size;
@@ -110,7 +109,7 @@ typedef struct {
   TPM2B_DIGEST data;
   TPMT_TK_HASHCHECK validation;
 } TPM2_HASH_RESPONSE;
-#pragma
+#pragma pack()
 
 EFI_STATUS
 EFIAPI
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117810): https://edk2.groups.io/g/devel/message/117810
Mute This Topic: https://groups.io/mt/105546457/7686176
Mute #pragma:https://edk2.groups.io/g/devel/mutehashtag/pragma
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 3/3] uefi-sct/SctPkg: TCG2 Protocol: #pragma pack cleanup

[Heinrich Schuchardt]

On 4/16/24 00:23, Stuart Yoder wrote:
> Fix compiler warning by adding #pragma pack() to close a pragma
> section.  Also delete extraneous #pragma pack(1).
> 
> Signed-off-by: Stuart Yoder <stuart.yoder@arm.com>

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

> ---
>   uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
> index deba13f21804..95307b7fa50f 100644
> --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
> @@ -79,7 +79,6 @@ typedef struct {
>     UINT8  buffer[TEST_STRING_LEN];
>   } TPM2B_MAX_BUFFER;
>   
> -#pragma pack(1)
>   // TPM2B_DIGEST as defined in Table 73 of TPM Library Spec Part 2: Structures
>   typedef struct {
>     UINT16 size;
> @@ -110,7 +109,7 @@ typedef struct {
>     TPM2B_DIGEST data;
>     TPMT_TK_HASHCHECK validation;
>   } TPM2_HASH_RESPONSE;
> -#pragma
> +#pragma pack()
>   
>   EFI_STATUS
>   EFIAPI



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117851): https://edk2.groups.io/g/devel/message/117851
Mute This Topic: https://groups.io/mt/105546457/7686176
Mute #pragma:https://edk2.groups.io/g/devel/mutehashtag/pragma
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-