Re: [PATCH 3/4] efi/libstub: Measure into CC protocol if TCG2 protocol is absent

Kuppuswamy Sathyanarayanan
2024-03-05T21:39:12Z

On 3/4/24 2:44 AM, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
>
> To accommodate confidential compute VMs that expose the simplified CC
> measurement protocol instead of the full-blown TCG2 one, fall back to
> the former if the latter does not exist.
>
> The CC protocol was designed to be used in this manner, which is why the
> types and prototypes have been kept the same where possible. So reuse
> the existing code, and only deviate from the TCG2 code path where
> needed.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---

Looks fine to me.

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> drivers/firmware/efi/libstub/efi-stub-helper.c | 67 +++++++++++++++-----
> drivers/firmware/efi/libstub/efistub.h | 3 +
> 2 files changed, 53 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index 0dbc9d3f4abd..21f4567324f6 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -223,44 +223,77 @@ static efi_status_t efi_measure_tagged_event(unsigned long load_addr,
> unsigned long load_size,
> enum efistub_event_type event)
> {
> + union {
> + efi_status_t
> + (__efiapi *hash_log_extend_event)(void *, u64, efi_physical_addr_t,
> + u64, const union efistub_event *);
> + struct { u32 hash_log_extend_event; } mixed_mode;
> + } method;
> struct efistub_measured_event *evt;
> int size = struct_size(evt, tagged_event_data,
> events[event].event_data_len);
> efi_guid_t tcg2_guid = EFI_TCG2_PROTOCOL_GUID;
> efi_tcg2_protocol_t *tcg2 = NULL;
> + union efistub_event ev;
> efi_status_t status;
> + void *protocol;
>
> efi_bs_call(locate_protocol, &tcg2_guid, NULL, (void **)&tcg2);
> if (tcg2) {
> - status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, size,
> - (void **)&evt);
> - if (status != EFI_SUCCESS)
> - goto fail;
> -
> - evt->event_data.tcg2_data = (struct efi_tcg2_event){
> + ev.tcg2_data = (struct efi_tcg2_event){
> .event_size = size,
> - .event_header.header_size = sizeof(evt->event_data.tcg2_data.event_header),
> + .event_header.header_size = sizeof(ev.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,
> };
> + protocol = tcg2;
> + method.hash_log_extend_event =
> + (void *)efi_table_attr(tcg2, hash_log_extend_event);
> + } else {
> + efi_guid_t cc_guid = EFI_CC_MEASUREMENT_PROTOCOL_GUID;
> + efi_cc_protocol_t *cc = NULL;
>
> - 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);
> + efi_bs_call(locate_protocol, &cc_guid, NULL, (void **)&cc);
> + if (!cc)
> + return EFI_UNSUPPORTED;
>
> - status = efi_call_proto(tcg2, hash_log_extend_event, 0,
> - load_addr, load_size, &evt->event_data.tcg2_data);
> - efi_bs_call(free_pool, evt);
> + ev.cc_data = (struct efi_cc_event){
> + .event_size = size,
> + .event_header.header_size = sizeof(ev.cc_data.event_header),
> + .event_header.header_version = EFI_CC_EVENT_HEADER_VERSION,
> + .event_header.event_type = EV_EVENT_TAG,
> + };
>
> + status = efi_call_proto(cc, map_pcr_to_mr_index,
> + events[event].pcr_index,
> + &ev.cc_data.event_header.mr_index);
> if (status != EFI_SUCCESS)
> goto fail;
> - return EFI_SUCCESS;
> +
> + protocol = cc;
> + method.hash_log_extend_event =
> + (void *)efi_table_attr(cc, hash_log_extend_event);
> }
>
> - return EFI_UNSUPPORTED;
> + status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, size, (void **)&evt);
> + if (status != EFI_SUCCESS)
> + goto fail;
> +
> + evt->event_data = ev;
> + 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_fn_call(&method, hash_log_extend_event, protocol, 0,
> + load_addr, load_size, &evt->event_data);
> + efi_bs_call(free_pool, evt);
> +
> + if (status == EFI_SUCCESS)
> + return EFI_SUCCESS;
> +
> fail:
> efi_warn("Failed to measure data for event %d: 0x%lx\n", event, status);
> return status;
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index d621bfb719c4..4bf9a76796b7 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -954,8 +954,11 @@ union efi_cc_protocol {
> } mixed_mode;
> };
>
> +static_assert(sizeof(efi_tcg2_event_t) == sizeof(efi_cc_event_t));
> +
> union efistub_event {
> efi_tcg2_event_t tcg2_data;
> + efi_cc_event_t cc_data;
> };
>
> struct efistub_measured_event {

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer