On Fri, 8 Mar 2024 at 09:58, Ard Biesheuvel <ardb+git@xxxxxxxxxx> wrote:
>
> From: Ard Biesheuvel <ardb@xxxxxxxxxx>
>
> Our efi_tcg2_tagged_event is not defined in the EFI spec, but it is not
> a local invention either: it was taken from the TCG PC Client spec,
> where it is called TCG_PCClientTaggedEvent.
>
> This spec also contains some guidance on how to populate it, which
> is not being followed closely at the moment; the event size should cover
> the TCG_PCClientTaggedEvent and its payload only, but it currently
> covers the preceding efi_tcg2_event too, and this may result in trailing
> garbage being measured into the TPM.
>
> So rename the struct and document its provenance, and fix up the use so
> only the tagged event data is represented in the size field.
>
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> ---
> drivers/firmware/efi/libstub/efi-stub-helper.c | 20 +++++++++++---------
> drivers/firmware/efi/libstub/efistub.h | 12 ++++++------
> 2 files changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index bfa30625f5d0..16843ab9b64d 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -11,6 +11,7 @@
>
> #include <linux/efi.h>
> #include <linux/kernel.h>
> +#include <linux/overflow.h>
> #include <asm/efi.h>
> #include <asm/setup.h>
>
> @@ -219,23 +220,24 @@ static const struct {
> },
> };
>
> +struct efistub_measured_event {
> + efi_tcg2_event_t event_data;
> + TCG_PCClientTaggedEvent tagged_event;
> +} __packed;
> +
> static efi_status_t efi_measure_tagged_event(unsigned long load_addr,
> unsigned long load_size,
> enum efistub_event event)
> {
> + struct efistub_measured_event *evt;
> + int size = struct_size(&evt->tagged_event, tagged_event_data,
> + events[event].event_data_len);
> efi_guid_t tcg2_guid = EFI_TCG2_PROTOCOL_GUID;
> efi_tcg2_protocol_t *tcg2 = NULL;
> efi_status_t status;
>
> efi_bs_call(locate_protocol, &tcg2_guid, NULL, (void **)&tcg2);
> if (tcg2) {
> - struct efi_measured_event {
> - efi_tcg2_event_t event_data;
> - efi_tcg2_tagged_event_t tagged_event;
> - u8 tagged_event_data[];
> - } *evt;
> - int size = sizeof(*evt) + events[event].event_data_len;
> -
> status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, size,
OK, now this size is wrong - this should be 'sizeof(efi_tcg2_event_t) + size'
> (void **)&evt);
> if (status != EFI_SUCCESS)
> @@ -249,12 +251,12 @@ static efi_status_t efi_measure_tagged_event(unsigned long load_addr,
> .event_header.event_type = EV_EVENT_TAG,
> };
>
> - evt->tagged_event = (struct efi_tcg2_tagged_event){
> + evt->tagged_event = (TCG_PCClientTaggedEvent){
> .tagged_event_id = events[event].event_id,
> .tagged_event_data_size = events[event].event_data_len,
> };
>
> - memcpy(evt->tagged_event_data, events[event].event_data,
> + memcpy(evt->tagged_event.tagged_event_data, events[event].event_data,
> events[event].event_data_len);
>
> status = efi_call_proto(tcg2, hash_log_extend_event, 0,
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index c04b82ea40f2..043a3ff435f3 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -843,14 +843,14 @@ struct efi_tcg2_event {
> /* u8[] event follows here */
> } __packed;
>
> -struct efi_tcg2_tagged_event {
> - u32 tagged_event_id;
> - u32 tagged_event_data_size;
> - /* u8 tagged event data follows here */
> -} __packed;
> +/* from TCG PC Client Platform Firmware Profile Specification */
> +typedef struct tdTCG_PCClientTaggedEvent {
> + u32 tagged_event_id;
> + u32 tagged_event_data_size;
> + u8 tagged_event_data[];
> +} TCG_PCClientTaggedEvent;
>
> typedef struct efi_tcg2_event efi_tcg2_event_t;
> -typedef struct efi_tcg2_tagged_event efi_tcg2_tagged_event_t;
> typedef union efi_tcg2_protocol efi_tcg2_protocol_t;
>
> union efi_tcg2_protocol {
> --
> 2.44.0.278.ge034bb2e1d-goog
>
