tpm2-tss icon indicating copy to clipboard operation
tpm2-tss copied to clipboard

tss2-sys/sysapi_uil.h: struct pack issues

Open jferreir opened this issue 1 year ago • 3 comments

The current pragma is not valid for some compilers

#pragma pack(push, 1)
typedef struct _TPM20_Header_In {
  TPM2_ST tag;
  UINT32 commandSize;
  UINT32 commandCode;
} TPM20_Header_In;

typedef struct _TPM20_Header_Out {
  TPM2_ST tag;
  UINT32 responseSize;
  UINT32 responseCode;
} TPM20_Header_Out;
#pragma pack(pop)

could we add something like the following? (the code below is not handling maybe all compiler, but you get the idea)

#if defined(__GNUC__)
#define PACK_STRUCT_STRUCT __attribute__((packed))
#define PACK_STRUCT_BEGIN
#define PACK_STRUCT_END
#endif

#if defined(__TASKING__)
#define PACK_STRUCT_STRUCT 
#define PACK_STRUCT_BEGIN #pragma pack 2
#define PACK_STRUCT_END #pragma pack 0
#endif

PACK_STRUCT_BEGIN
typedef struct _TPM20_Header_In {
  TPM2_ST tag;
  UINT32 commandSize;
  UINT32 commandCode;
} PACK_STRUCT_STRUCT TPM20_Header_In;

typedef struct _TPM20_Header_Out {
  TPM2_ST tag;
  UINT32 responseSize;
  UINT32 responseCode;
} PACK_STRUCT_STRUCT TPM20_Header_Out;
PACK_STRUCT_END

jferreir avatar May 08 '23 09:05 jferreir

I would actually prefer to add proper use of the Tss2_MU_XYZ_Marshal/Unmarshal() functions instead of having a packed struct. I do not think that there is actually a need for any packed anywhere in the code. So we should rework the code accordingly.

AndreasFuchsTPM avatar May 08 '23 12:05 AndreasFuchsTPM

Maybe the easiest would be to add a

typedef struct _TPM2_Header {
  TPM2_ST tag;
  UINT32 size;
  UINT32 code;
} TPM2_Header;

And corresponding function Tss2_MU_TPM2_Header_Marshal() and Tss2_MU_TPM2_Header_Unmarshal() and use them everywhere.

AndreasFuchsTPM avatar May 08 '23 12:05 AndreasFuchsTPM

I would actually prefer to add proper use of the Tss2_MU_XYZ_Marshal/Unmarshal() functions instead of having a packed struct. I do not think that there is actually a need for any packed anywhere in the code. So we should rework the code accordingly.

@AndreasFuchsTPM also the FAPI event parser and the tcti pcap builder use packed structures. Would you say we should also refactor this code because some embedded or specialized systems may use custom compilers that do not support structure packing? Perhaps it would be sufficient to refactor the use of the TPM2_Header?

JuergenReppSIT avatar May 09 '23 18:05 JuergenReppSIT