Hi Stuart,
[...] > >> EFI_TCG2_EVENT_ALGORITHM_BITMAP ActivePcrBanks > >> }; > >> > >> The intent in the TCG spec is for the caller to be able to > >> determine the size of the struct by passing in a small > >> buffer (e.g. 1 byte buffer), and then the firmware > >> should return sizeof(EFI_TCG2_BOOT_SERVICE_CAPABILITY) in the > >> 1-byte "Size" field. And the return value should be > >> EFI_BUFFER_TOO_SMALL as per the spec. > >> > >> In order to detect a 1.0 client, which you don't want to support, > >> I think you need a _new_ check that is something like this: > >> > >> // detect 1.0 client > >> if (capability->size < sizeof(*capability) && > >> capability->size >= offsetof(EFI_TCG2_BOOT_SERVICE_CAPABILITY, > >> NumberOfPcrBanks)) { > >> efi_ret = EFI_UNSUPPORTED; > >> goto out; > >> } > >> > >> The last field in the 1.0 struct was ManufacturerID, so you should > >> be able to detect 1.0 clients that expect that based on the size they > >> pass in. > > > > The patch doesn't show the entire u-boot code there and is a bit misleading. > > There's another check that precedes the one I am patching, which if I > > am reading it correctly does exactly what you describe > > > > #define BOOT_SERVICE_CAPABILITY_MIN \ > > offsetof(struct efi_tcg2_boot_service_capability, > > number_of_pcr_banks) > > if (capability->size < BOOT_SERVICE_CAPABILITY_MIN) { > > capability->size = BOOT_SERVICE_CAPABILITY_MIN; > > efi_ret = EFI_BUFFER_TOO_SMALL; > > goto out; > > } > > I took a look at the u-boot code. What I think we want is this: > > diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h > index b1c3abd097..6a46d9e51c 100644 > --- a/include/efi_tcg2.h > +++ b/include/efi_tcg2.h > @@ -126,7 +126,7 @@ struct efi_tcg2_boot_service_capability { > }; > > /* up to and including the vendor ID (manufacturer_id) field */ > -#define BOOT_SERVICE_CAPABILITY_MIN \ > +#define BOOT_SERVICE_CAPABILITY_1_0 \ > offsetof(struct efi_tcg2_boot_service_capability, > number_of_pcr_banks) > > #define TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03 "Spec ID Event03" > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > index a83ae7a46c..d37b896b7e 100644 > --- a/lib/efi_loader/efi_tcg2.c > +++ b/lib/efi_loader/efi_tcg2.c > @@ -742,15 +742,15 @@ efi_tcg2_get_capability(struct efi_tcg2_protocol > *this, > goto out; > } > > - if (capability->size < BOOT_SERVICE_CAPABILITY_MIN) { > - capability->size = BOOT_SERVICE_CAPABILITY_MIN; > + if (capability->size < BOOT_SERVICE_CAPABILITY_1_0) { > + capability->size = sizeof(*capability); > efi_ret = EFI_BUFFER_TOO_SMALL; > goto out; > } > > if (capability->size < sizeof(*capability)) { > capability->size = sizeof(*capability); > - efi_ret = EFI_BUFFER_TOO_SMALL; > + efi_ret = EFI_UNSUPPORTED; > goto out; > } > > That is: > > -if the passed in size is less than the 1.0 struct size > then return sizeof() the full struct. This allows > client to query and determine the supported struct > size > > -to detect and reject 1.0 clients, if the passed in > size is >= 1.0 struct size AND less than sizeof(*capability) > then return EFI_UNSUPPORTED > > Does that make sense? It does but that violates the spec once again. The spec says that the firmware should return BOOT_SERVICE_CAPABILITY_MIN if the size is less than the size of the EFI_TCG2_BOOT_SERVICE_CAPABILITY. The current logic is similar, the only thing that differs on your patch is the size of the struct we return. Thanks /Ilias > > Thanks, > Stuart