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
