| Message ID | 20211125060727.26093-1-masahisa.kojima@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | efi_loader: check tcg2 protocol installation outside the TCG protocol | expand |
Hello Kojima-san On Thu, 25 Nov 2021 at 08:04, Masahisa Kojima <masahisa.kojima@linaro.org> wrote: > > There are functions that calls tcg2_agile_log_append() outside > of the TCG protocol invocation (e.g tcg2_measure_pe_image). > These functions must to check that tcg2 protocol is installed. > If not, measurement shall be skipped. > > Together with above change, this commit also removes the > unnecessary tcg2_uninit() call in efi_tcg2_register(), > refactors efi_tcg2_register() not to include the process > that is not related to the tcg2 protocol registration. > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > --- > include/efi_loader.h | 4 ++ > lib/efi_loader/efi_setup.c | 3 ++ > lib/efi_loader/efi_tcg2.c | 89 +++++++++++++++++++++++++++++++------- > 3 files changed, 81 insertions(+), 15 deletions(-) > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index d52e399841..fe29c10906 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -525,6 +525,10 @@ efi_status_t efi_disk_register(void); > efi_status_t efi_rng_register(void); > /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */ > efi_status_t efi_tcg2_register(void); > +/* Called by efi_init_obj_list() to do the required setup for the measurement */ > +efi_status_t efi_tcg2_setup_measurement(void); > +/* Called by efi_init_obj_list() to do initial measurement */ > +efi_status_t efi_tcg2_do_initial_measurement(void); > /* measure the pe-coff image, extend PCR and add Event Log */ > efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size, > struct efi_loaded_image_obj *handle, > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c > index a2338d74af..781d10590d 100644 > --- a/lib/efi_loader/efi_setup.c > +++ b/lib/efi_loader/efi_setup.c > @@ -271,6 +271,9 @@ efi_status_t efi_init_obj_list(void) > ret = efi_tcg2_register(); > if (ret != EFI_SUCCESS) > goto out; > + > + efi_tcg2_setup_measurement(); > + efi_tcg2_do_initial_measurement(); > } > > /* Secure boot */ > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > index 8c1f22e337..46300cae79 100644 > --- a/lib/efi_loader/efi_tcg2.c > +++ b/lib/efi_loader/efi_tcg2.c > @@ -153,6 +153,20 @@ static u16 alg_to_len(u16 hash_alg) > return 0; > } > > +/** > + * is_tcg2_protocol_installed - chech whether tcg2 protocol is installed > + * > + * @Return: true if tcg2 protocol is installed, false if not > + */ > +bool is_tcg2_protocol_installed(void) > +{ > + struct efi_handler *handler; > + efi_status_t ret; > + > + ret = efi_search_protocol(efi_root, &efi_guid_tcg2_protocol, &handler); > + return ((ret == EFI_SUCCESS) ? true : false); > +} > + > static u32 tcg_event_final_size(struct tpml_digest_values *digest_list) > { > u32 len; > @@ -886,6 +900,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size, > IMAGE_NT_HEADERS32 *nt; > struct efi_handler *handler; > > + if (!is_tcg2_protocol_installed()) > + return EFI_NOT_READY; > + > ret = platform_get_tpm2_device(&dev); > if (ret != EFI_SUCCESS) > return ret; > @@ -1759,6 +1776,9 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha > u32 event = 0; > struct smbios_entry *entry; > > + if (!is_tcg2_protocol_installed()) > + return EFI_NOT_READY; > + > if (tcg2_efi_app_invoked) > return EFI_SUCCESS; > > @@ -1809,6 +1829,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void) > efi_status_t ret; > struct udevice *dev; > > + if (!is_tcg2_protocol_installed()) > + return EFI_NOT_READY; > + > ret = platform_get_tpm2_device(&dev); > if (ret != EFI_SUCCESS) > return ret; > @@ -1833,6 +1856,11 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context) > > EFI_ENTRY("%p, %p", event, context); > > + if (!is_tcg2_protocol_installed()) { > + ret = EFI_NOT_READY; > + goto out; > + } > + > event_log.ebs_called = true; > ret = platform_get_tpm2_device(&dev); > if (ret != EFI_SUCCESS) > @@ -1863,6 +1891,9 @@ efi_status_t efi_tcg2_notify_exit_boot_services_failed(void) > struct udevice *dev; > efi_status_t ret; > > + if (!is_tcg2_protocol_installed()) > + return EFI_NOT_READY; > + > ret = platform_get_tpm2_device(&dev); > if (ret != EFI_SUCCESS) > goto out; > @@ -1932,6 +1963,45 @@ error: > return ret; > } > > +/** > + * efi_tcg2_setup_measurement() - setup for the measurement > + * > + * Return: status code > + */ > +efi_status_t efi_tcg2_setup_measurement(void) > +{ > + efi_status_t ret; > + struct efi_event *event; > + > + ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK, > + efi_tcg2_notify_exit_boot_services, NULL, > + NULL, &event); > + > + return ret; > +} > + > +/** > + * efi_tcg2_do_initial_measurement() - do initial measurement > + * > + * Return: status code > + */ > +efi_status_t efi_tcg2_do_initial_measurement(void) > +{ > + efi_status_t ret; > + struct udevice *dev; > + > + ret = platform_get_tpm2_device(&dev); > + if (ret != EFI_SUCCESS) > + goto out; > + > + ret = tcg2_measure_secure_boot_variable(dev); > + if (ret != EFI_SUCCESS) > + goto out; > + > +out: > + return ret; > +} > + > /** > * efi_tcg2_register() - register EFI_TCG2_PROTOCOL > * > @@ -1943,7 +2013,6 @@ efi_status_t efi_tcg2_register(void) > { > efi_status_t ret = EFI_SUCCESS; > struct udevice *dev; > - struct efi_event *event; > u32 err; > > ret = platform_get_tpm2_device(&dev); > @@ -1969,6 +2038,10 @@ efi_status_t efi_tcg2_register(void) > goto fail; > } > > + /* > + * efi_add_protocol() is called at last on purpose. > + * tcg2_uninit() does not uninstall the tcg2 protocol, but it is intended. > + */ > ret = efi_add_protocol(efi_root, &efi_guid_tcg2_protocol, > (void *)&efi_tcg2_protocol); > if (ret != EFI_SUCCESS) { > @@ -1976,20 +2049,6 @@ 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(); > - goto fail; > - } > - > return ret; > > fail: > -- > 2.17.1 > This looks reasonable. I've just finished reviewing Ruchika's patchset adding TF-A eventlog support though [1]. Can you please rebase this on top of that series? [1] https://lore.kernel.org/u-boot/CAC_iWjLPfwos2_x9e536jxwufQ3g7JhuzyEBf8s70MBEqfOadA@mail.gmail.com/ Thanks /Ilias
Hi Ilias, On Thu, 25 Nov 2021 at 17:25, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hello Kojima-san > > On Thu, 25 Nov 2021 at 08:04, Masahisa Kojima > <masahisa.kojima@linaro.org> wrote: > > > > There are functions that calls tcg2_agile_log_append() outside > > of the TCG protocol invocation (e.g tcg2_measure_pe_image). > > These functions must to check that tcg2 protocol is installed. > > If not, measurement shall be skipped. > > > > Together with above change, this commit also removes the > > unnecessary tcg2_uninit() call in efi_tcg2_register(), > > refactors efi_tcg2_register() not to include the process > > that is not related to the tcg2 protocol registration. > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > > --- > > include/efi_loader.h | 4 ++ > > lib/efi_loader/efi_setup.c | 3 ++ > > lib/efi_loader/efi_tcg2.c | 89 +++++++++++++++++++++++++++++++------- > > 3 files changed, 81 insertions(+), 15 deletions(-) > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > index d52e399841..fe29c10906 100644 > > --- a/include/efi_loader.h > > +++ b/include/efi_loader.h > > @@ -525,6 +525,10 @@ efi_status_t efi_disk_register(void); > > efi_status_t efi_rng_register(void); > > /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */ > > efi_status_t efi_tcg2_register(void); > > +/* Called by efi_init_obj_list() to do the required setup for the measurement */ > > +efi_status_t efi_tcg2_setup_measurement(void); > > +/* Called by efi_init_obj_list() to do initial measurement */ > > +efi_status_t efi_tcg2_do_initial_measurement(void); > > /* measure the pe-coff image, extend PCR and add Event Log */ > > efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size, > > struct efi_loaded_image_obj *handle, > > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c > > index a2338d74af..781d10590d 100644 > > --- a/lib/efi_loader/efi_setup.c > > +++ b/lib/efi_loader/efi_setup.c > > @@ -271,6 +271,9 @@ efi_status_t efi_init_obj_list(void) > > ret = efi_tcg2_register(); > > if (ret != EFI_SUCCESS) > > goto out; > > + > > + efi_tcg2_setup_measurement(); > > + efi_tcg2_do_initial_measurement(); > > } > > > > /* Secure boot */ > > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > > index 8c1f22e337..46300cae79 100644 > > --- a/lib/efi_loader/efi_tcg2.c > > +++ b/lib/efi_loader/efi_tcg2.c > > @@ -153,6 +153,20 @@ static u16 alg_to_len(u16 hash_alg) > > return 0; > > } > > > > +/** > > + * is_tcg2_protocol_installed - chech whether tcg2 protocol is installed > > + * > > + * @Return: true if tcg2 protocol is installed, false if not > > + */ > > +bool is_tcg2_protocol_installed(void) > > +{ > > + struct efi_handler *handler; > > + efi_status_t ret; > > + > > + ret = efi_search_protocol(efi_root, &efi_guid_tcg2_protocol, &handler); > > + return ((ret == EFI_SUCCESS) ? true : false); > > +} > > + > > static u32 tcg_event_final_size(struct tpml_digest_values *digest_list) > > { > > u32 len; > > @@ -886,6 +900,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size, > > IMAGE_NT_HEADERS32 *nt; > > struct efi_handler *handler; > > > > + if (!is_tcg2_protocol_installed()) > > + return EFI_NOT_READY; > > + > > ret = platform_get_tpm2_device(&dev); > > if (ret != EFI_SUCCESS) > > return ret; > > @@ -1759,6 +1776,9 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha > > u32 event = 0; > > struct smbios_entry *entry; > > > > + if (!is_tcg2_protocol_installed()) > > + return EFI_NOT_READY; > > + > > if (tcg2_efi_app_invoked) > > return EFI_SUCCESS; > > > > @@ -1809,6 +1829,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void) > > efi_status_t ret; > > struct udevice *dev; > > > > + if (!is_tcg2_protocol_installed()) > > + return EFI_NOT_READY; > > + > > ret = platform_get_tpm2_device(&dev); > > if (ret != EFI_SUCCESS) > > return ret; > > @@ -1833,6 +1856,11 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context) > > > > EFI_ENTRY("%p, %p", event, context); > > > > + if (!is_tcg2_protocol_installed()) { > > + ret = EFI_NOT_READY; > > + goto out; > > + } > > + > > event_log.ebs_called = true; > > ret = platform_get_tpm2_device(&dev); > > if (ret != EFI_SUCCESS) > > @@ -1863,6 +1891,9 @@ efi_status_t efi_tcg2_notify_exit_boot_services_failed(void) > > struct udevice *dev; > > efi_status_t ret; > > > > + if (!is_tcg2_protocol_installed()) > > + return EFI_NOT_READY; > > + > > ret = platform_get_tpm2_device(&dev); > > if (ret != EFI_SUCCESS) > > goto out; > > @@ -1932,6 +1963,45 @@ error: > > return ret; > > } > > > > +/** > > + * efi_tcg2_setup_measurement() - setup for the measurement > > + * > > + * Return: status code > > + */ > > +efi_status_t efi_tcg2_setup_measurement(void) > > +{ > > + efi_status_t ret; > > + struct efi_event *event; > > + > > + ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK, > > + efi_tcg2_notify_exit_boot_services, NULL, > > + NULL, &event); > > + > > + return ret; > > +} > > + > > +/** > > + * efi_tcg2_do_initial_measurement() - do initial measurement > > + * > > + * Return: status code > > + */ > > +efi_status_t efi_tcg2_do_initial_measurement(void) > > +{ > > + efi_status_t ret; > > + struct udevice *dev; > > + > > + ret = platform_get_tpm2_device(&dev); > > + if (ret != EFI_SUCCESS) > > + goto out; > > + > > + ret = tcg2_measure_secure_boot_variable(dev); > > + if (ret != EFI_SUCCESS) > > + goto out; > > + > > +out: > > + return ret; > > +} > > + > > /** > > * efi_tcg2_register() - register EFI_TCG2_PROTOCOL > > * > > @@ -1943,7 +2013,6 @@ efi_status_t efi_tcg2_register(void) > > { > > efi_status_t ret = EFI_SUCCESS; > > struct udevice *dev; > > - struct efi_event *event; > > u32 err; > > > > ret = platform_get_tpm2_device(&dev); > > @@ -1969,6 +2038,10 @@ efi_status_t efi_tcg2_register(void) > > goto fail; > > } > > > > + /* > > + * efi_add_protocol() is called at last on purpose. > > + * tcg2_uninit() does not uninstall the tcg2 protocol, but it is intended. > > + */ > > ret = efi_add_protocol(efi_root, &efi_guid_tcg2_protocol, > > (void *)&efi_tcg2_protocol); > > if (ret != EFI_SUCCESS) { > > @@ -1976,20 +2049,6 @@ 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(); > > - goto fail; > > - } > > - > > return ret; > > > > fail: > > -- > > 2.17.1 > > > > This looks reasonable. I've just finished reviewing Ruchika's > patchset adding TF-A eventlog support though [1]. Can you please > rebase this on top of that series? I have sent the rebased v2 patch. Thanks, Masahisa Kojima > > > [1] https://lore.kernel.org/u-boot/CAC_iWjLPfwos2_x9e536jxwufQ3g7JhuzyEBf8s70MBEqfOadA@mail.gmail.com/ > > Thanks > /Ilias
diff --git a/include/efi_loader.h b/include/efi_loader.h index d52e399841..fe29c10906 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -525,6 +525,10 @@ efi_status_t efi_disk_register(void); efi_status_t efi_rng_register(void); /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */ efi_status_t efi_tcg2_register(void); +/* Called by efi_init_obj_list() to do the required setup for the measurement */ +efi_status_t efi_tcg2_setup_measurement(void); +/* Called by efi_init_obj_list() to do initial measurement */ +efi_status_t efi_tcg2_do_initial_measurement(void); /* measure the pe-coff image, extend PCR and add Event Log */ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size, struct efi_loaded_image_obj *handle, diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c index a2338d74af..781d10590d 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -271,6 +271,9 @@ efi_status_t efi_init_obj_list(void) ret = efi_tcg2_register(); if (ret != EFI_SUCCESS) goto out; + + efi_tcg2_setup_measurement(); + efi_tcg2_do_initial_measurement(); } /* Secure boot */ diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 8c1f22e337..46300cae79 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -153,6 +153,20 @@ static u16 alg_to_len(u16 hash_alg) return 0; } +/** + * is_tcg2_protocol_installed - chech whether tcg2 protocol is installed + * + * @Return: true if tcg2 protocol is installed, false if not + */ +bool is_tcg2_protocol_installed(void) +{ + struct efi_handler *handler; + efi_status_t ret; + + ret = efi_search_protocol(efi_root, &efi_guid_tcg2_protocol, &handler); + return ((ret == EFI_SUCCESS) ? true : false); +} + static u32 tcg_event_final_size(struct tpml_digest_values *digest_list) { u32 len; @@ -886,6 +900,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size, IMAGE_NT_HEADERS32 *nt; struct efi_handler *handler; + if (!is_tcg2_protocol_installed()) + return EFI_NOT_READY; + ret = platform_get_tpm2_device(&dev); if (ret != EFI_SUCCESS) return ret; @@ -1759,6 +1776,9 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha u32 event = 0; struct smbios_entry *entry; + if (!is_tcg2_protocol_installed()) + return EFI_NOT_READY; + if (tcg2_efi_app_invoked) return EFI_SUCCESS; @@ -1809,6 +1829,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void) efi_status_t ret; struct udevice *dev; + if (!is_tcg2_protocol_installed()) + return EFI_NOT_READY; + ret = platform_get_tpm2_device(&dev); if (ret != EFI_SUCCESS) return ret; @@ -1833,6 +1856,11 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context) EFI_ENTRY("%p, %p", event, context); + if (!is_tcg2_protocol_installed()) { + ret = EFI_NOT_READY; + goto out; + } + event_log.ebs_called = true; ret = platform_get_tpm2_device(&dev); if (ret != EFI_SUCCESS) @@ -1863,6 +1891,9 @@ efi_status_t efi_tcg2_notify_exit_boot_services_failed(void) struct udevice *dev; efi_status_t ret; + if (!is_tcg2_protocol_installed()) + return EFI_NOT_READY; + ret = platform_get_tpm2_device(&dev); if (ret != EFI_SUCCESS) goto out; @@ -1932,6 +1963,45 @@ error: return ret; } +/** + * efi_tcg2_setup_measurement() - setup for the measurement + * + * Return: status code + */ +efi_status_t efi_tcg2_setup_measurement(void) +{ + efi_status_t ret; + struct efi_event *event; + + ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK, + efi_tcg2_notify_exit_boot_services, NULL, + NULL, &event); + + return ret; +} + +/** + * efi_tcg2_do_initial_measurement() - do initial measurement + * + * Return: status code + */ +efi_status_t efi_tcg2_do_initial_measurement(void) +{ + efi_status_t ret; + struct udevice *dev; + + ret = platform_get_tpm2_device(&dev); + if (ret != EFI_SUCCESS) + goto out; + + ret = tcg2_measure_secure_boot_variable(dev); + if (ret != EFI_SUCCESS) + goto out; + +out: + return ret; +} + /** * efi_tcg2_register() - register EFI_TCG2_PROTOCOL * @@ -1943,7 +2013,6 @@ efi_status_t efi_tcg2_register(void) { efi_status_t ret = EFI_SUCCESS; struct udevice *dev; - struct efi_event *event; u32 err; ret = platform_get_tpm2_device(&dev); @@ -1969,6 +2038,10 @@ efi_status_t efi_tcg2_register(void) goto fail; } + /* + * efi_add_protocol() is called at last on purpose. + * tcg2_uninit() does not uninstall the tcg2 protocol, but it is intended. + */ ret = efi_add_protocol(efi_root, &efi_guid_tcg2_protocol, (void *)&efi_tcg2_protocol); if (ret != EFI_SUCCESS) { @@ -1976,20 +2049,6 @@ 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(); - goto fail; - } - return ret; fail:
There are functions that calls tcg2_agile_log_append() outside of the TCG protocol invocation (e.g tcg2_measure_pe_image). These functions must to check that tcg2 protocol is installed. If not, measurement shall be skipped. Together with above change, this commit also removes the unnecessary tcg2_uninit() call in efi_tcg2_register(), refactors efi_tcg2_register() not to include the process that is not related to the tcg2 protocol registration. Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> --- include/efi_loader.h | 4 ++ lib/efi_loader/efi_setup.c | 3 ++ lib/efi_loader/efi_tcg2.c | 89 +++++++++++++++++++++++++++++++------- 3 files changed, 81 insertions(+), 15 deletions(-)