On Fri, 9 Jul 2021 at 02:40, Heinrich Schuchardt <xypron.g...@gmx.de> wrote:
>
> On 7/7/21 3:36 PM, Masahisa Kojima wrote:
> > TCG PC Client PFP spec requires to measure
> > "Exit Boot Services Invocation" if ExitBootServices() is invoked.
> > Depending upon the return code from the ExitBootServices() call,
> > "Exit Boot Services Returned with Success" or "Exit Boot Services
> > Returned with Failure" is also measured.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org>
> > ---
> > include/efi_loader.h | 1 +
> > lib/efi_loader/efi_boottime.c | 5 +++
> > lib/efi_loader/efi_tcg2.c | 70 +++++++++++++++++++++++++++++++++++
> > 3 files changed, 76 insertions(+)
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index 281ffff30f..e9bd1aac08 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -407,6 +407,7 @@ efi_status_t efi_run_image(void *source_buffer,
> > efi_uintn_t source_size);
> > efi_status_t efi_init_variables(void);
> > /* Notify ExitBootServices() is called */
> > void efi_variables_boot_exit_notify(void);
> > +efi_status_t efi_tcg2_notify_exit_boot_services_failed(void);
> > /* Measure efi application invocation */
> > efi_status_t EFIAPI efi_tcg2_measure_efi_app_invocation(void);
> > /* Measure efi application exit */
> > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> > index 2914800c56..6e07ef65bc 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -2181,6 +2181,11 @@ static efi_status_t EFIAPI
> > efi_exit_boot_services(efi_handle_t image_handle,
> > efi_set_watchdog(0);
> > WATCHDOG_RESET();
> > out:
> > + if (ret != EFI_SUCCESS) {
> > + if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL))
> > + efi_tcg2_notify_exit_boot_services_failed();
> > + }
> > +
> > return EFI_EXIT(ret);
> > }
> >
> > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> > index 6e903e3cb3..823abd8217 100644
> > --- a/lib/efi_loader/efi_tcg2.c
> > +++ b/lib/efi_loader/efi_tcg2.c
> > @@ -1506,6 +1506,67 @@ efi_status_t EFIAPI
> > efi_tcg2_measure_efi_app_exit(void)
> > return ret;
> > }
> >
> > +/**
> > + * efi_tcg2_notify_exit_boot_services() - ExitBootService callback
> > + *
> > + * @event: callback event
> > + * @context: callback context
> > + */
> > +static void EFIAPI
> > +efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
> > +{
> > + efi_status_t ret;
> > + struct udevice *dev;
> > +
> > + EFI_ENTRY("%p, %p", event, context);
> > +
> > + ret = platform_get_tpm2_device(&dev);
> > + if (ret != EFI_SUCCESS)
> > + goto out;
> > +
> > + ret = tcg2_measure_event(dev, 5, EV_EFI_ACTION,
> > + strlen(EFI_EXIT_BOOT_SERVICES_INVOCATION),
> > + (u8 *)EFI_EXIT_BOOT_SERVICES_INVOCATION);
> > + if (ret != EFI_SUCCESS)
> > + goto out;
> > +
> > + ret = tcg2_measure_event(dev, 5, EV_EFI_ACTION,
> > + strlen(EFI_EXIT_BOOT_SERVICES_SUCCEEDED),
> > + (u8 *)EFI_EXIT_BOOT_SERVICES_SUCCEEDED);
> > +
> > +out:
> > + EFI_EXIT(ret);
> > +}
> > +
> > +/**
> > + * efi_tcg2_notify_exit_boot_services_failed()
> > + * - notify ExitBootServices() is failed
> > + *
> > + * Return: status code
> > + */
> > +efi_status_t efi_tcg2_notify_exit_boot_services_failed(void)
> > +{
> > + struct udevice *dev;
> > + efi_status_t ret;
> > +
> > + ret = platform_get_tpm2_device(&dev);
> > + if (ret != EFI_SUCCESS)
> > + goto out;
> > +
> > + ret = tcg2_measure_event(dev, 5, EV_EFI_ACTION,
> > + sizeof(EFI_EXIT_BOOT_SERVICES_INVOCATION),
> > + (u8 *)EFI_EXIT_BOOT_SERVICES_INVOCATION);
> > + if (ret != EFI_SUCCESS)
> > + goto out;
> > +
> > + ret = tcg2_measure_event(dev, 5, EV_EFI_ACTION,
> > + sizeof(EFI_EXIT_BOOT_SERVICES_FAILED),
> > + (u8 *)EFI_EXIT_BOOT_SERVICES_FAILED);
> > +
> > +out:
> > + return ret;
> > +}
>
> We must keep out code small. Please, carve out a function to which pass
> a the event type and an u8 string and use it where applicable.
I'm not sure I understand your comment correctly.
pcr_index is also required for carved out function because pcr_index
to extend PCR is different for each EV_EFI_ACTION event.
So the interface of carved out function is almost same as tcg2_measure_event().
I meant to create tcg2_measure_event() as a common sub-function.
to reduce the number of code.
Thanks,
Masahisa Kojima
>
> Best regards
>
> Heinrich
>
> > +
> > /**
> > * tcg2_measure_secure_boot_variable() - measure secure boot variables
> > *
> > @@ -1556,6 +1617,7 @@ efi_status_t efi_tcg2_register(void)
> > {
> > efi_status_t ret = EFI_SUCCESS;
> > struct udevice *dev;
> > + struct efi_event *event;
> >
> > ret = platform_get_tpm2_device(&dev);
> > if (ret != EFI_SUCCESS) {
> > @@ -1580,6 +1642,14 @@ efi_status_t efi_tcg2_register(void)
> > goto fail;
> > }
> >
> > + ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK,
> > + efi_tcg2_notify_exit_boot_services, NULL,
> > + NULL, &event);
> > + if (ret != EFI_SUCCESS) {
> > + tcg2_uninit();
> > + goto fail;
> > + }
> > +
> > ret = tcg2_measure_secure_boot_variable(dev);
> > if (ret != EFI_SUCCESS) {
> > tcg2_uninit();
> >
>
