[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAC_iWj+p6gmcb4-_-f8Kr4Ds6EmSsq2D4GK1XymyybX7i1TMFw@mail.gmail.com>
Date: Mon, 19 Feb 2024 08:38:23 +0200
From: Ilias Apalodimas <ilias.apalodimas@...aro.org>
To: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
Cc: Ard Biesheuvel <ardb@...nel.org>, linux-kernel@...r.kernel.org,
linux-efi@...r.kernel.org
Subject: Re: [PATCH v2 1/2] efi/libstub: Add Confidential Computing (CC)
measurement support
Hi Kuppuswamy,
On Thu, 15 Feb 2024 at 05:02, Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@...ux.intel.com> wrote:
>
> If the virtual firmware implements TPM support, TCG2 protocol will be
> used for kernel measurements and event logging support. But in CC
> environment, not all platforms support or enable the TPM feature. UEFI
> specification [1] exposes protocol and interfaces used for kernel
> measurements in CC platforms without TPM support.
>
> Currently, the efi-stub only supports the kernel related measurements
> for the platform that supports TCG2 protocol. So, extend it add
> CC measurement protocol (EFI_CC_MEASUREMENT_PROTOCOL) and event logging
> support. Event logging format in the CC environment is the same as
> TCG2.
>
> More details about the EFI CC measurements and logging can be found
> in [1].
>
> Link: https://uefi.org/specs/UEFI/2.10/38_Confidential_Computing.html#efi-cc-measurement-protocol [1]
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
> ---
>
> Changes since v1:
> * Fixed missing tagged event data.
>
> .../firmware/efi/libstub/efi-stub-helper.c | 127 ++++++++++++++----
> drivers/firmware/efi/libstub/efistub.h | 74 ++++++++++
> include/linux/efi.h | 1 +
> 3 files changed, 174 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index bfa30625f5d0..cc31f8143190 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -219,50 +219,121 @@ static const struct {
> },
> };
>
> +static efi_status_t tcg2_efi_measure(efi_tcg2_protocol_t *tcg2,
> + unsigned long load_addr,
> + unsigned long load_size,
> + enum efistub_event event)
> +{
> + 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;
This is defined as size_t on the cc variant. I guess both are ok, just pick one
> + efi_status_t status;
> +
> + status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, size,
> + (void **)&evt);
> + if (status != EFI_SUCCESS)
pr_err() here as done in the cc variant?
> + return status;
> +
> + evt->event_data = (struct efi_tcg2_event){
> + .event_size = size,
> + .event_header.header_size = sizeof(evt->event_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,
> + };
> +
> + 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);
The struct filling/memcpying looks similar across the 2 functions. I
wonder if it makes sense to have a common function for that, with an
argument for the event data type.
> + efi_bs_call(free_pool, evt);
> +
> + return status;
> +}
> +
> +static efi_status_t cc_efi_measure(efi_cc_protocol_t *cc,
> + unsigned long load_addr,
> + unsigned long load_size,
> + enum efistub_event event)
> +{
> + struct efi_measured_event {
> + efi_cc_event_t event_data;
> + efi_tcg2_tagged_event_t tagged_event;
> + u8 tagged_event_data[];
> + } *evt;
> + size_t size = sizeof(*evt) + events[event].event_data_len;
> + efi_cc_mr_index_t mr;
> + efi_status_t status;
> +
> + status = efi_call_proto(cc, map_pcr_to_mr_index, events[event].pcr_index, &mr);
> + if (status != EFI_SUCCESS) {
> + efi_err("CC_MEASURE: PCR to MR mapping failed\n");
> + return status;
> + }
> +
> + status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, size, (void **)&evt);
> + if (status != EFI_SUCCESS) {
> + efi_err("CC_MEASURE: Allocating event struct failed\n");
> + return status;
> + }
> +
> + evt->event_data = (struct efi_cc_event){
> + .event_size = size,
> + .event_header.header_size = sizeof(evt->event_data.event_header),
> + .event_header.header_version = EFI_CC_EVENT_HEADER_VERSION,
> + .event_header.mr_index = mr,
> + .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,
> + };
> +
> + memcpy(evt->tagged_event_data, events[event].event_data,
> + events[event].event_data_len);
> +
> + status = efi_call_proto(cc, hash_log_extend_event, 0,
> + load_addr, load_size, &evt->event_data);
> +
> + efi_bs_call(free_pool, evt);
> +
> + return status;
> +}
> static efi_status_t efi_measure_tagged_event(unsigned long load_addr,
> unsigned long load_size,
> enum efistub_event event)
> {
> efi_guid_t tcg2_guid = EFI_TCG2_PROTOCOL_GUID;
> + efi_guid_t cc_guid = EFI_CC_MEASUREMENT_PROTOCOL_GUID;
> + efi_cc_protocol_t *cc = NULL;
> 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);
> + status = tcg2_efi_measure(tcg2, load_addr, load_size, event);
> if (status != EFI_SUCCESS)
> goto fail;
>
> - evt->event_data = (struct efi_tcg2_event){
> - .event_size = size,
> - .event_header.header_size = sizeof(evt->event_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,
> - };
> -
> - 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);
> - efi_bs_call(free_pool, evt);
> + return EFI_SUCCESS;
> + }
>
> + efi_bs_call(locate_protocol, &cc_guid, NULL, (void **)&cc);
> + if (cc) {
> + status = cc_efi_measure(cc, load_addr, load_size, event);
> if (status != EFI_SUCCESS)
> goto fail;
> +
> return EFI_SUCCESS;
> }
>
[...]
Thanks
/Ilias
Powered by blists - more mailing lists