On Tue, 5 Mar 2024 at 05:30, Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> wrote:
>
>
> On 3/4/24 2:44 AM, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@xxxxxxxxxx>
> >
> > In spite of the efi_ prefix, struct efi_tcg2_tagged_event is specific to
> > the EFI stub, and so we can tweak it to our liking if needed, e.g., to
> > accommodate the TDX variant of the TCG2 measurement protocol.
> >
> > In preparation for that, get rid of it entirely, and combine it with the
> > efi_measured_event struct used by the measurement code.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > ---
> > drivers/firmware/efi/libstub/efi-stub-helper.c | 26 ++++++++------------
> > drivers/firmware/efi/libstub/efistub.h | 18 ++++++++------
> > 2 files changed, 21 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> > index bfa30625f5d0..0dbc9d3f4abd 100644
> > --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> > +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> > @@ -193,7 +193,7 @@ void efi_apply_loadoptions_quirk(const void **load_options, u32 *load_options_si
> > *load_options_size = load_option_unpacked.optional_data_size;
> > }
> >
> > -enum efistub_event {
> > +enum efistub_event_type {
> > EFISTUB_EVT_INITRD,
> > EFISTUB_EVT_LOAD_OPTIONS,
> > EFISTUB_EVT_COUNT,
> > @@ -221,44 +221,38 @@ static const struct {
> >
> > static efi_status_t efi_measure_tagged_event(unsigned long load_addr,
> > unsigned long load_size,
> > - enum efistub_event event)
> > + enum efistub_event_type event)
> > {
> > + struct efistub_measured_event *evt;
> > + int size = struct_size(evt, tagged_event_data,
> > + events[event].event_data_len);
>
> Include linux/overflow.h explicitly?
>
Yes, good point.
> > 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,
> > (void **)&evt);
>
> It looks like in patch 3 you have converted evt as stack variable. Since that
> change is not specific to CC fallback, can it be moved here?
>
Not sure what you mean here. evt is still there after parch #3
> > if (status != EFI_SUCCESS)
> > goto fail;
> >
> > - evt->event_data = (struct efi_tcg2_event){
> > + evt->event_data.tcg2_data = (struct efi_tcg2_event){
> > .event_size = size,
> > - .event_header.header_size = sizeof(evt->event_data.event_header),
> > + .event_header.header_size = sizeof(evt->event_data.tcg2_data.event_header),
> > .event_header.header_version = EFI_TCG2_EVENT_HEADER_VERSION,
> > .event_header.pcr_index = events[event].pcr_index,
> > .event_header.event_type = EV_EVENT_TAG,
> > };
> >
> > - evt->tagged_event = (struct efi_tcg2_tagged_event){
> > - .tagged_event_id = events[event].event_id,
> > - .tagged_event_data_size = events[event].event_data_len,
> > - };
> > + evt->tagged_event_id = events[event].event_id;
> > + evt->tagged_event_data_size = events[event].event_data_len;
> >
> > memcpy(evt->tagged_event_data, events[event].event_data,
> > events[event].event_data_len);
> >
> > status = efi_call_proto(tcg2, hash_log_extend_event, 0,
> > - load_addr, load_size, &evt->event_data);
> > + load_addr, load_size, &evt->event_data.tcg2_data);
> > efi_bs_call(free_pool, evt);
> >
> > if (status != EFI_SUCCESS)
> > diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> > index c04b82ea40f2..b2c50dce48b8 100644
> > --- a/drivers/firmware/efi/libstub/efistub.h
> > +++ b/drivers/firmware/efi/libstub/efistub.h
> > @@ -843,14 +843,7 @@ 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;
> > -
> > 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 {
> > @@ -882,6 +875,17 @@ union efi_tcg2_protocol {
> > } mixed_mode;
> > };
> >
> > +union efistub_event {
> > + efi_tcg2_event_t tcg2_data;
> > +};
> > +
> > +struct efistub_measured_event {
> > + union efistub_event event_data;
> > + u32 tagged_event_id;
> > + u32 tagged_event_data_size;
> > + u8 tagged_event_data[];
> > +} __packed;
> > +
>
> Since efistub_measured_event is only used efi-stub-helper.c, why
> not leave it there?
>
Indeed. I will move it back.
