diff mbox series

[2/7] efi_loader: fix the return values on efi_tcg

Message ID 20240622143601.187723-3-ilias.apalodimas@linaro.org
State Superseded
Headers show
Series The great TCG deduplication saga | expand

Commit Message

Ilias Apalodimas June 22, 2024, 2:35 p.m. UTC
A while back we moved the core functions of the EFI TCG protocol to the
TPM APIs in order for them to be used with bootm, booti etc.
Some prototypes changed from returning efi_status_t to int, which is more
appropriate for the non-EFI APIs. However, some of the EFI callsites never
changed and we ended up assigning the int value to efi_status_t.

This is unlikely to cause any problems, apart from returning invalid
values on failures and violating the EFI spec. Let's fix them
by looking at the new return code and map it to the proper EFI return
code on failures.

Fixes: commit 97707f12fdab ("tpm: Support boot measurements")
Fixes: commit d6b55a420cfc ("efi_loader: startup the tpm device when installing the protocol")
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 lib/efi_loader/efi_tcg2.c | 121 ++++++++++++++++++++------------------
 1 file changed, 64 insertions(+), 57 deletions(-)

Comments

Heinrich Schuchardt June 22, 2024, 3:55 p.m. UTC | #1
On 22.06.24 16:35, Ilias Apalodimas wrote:
> A while back we moved the core functions of the EFI TCG protocol to the
> TPM APIs in order for them to be used with bootm, booti etc.
> Some prototypes changed from returning efi_status_t to int, which is more
> appropriate for the non-EFI APIs. However, some of the EFI callsites never
> changed and we ended up assigning the int value to efi_status_t.
>
> This is unlikely to cause any problems, apart from returning invalid
> values on failures and violating the EFI spec. Let's fix them
> by looking at the new return code and map it to the proper EFI return
> code on failures.
>
> Fixes: commit 97707f12fdab ("tpm: Support boot measurements")
> Fixes: commit d6b55a420cfc ("efi_loader: startup the tpm device when installing the protocol")
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>   lib/efi_loader/efi_tcg2.c | 121 ++++++++++++++++++++------------------
>   1 file changed, 64 insertions(+), 57 deletions(-)
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index d56bd5657c8a..10c09caac35a 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -257,8 +257,8 @@ efi_tcg2_get_capability(struct efi_tcg2_protocol *this,
>   	capability->protocol_version.major = 1;
>   	capability->protocol_version.minor = 1;
>
> -	efi_ret = tcg2_platform_get_tpm2(&dev);
> -	if (efi_ret != EFI_SUCCESS) {
> +	ret = tcg2_platform_get_tpm2(&dev);
> +	if (ret) {
>   		capability->supported_event_logs = 0;
>   		capability->hash_algorithm_bitmap = 0;
>   		capability->tpm_present_flag = false;
> @@ -353,8 +353,7 @@ efi_tcg2_get_eventlog(struct efi_tcg2_protocol *this,
>   		goto out;
>   	}
>
> -	ret = tcg2_platform_get_tpm2(&dev);
> -	if (ret != EFI_SUCCESS) {
> +	if (tcg2_platform_get_tpm2(&dev)) {
>   		event_log_location = NULL;
>   		event_log_last_entry = NULL;
>   		*event_log_truncated = false;
> @@ -389,7 +388,7 @@ static efi_status_t tcg2_hash_pe_image(void *efi, u64 efi_size,
>   	void *new_efi = NULL;
>   	u8 hash[TPM2_SHA512_DIGEST_SIZE];
>   	struct udevice *dev;
> -	efi_status_t ret;
> +	efi_status_t ret = EFI_SUCCESS;
>   	u32 active;
>   	int i;
>
> @@ -404,12 +403,13 @@ static efi_status_t tcg2_hash_pe_image(void *efi, u64 efi_size,
>   		goto out;
>   	}
>
> -	ret = tcg2_platform_get_tpm2(&dev);
> -	if (ret != EFI_SUCCESS)
> +	if (tcg2_platform_get_tpm2(&dev)) {
> +		ret = EFI_DEVICE_ERROR;
>   		goto out;
> +	}
>
> -	ret = tcg2_get_active_pcr_banks(dev, &active);
> -	if (ret != EFI_SUCCESS) {
> +	if (tcg2_get_active_pcr_banks(dev, &active)) {
> +		ret = EFI_DEVICE_ERROR;
>   		goto out;
>   	}
>
> @@ -473,12 +473,12 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
>   	IMAGE_DOS_HEADER *dos;
>   	IMAGE_NT_HEADERS32 *nt;
>   	struct efi_handler *handler;
> +	int rc;
>
>   	if (!is_tcg2_protocol_installed())
>   		return EFI_SUCCESS;
>
> -	ret = tcg2_platform_get_tpm2(&dev);
> -	if (ret != EFI_SUCCESS)
> +	if (tcg2_platform_get_tpm2(&dev))
>   		return EFI_SECURITY_VIOLATION;
>
>   	switch (handle->image_type) {
> @@ -502,9 +502,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
>   	if (ret != EFI_SUCCESS)
>   		return ret;
>
> -	ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
> -	if (ret != EFI_SUCCESS)
> -		return ret;
> +	rc = tcg2_pcr_extend(dev, pcr_index, &digest_list);
> +	if (rc)
> +		return EFI_DEVICE_ERROR;
>
>   	ret = efi_search_protocol(&handle->header,
>   				  &efi_guid_loaded_image_device_path, &handler);
> @@ -574,9 +574,10 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags,
>   			       struct efi_tcg2_event *efi_tcg_event)
>   {
>   	struct udevice *dev;
> -	efi_status_t ret;
> +	efi_status_t ret = EFI_SUCCESS;
>   	u32 event_type, pcr_index, event_size;
>   	struct tpml_digest_values digest_list;
> +	int rc = 0;
>
>   	EFI_ENTRY("%p, %llu, %llu, %llu, %p", this, flags, data_to_hash,
>   		  data_to_hash_len, efi_tcg_event);
> @@ -586,9 +587,10 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags,
>   		goto out;
>   	}
>
> -	ret = tcg2_platform_get_tpm2(&dev);
> -	if (ret != EFI_SUCCESS)
> +	if (tcg2_platform_get_tpm2(&dev)) {
> +		ret = EFI_DEVICE_ERROR;
>   		goto out;
> +	}
>
>   	if (efi_tcg_event->size < efi_tcg_event->header.header_size +
>   	    sizeof(u32)) {
> @@ -621,8 +623,10 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags,
>   		ret = tcg2_hash_pe_image((void *)(uintptr_t)data_to_hash,
>   					 data_to_hash_len, &digest_list);
>   	} else {
> -		ret = tcg2_create_digest(dev, (u8 *)(uintptr_t)data_to_hash,
> -					 data_to_hash_len, &digest_list);
> +		rc = tcg2_create_digest(dev, (u8 *)(uintptr_t)data_to_hash,
> +					data_to_hash_len, &digest_list);
> +		if (rc)
> +			ret = EFI_DEVICE_ERROR;
>   	}
>
>   	if (ret != EFI_SUCCESS)
> @@ -631,9 +635,11 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags,
>   	pcr_index = efi_tcg_event->header.pcr_index;
>   	event_type = efi_tcg_event->header.event_type;
>
> -	ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
> -	if (ret != EFI_SUCCESS)
> +	rc = tcg2_pcr_extend(dev, pcr_index, &digest_list);
> +	if (rc) {
> +		ret = EFI_DEVICE_ERROR;
>   		goto out;
> +	}
>
>   	if (flags & EFI_TCG2_EXTEND_ONLY) {
>   		if (event_log.truncated)
> @@ -672,7 +678,7 @@ efi_tcg2_submit_command(struct efi_tcg2_protocol *this,
>   			u8 *output_param_block)
>   {
>   	struct udevice *dev;
> -	efi_status_t ret;
> +	efi_status_t ret = EFI_SUCCESS;
>   	u32 rc;
>   	size_t resp_buf_size = output_param_block_size;
>
> @@ -684,9 +690,10 @@ efi_tcg2_submit_command(struct efi_tcg2_protocol *this,
>   		goto out;
>   	}
>
> -	ret = tcg2_platform_get_tpm2(&dev);
> -	if (ret != EFI_SUCCESS)
> +	if (tcg2_platform_get_tpm2(&dev)) {
> +		ret = EFI_DEVICE_ERROR;
>   		goto out;
> +	}
>
>   	rc = tpm2_submit_command(dev, input_param_block,
>   				 output_param_block, &resp_buf_size);
> @@ -714,19 +721,20 @@ efi_tcg2_get_active_pcr_banks(struct efi_tcg2_protocol *this,
>   			      u32 *active_pcr_banks)
>   {
>   	struct udevice *dev;
> -	efi_status_t ret;
> +	efi_status_t ret = EFI_INVALID_PARAMETER;
>
>   	EFI_ENTRY("%p, %p", this, active_pcr_banks);
>
> -	if (!this || !active_pcr_banks) {
> -		ret = EFI_INVALID_PARAMETER;
> +	if (!this || !active_pcr_banks)
>   		goto out;
> -	}
> -	ret = tcg2_platform_get_tpm2(&dev);
> -	if (ret != EFI_SUCCESS)
> +
> +	if (tcg2_platform_get_tpm2(&dev))

EFI_INVALID_PARAMETER does not convey the problem type.
Should we return EFI_DEVICE_ERROR here?

The authors of the specification only foresaw one or more of the
parameters being incorrect (EFI_INVALID_PARAMETER).

> +		goto out;
> +
> +	if (tcg2_get_active_pcr_banks(dev, active_pcr_banks))

EFI_DEVICE_ERROR?

Best regards

Heinrich

>   		goto out;
>
> -	ret = tcg2_get_active_pcr_banks(dev, active_pcr_banks);
> +	ret = EFI_SUCCESS;
>
>   out:
>   	return EFI_EXIT(ret);
> @@ -852,14 +860,15 @@ static efi_status_t measure_event(struct udevice *dev, u32 pcr_index,
>   				  u32 event_type, u32 size, u8 event[])
>   {
>   	struct tpml_digest_values digest_list;
> -	efi_status_t ret;
> +	efi_status_t ret = EFI_DEVICE_ERROR;
> +	int rc;
>
> -	ret = tcg2_create_digest(dev, event, size, &digest_list);
> -	if (ret != EFI_SUCCESS)
> +	rc = tcg2_create_digest(dev, event, size, &digest_list);
> +	if (rc)
>   		goto out;
>
> -	ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
> -	if (ret != EFI_SUCCESS)
> +	rc = tcg2_pcr_extend(dev, pcr_index, &digest_list);
> +	if (rc)
>   		goto out;
>
>   	ret = tcg2_agile_log_append(pcr_index, event_type, &digest_list,
> @@ -901,10 +910,10 @@ static efi_status_t efi_init_event_log(void)
>   	struct tcg2_event_log elog;
>   	struct udevice *dev;
>   	efi_status_t ret;
> +	int rc;
>
> -	ret = tcg2_platform_get_tpm2(&dev);
> -	if (ret != EFI_SUCCESS)
> -		return ret;
> +	if (tcg2_platform_get_tpm2(&dev))
> +		return EFI_DEVICE_ERROR;
>
>   	ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, TPM2_EVENT_LOG_SIZE,
>   				(void **)&event_log.buffer);
> @@ -933,9 +942,11 @@ static efi_status_t efi_init_event_log(void)
>   	 */
>   	elog.log = event_log.buffer;
>   	elog.log_size = TPM2_EVENT_LOG_SIZE;
> -	ret = tcg2_log_prepare_buffer(dev, &elog, false);
> -	if (ret != EFI_SUCCESS)
> +	rc = tcg2_log_prepare_buffer(dev, &elog, false);
> +	if (rc) {
> +		ret = (rc == -ENOBUFS) ? EFI_BUFFER_TOO_SMALL : EFI_DEVICE_ERROR;
>   		goto free_pool;
> +	}
>
>   	event_log.pos = elog.log_position;
>
> @@ -1306,8 +1317,7 @@ efi_status_t efi_tcg2_measure_dtb(void *dtb)
>   	if (!is_tcg2_protocol_installed())
>   		return EFI_SUCCESS;
>
> -	ret = tcg2_platform_get_tpm2(&dev);
> -	if (ret != EFI_SUCCESS)
> +	if (tcg2_platform_get_tpm2(&dev))
>   		return EFI_SECURITY_VIOLATION;
>
>   	rsvmap_size = size_of_rsvmap(dtb);
> @@ -1356,8 +1366,7 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha
>   	if (tcg2_efi_app_invoked)
>   		return EFI_SUCCESS;
>
> -	ret = tcg2_platform_get_tpm2(&dev);
> -	if (ret != EFI_SUCCESS)
> +	if (tcg2_platform_get_tpm2(&dev))
>   		return EFI_SECURITY_VIOLATION;
>
>   	ret = tcg2_measure_boot_variable(dev);
> @@ -1406,9 +1415,8 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
>   	if (!is_tcg2_protocol_installed())
>   		return EFI_SUCCESS;
>
> -	ret = tcg2_platform_get_tpm2(&dev);
> -	if (ret != EFI_SUCCESS)
> -		return ret;
> +	if (tcg2_platform_get_tpm2(&dev))
> +		return EFI_SECURITY_VIOLATION;
>
>   	ret = measure_event(dev, 4, EV_EFI_ACTION,
>   			    strlen(EFI_RETURNING_FROM_EFI_APPLICATION),
> @@ -1437,9 +1445,10 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
>   		goto out;
>   	}
>
> -	ret = tcg2_platform_get_tpm2(&dev);
> -	if (ret != EFI_SUCCESS)
> +	if (tcg2_platform_get_tpm2(&dev)) {
> +		ret = EFI_SECURITY_VIOLATION;
>   		goto out;
> +	}
>
>   	ret = measure_event(dev, 5, EV_EFI_ACTION,
>   			    strlen(EFI_EXIT_BOOT_SERVICES_INVOCATION),
> @@ -1469,9 +1478,8 @@ efi_status_t efi_tcg2_notify_exit_boot_services_failed(void)
>   	if (!is_tcg2_protocol_installed())
>   		return EFI_SUCCESS;
>
> -	ret = tcg2_platform_get_tpm2(&dev);
> -	if (ret != EFI_SUCCESS)
> -		goto out;
> +	if (tcg2_platform_get_tpm2(&dev))
> +		return EFI_SECURITY_VIOLATION;
>
>   	ret = measure_event(dev, 5, EV_EFI_ACTION,
>   			    strlen(EFI_EXIT_BOOT_SERVICES_INVOCATION),
> @@ -1551,8 +1559,7 @@ efi_status_t efi_tcg2_do_initial_measurement(void)
>   	if (!is_tcg2_protocol_installed())
>   		return EFI_SUCCESS;
>
> -	ret = tcg2_platform_get_tpm2(&dev);
> -	if (ret != EFI_SUCCESS)
> +	if (tcg2_platform_get_tpm2(&dev))
>   		return EFI_SECURITY_VIOLATION;
>
>   	ret = tcg2_measure_secure_boot_variable(dev);
> @@ -1577,8 +1584,7 @@ efi_status_t efi_tcg2_register(void)
>   	struct efi_event *event;
>   	u32 err;
>
> -	ret = tcg2_platform_get_tpm2(&dev);
> -	if (ret != EFI_SUCCESS) {
> +	if (tcg2_platform_get_tpm2(&dev)) {
>   		log_warning("Missing TPMv2 device for EFI_TCG_PROTOCOL\n");
>   		return EFI_SUCCESS;
>   	}
> @@ -1586,6 +1592,7 @@ efi_status_t efi_tcg2_register(void)
>   	/* initialize the TPM as early as possible. */
>   	err = tpm_auto_start(dev);
>   	if (err) {
> +		ret = EFI_DEVICE_ERROR;
>   		log_err("TPM startup failed\n");
>   		goto fail;
>   	}
Ilias Apalodimas June 22, 2024, 4:09 p.m. UTC | #2
Hi Heinrich,

[...]

> >       rc = tpm2_submit_command(dev, input_param_block,
> >                                output_param_block, &resp_buf_size);
> > @@ -714,19 +721,20 @@ efi_tcg2_get_active_pcr_banks(struct efi_tcg2_protocol *this,
> >                             u32 *active_pcr_banks)
> >   {
> >       struct udevice *dev;
> > -     efi_status_t ret;
> > +     efi_status_t ret = EFI_INVALID_PARAMETER;
> >
> >       EFI_ENTRY("%p, %p", this, active_pcr_banks);
> >
> > -     if (!this || !active_pcr_banks) {
> > -             ret = EFI_INVALID_PARAMETER;
> > +     if (!this || !active_pcr_banks)
> >               goto out;
> > -     }
> > -     ret = tcg2_platform_get_tpm2(&dev);
> > -     if (ret != EFI_SUCCESS)
> > +
> > +     if (tcg2_platform_get_tpm2(&dev))
>
> EFI_INVALID_PARAMETER does not convey the problem type.
> Should we return EFI_DEVICE_ERROR here?
>
> The authors of the specification only foresaw one or more of the
> parameters being incorrect (EFI_INVALID_PARAMETER).

I completely agree that the result is misleading. However, I'd prefer
sticking to the spec for now and maybe add a comment?


>
> > +             goto out;
> > +
> > +     if (tcg2_get_active_pcr_banks(dev, active_pcr_banks))
>
> EFI_DEVICE_ERROR?

Same here

Thanks for the qucik review!
/Ilias
>
> Best regards
>
> Heinrich
>
> >               goto out;
> >
> > -     ret = tcg2_get_active_pcr_banks(dev, active_pcr_banks);
> > +     ret = EFI_SUCCESS;
> >
> >   out:
> >       return EFI_EXIT(ret);
> > @@ -852,14 +860,15 @@ static efi_status_t measure_event(struct udevice *dev, u32 pcr_index,
> >                                 u32 event_type, u32 size, u8 event[])
> >   {
> >       struct tpml_digest_values digest_list;
> > -     efi_status_t ret;
> > +     efi_status_t ret = EFI_DEVICE_ERROR;
> > +     int rc;
> >
> > -     ret = tcg2_create_digest(dev, event, size, &digest_list);
> > -     if (ret != EFI_SUCCESS)
> > +     rc = tcg2_create_digest(dev, event, size, &digest_list);
> > +     if (rc)
> >               goto out;
> >
> > -     ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
> > -     if (ret != EFI_SUCCESS)
> > +     rc = tcg2_pcr_extend(dev, pcr_index, &digest_list);
> > +     if (rc)
> >               goto out;
> >
> >       ret = tcg2_agile_log_append(pcr_index, event_type, &digest_list,
> > @@ -901,10 +910,10 @@ static efi_status_t efi_init_event_log(void)
> >       struct tcg2_event_log elog;
> >       struct udevice *dev;
> >       efi_status_t ret;
> > +     int rc;
> >
> > -     ret = tcg2_platform_get_tpm2(&dev);
> > -     if (ret != EFI_SUCCESS)
> > -             return ret;
> > +     if (tcg2_platform_get_tpm2(&dev))
> > +             return EFI_DEVICE_ERROR;
> >
> >       ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, TPM2_EVENT_LOG_SIZE,
> >                               (void **)&event_log.buffer);
> > @@ -933,9 +942,11 @@ static efi_status_t efi_init_event_log(void)
> >        */
> >       elog.log = event_log.buffer;
> >       elog.log_size = TPM2_EVENT_LOG_SIZE;
> > -     ret = tcg2_log_prepare_buffer(dev, &elog, false);
> > -     if (ret != EFI_SUCCESS)
> > +     rc = tcg2_log_prepare_buffer(dev, &elog, false);
> > +     if (rc) {
> > +             ret = (rc == -ENOBUFS) ? EFI_BUFFER_TOO_SMALL : EFI_DEVICE_ERROR;
> >               goto free_pool;
> > +     }
> >
> >       event_log.pos = elog.log_position;
> >
> > @@ -1306,8 +1317,7 @@ efi_status_t efi_tcg2_measure_dtb(void *dtb)
> >       if (!is_tcg2_protocol_installed())
> >               return EFI_SUCCESS;
> >
> > -     ret = tcg2_platform_get_tpm2(&dev);
> > -     if (ret != EFI_SUCCESS)
> > +     if (tcg2_platform_get_tpm2(&dev))
> >               return EFI_SECURITY_VIOLATION;
> >
> >       rsvmap_size = size_of_rsvmap(dtb);
> > @@ -1356,8 +1366,7 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha
> >       if (tcg2_efi_app_invoked)
> >               return EFI_SUCCESS;
> >
> > -     ret = tcg2_platform_get_tpm2(&dev);
> > -     if (ret != EFI_SUCCESS)
> > +     if (tcg2_platform_get_tpm2(&dev))
> >               return EFI_SECURITY_VIOLATION;
> >
> >       ret = tcg2_measure_boot_variable(dev);
> > @@ -1406,9 +1415,8 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
> >       if (!is_tcg2_protocol_installed())
> >               return EFI_SUCCESS;
> >
> > -     ret = tcg2_platform_get_tpm2(&dev);
> > -     if (ret != EFI_SUCCESS)
> > -             return ret;
> > +     if (tcg2_platform_get_tpm2(&dev))
> > +             return EFI_SECURITY_VIOLATION;
> >
> >       ret = measure_event(dev, 4, EV_EFI_ACTION,
> >                           strlen(EFI_RETURNING_FROM_EFI_APPLICATION),
> > @@ -1437,9 +1445,10 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
> >               goto out;
> >       }
> >
> > -     ret = tcg2_platform_get_tpm2(&dev);
> > -     if (ret != EFI_SUCCESS)
> > +     if (tcg2_platform_get_tpm2(&dev)) {
> > +             ret = EFI_SECURITY_VIOLATION;
> >               goto out;
> > +     }
> >
> >       ret = measure_event(dev, 5, EV_EFI_ACTION,
> >                           strlen(EFI_EXIT_BOOT_SERVICES_INVOCATION),
> > @@ -1469,9 +1478,8 @@ efi_status_t efi_tcg2_notify_exit_boot_services_failed(void)
> >       if (!is_tcg2_protocol_installed())
> >               return EFI_SUCCESS;
> >
> > -     ret = tcg2_platform_get_tpm2(&dev);
> > -     if (ret != EFI_SUCCESS)
> > -             goto out;
> > +     if (tcg2_platform_get_tpm2(&dev))
> > +             return EFI_SECURITY_VIOLATION;
> >
> >       ret = measure_event(dev, 5, EV_EFI_ACTION,
> >                           strlen(EFI_EXIT_BOOT_SERVICES_INVOCATION),
> > @@ -1551,8 +1559,7 @@ efi_status_t efi_tcg2_do_initial_measurement(void)
> >       if (!is_tcg2_protocol_installed())
> >               return EFI_SUCCESS;
> >
> > -     ret = tcg2_platform_get_tpm2(&dev);
> > -     if (ret != EFI_SUCCESS)
> > +     if (tcg2_platform_get_tpm2(&dev))
> >               return EFI_SECURITY_VIOLATION;
> >
> >       ret = tcg2_measure_secure_boot_variable(dev);
> > @@ -1577,8 +1584,7 @@ efi_status_t efi_tcg2_register(void)
> >       struct efi_event *event;
> >       u32 err;
> >
> > -     ret = tcg2_platform_get_tpm2(&dev);
> > -     if (ret != EFI_SUCCESS) {
> > +     if (tcg2_platform_get_tpm2(&dev)) {
> >               log_warning("Missing TPMv2 device for EFI_TCG_PROTOCOL\n");
> >               return EFI_SUCCESS;
> >       }
> > @@ -1586,6 +1592,7 @@ efi_status_t efi_tcg2_register(void)
> >       /* initialize the TPM as early as possible. */
> >       err = tpm_auto_start(dev);
> >       if (err) {
> > +             ret = EFI_DEVICE_ERROR;
> >               log_err("TPM startup failed\n");
> >               goto fail;
> >       }
>
Heinrich Schuchardt June 22, 2024, 6:01 p.m. UTC | #3
Am 22. Juni 2024 18:09:40 MESZ schrieb Ilias Apalodimas <ilias.apalodimas@linaro.org>:
>Hi Heinrich,
>
>[...]
>
>> >       rc = tpm2_submit_command(dev, input_param_block,
>> >                                output_param_block, &resp_buf_size);
>> > @@ -714,19 +721,20 @@ efi_tcg2_get_active_pcr_banks(struct efi_tcg2_protocol *this,
>> >                             u32 *active_pcr_banks)
>> >   {
>> >       struct udevice *dev;
>> > -     efi_status_t ret;
>> > +     efi_status_t ret = EFI_INVALID_PARAMETER;
>> >
>> >       EFI_ENTRY("%p, %p", this, active_pcr_banks);
>> >
>> > -     if (!this || !active_pcr_banks) {
>> > -             ret = EFI_INVALID_PARAMETER;
>> > +     if (!this || !active_pcr_banks)
>> >               goto out;
>> > -     }
>> > -     ret = tcg2_platform_get_tpm2(&dev);
>> > -     if (ret != EFI_SUCCESS)
>> > +
>> > +     if (tcg2_platform_get_tpm2(&dev))
>>
>> EFI_INVALID_PARAMETER does not convey the problem type.
>> Should we return EFI_DEVICE_ERROR here?
>>
>> The authors of the specification only foresaw one or more of the
>> parameters being incorrect (EFI_INVALID_PARAMETER).
>
>I completely agree that the result is misleading. However, I'd prefer
>sticking to the spec for now and maybe add a comment?
>
>
>>
>> > +             goto out;
>> > +
>> > +     if (tcg2_get_active_pcr_banks(dev, active_pcr_banks))
>>
>> EFI_DEVICE_ERROR?
>
>Same here
>
>Thanks for the qucik review!
>/Ilias


Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>


>>
>> Best regards
>>
>> Heinrich
>>
>> >               goto out;
>> >
>> > -     ret = tcg2_get_active_pcr_banks(dev, active_pcr_banks);
>> > +     ret = EFI_SUCCESS;
>> >
>> >   out:
>> >       return EFI_EXIT(ret);
>> > @@ -852,14 +860,15 @@ static efi_status_t measure_event(struct udevice *dev, u32 pcr_index,
>> >                                 u32 event_type, u32 size, u8 event[])
>> >   {
>> >       struct tpml_digest_values digest_list;
>> > -     efi_status_t ret;
>> > +     efi_status_t ret = EFI_DEVICE_ERROR;
>> > +     int rc;
>> >
>> > -     ret = tcg2_create_digest(dev, event, size, &digest_list);
>> > -     if (ret != EFI_SUCCESS)
>> > +     rc = tcg2_create_digest(dev, event, size, &digest_list);
>> > +     if (rc)
>> >               goto out;
>> >
>> > -     ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
>> > -     if (ret != EFI_SUCCESS)
>> > +     rc = tcg2_pcr_extend(dev, pcr_index, &digest_list);
>> > +     if (rc)
>> >               goto out;
>> >
>> >       ret = tcg2_agile_log_append(pcr_index, event_type, &digest_list,
>> > @@ -901,10 +910,10 @@ static efi_status_t efi_init_event_log(void)
>> >       struct tcg2_event_log elog;
>> >       struct udevice *dev;
>> >       efi_status_t ret;
>> > +     int rc;
>> >
>> > -     ret = tcg2_platform_get_tpm2(&dev);
>> > -     if (ret != EFI_SUCCESS)
>> > -             return ret;
>> > +     if (tcg2_platform_get_tpm2(&dev))
>> > +             return EFI_DEVICE_ERROR;
>> >
>> >       ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, TPM2_EVENT_LOG_SIZE,
>> >                               (void **)&event_log.buffer);
>> > @@ -933,9 +942,11 @@ static efi_status_t efi_init_event_log(void)
>> >        */
>> >       elog.log = event_log.buffer;
>> >       elog.log_size = TPM2_EVENT_LOG_SIZE;
>> > -     ret = tcg2_log_prepare_buffer(dev, &elog, false);
>> > -     if (ret != EFI_SUCCESS)
>> > +     rc = tcg2_log_prepare_buffer(dev, &elog, false);
>> > +     if (rc) {
>> > +             ret = (rc == -ENOBUFS) ? EFI_BUFFER_TOO_SMALL : EFI_DEVICE_ERROR;
>> >               goto free_pool;
>> > +     }
>> >
>> >       event_log.pos = elog.log_position;
>> >
>> > @@ -1306,8 +1317,7 @@ efi_status_t efi_tcg2_measure_dtb(void *dtb)
>> >       if (!is_tcg2_protocol_installed())
>> >               return EFI_SUCCESS;
>> >
>> > -     ret = tcg2_platform_get_tpm2(&dev);
>> > -     if (ret != EFI_SUCCESS)
>> > +     if (tcg2_platform_get_tpm2(&dev))
>> >               return EFI_SECURITY_VIOLATION;
>> >
>> >       rsvmap_size = size_of_rsvmap(dtb);
>> > @@ -1356,8 +1366,7 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha
>> >       if (tcg2_efi_app_invoked)
>> >               return EFI_SUCCESS;
>> >
>> > -     ret = tcg2_platform_get_tpm2(&dev);
>> > -     if (ret != EFI_SUCCESS)
>> > +     if (tcg2_platform_get_tpm2(&dev))
>> >               return EFI_SECURITY_VIOLATION;
>> >
>> >       ret = tcg2_measure_boot_variable(dev);
>> > @@ -1406,9 +1415,8 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
>> >       if (!is_tcg2_protocol_installed())
>> >               return EFI_SUCCESS;
>> >
>> > -     ret = tcg2_platform_get_tpm2(&dev);
>> > -     if (ret != EFI_SUCCESS)
>> > -             return ret;
>> > +     if (tcg2_platform_get_tpm2(&dev))
>> > +             return EFI_SECURITY_VIOLATION;
>> >
>> >       ret = measure_event(dev, 4, EV_EFI_ACTION,
>> >                           strlen(EFI_RETURNING_FROM_EFI_APPLICATION),
>> > @@ -1437,9 +1445,10 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
>> >               goto out;
>> >       }
>> >
>> > -     ret = tcg2_platform_get_tpm2(&dev);
>> > -     if (ret != EFI_SUCCESS)
>> > +     if (tcg2_platform_get_tpm2(&dev)) {
>> > +             ret = EFI_SECURITY_VIOLATION;
>> >               goto out;
>> > +     }
>> >
>> >       ret = measure_event(dev, 5, EV_EFI_ACTION,
>> >                           strlen(EFI_EXIT_BOOT_SERVICES_INVOCATION),
>> > @@ -1469,9 +1478,8 @@ efi_status_t efi_tcg2_notify_exit_boot_services_failed(void)
>> >       if (!is_tcg2_protocol_installed())
>> >               return EFI_SUCCESS;
>> >
>> > -     ret = tcg2_platform_get_tpm2(&dev);
>> > -     if (ret != EFI_SUCCESS)
>> > -             goto out;
>> > +     if (tcg2_platform_get_tpm2(&dev))
>> > +             return EFI_SECURITY_VIOLATION;
>> >
>> >       ret = measure_event(dev, 5, EV_EFI_ACTION,
>> >                           strlen(EFI_EXIT_BOOT_SERVICES_INVOCATION),
>> > @@ -1551,8 +1559,7 @@ efi_status_t efi_tcg2_do_initial_measurement(void)
>> >       if (!is_tcg2_protocol_installed())
>> >               return EFI_SUCCESS;
>> >
>> > -     ret = tcg2_platform_get_tpm2(&dev);
>> > -     if (ret != EFI_SUCCESS)
>> > +     if (tcg2_platform_get_tpm2(&dev))
>> >               return EFI_SECURITY_VIOLATION;
>> >
>> >       ret = tcg2_measure_secure_boot_variable(dev);
>> > @@ -1577,8 +1584,7 @@ efi_status_t efi_tcg2_register(void)
>> >       struct efi_event *event;
>> >       u32 err;
>> >
>> > -     ret = tcg2_platform_get_tpm2(&dev);
>> > -     if (ret != EFI_SUCCESS) {
>> > +     if (tcg2_platform_get_tpm2(&dev)) {
>> >               log_warning("Missing TPMv2 device for EFI_TCG_PROTOCOL\n");
>> >               return EFI_SUCCESS;
>> >       }
>> > @@ -1586,6 +1592,7 @@ efi_status_t efi_tcg2_register(void)
>> >       /* initialize the TPM as early as possible. */
>> >       err = tpm_auto_start(dev);
>> >       if (err) {
>> > +             ret = EFI_DEVICE_ERROR;
>> >               log_err("TPM startup failed\n");
>> >               goto fail;
>> >       }
>>
Ilias Apalodimas June 22, 2024, 6:02 p.m. UTC | #4
On Sat, 22 Jun 2024 at 21:01, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> Am 22. Juni 2024 18:09:40 MESZ schrieb Ilias Apalodimas <ilias.apalodimas@linaro.org>:
> >Hi Heinrich,
> >
> >[...]
> >
> >> >       rc = tpm2_submit_command(dev, input_param_block,
> >> >                                output_param_block, &resp_buf_size);
> >> > @@ -714,19 +721,20 @@ efi_tcg2_get_active_pcr_banks(struct efi_tcg2_protocol *this,
> >> >                             u32 *active_pcr_banks)
> >> >   {
> >> >       struct udevice *dev;
> >> > -     efi_status_t ret;
> >> > +     efi_status_t ret = EFI_INVALID_PARAMETER;
> >> >
> >> >       EFI_ENTRY("%p, %p", this, active_pcr_banks);
> >> >
> >> > -     if (!this || !active_pcr_banks) {
> >> > -             ret = EFI_INVALID_PARAMETER;
> >> > +     if (!this || !active_pcr_banks)
> >> >               goto out;
> >> > -     }
> >> > -     ret = tcg2_platform_get_tpm2(&dev);
> >> > -     if (ret != EFI_SUCCESS)
> >> > +
> >> > +     if (tcg2_platform_get_tpm2(&dev))
> >>
> >> EFI_INVALID_PARAMETER does not convey the problem type.
> >> Should we return EFI_DEVICE_ERROR here?
> >>
> >> The authors of the specification only foresaw one or more of the
> >> parameters being incorrect (EFI_INVALID_PARAMETER).
> >
> >I completely agree that the result is misleading. However, I'd prefer
> >sticking to the spec for now and maybe add a comment?
> >
> >
> >>
> >> > +             goto out;
> >> > +
> >> > +     if (tcg2_get_active_pcr_banks(dev, active_pcr_banks))
> >>
> >> EFI_DEVICE_ERROR?
> >
> >Same here
> >
> >Thanks for the qucik review!
> >/Ilias
>
>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Thanks Heinrich.
FWIW I added a comment with the misleading result

>
>
> >>
> >> Best regards
> >>
> >> Heinrich
> >>
> >> >               goto out;
> >> >
> >> > -     ret = tcg2_get_active_pcr_banks(dev, active_pcr_banks);
> >> > +     ret = EFI_SUCCESS;
> >> >
> >> >   out:
> >> >       return EFI_EXIT(ret);
> >> > @@ -852,14 +860,15 @@ static efi_status_t measure_event(struct udevice *dev, u32 pcr_index,
> >> >                                 u32 event_type, u32 size, u8 event[])
> >> >   {
> >> >       struct tpml_digest_values digest_list;
> >> > -     efi_status_t ret;
> >> > +     efi_status_t ret = EFI_DEVICE_ERROR;
> >> > +     int rc;
> >> >
> >> > -     ret = tcg2_create_digest(dev, event, size, &digest_list);
> >> > -     if (ret != EFI_SUCCESS)
> >> > +     rc = tcg2_create_digest(dev, event, size, &digest_list);
> >> > +     if (rc)
> >> >               goto out;
> >> >
> >> > -     ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
> >> > -     if (ret != EFI_SUCCESS)
> >> > +     rc = tcg2_pcr_extend(dev, pcr_index, &digest_list);
> >> > +     if (rc)
> >> >               goto out;
> >> >
> >> >       ret = tcg2_agile_log_append(pcr_index, event_type, &digest_list,
> >> > @@ -901,10 +910,10 @@ static efi_status_t efi_init_event_log(void)
> >> >       struct tcg2_event_log elog;
> >> >       struct udevice *dev;
> >> >       efi_status_t ret;
> >> > +     int rc;
> >> >
> >> > -     ret = tcg2_platform_get_tpm2(&dev);
> >> > -     if (ret != EFI_SUCCESS)
> >> > -             return ret;
> >> > +     if (tcg2_platform_get_tpm2(&dev))
> >> > +             return EFI_DEVICE_ERROR;
> >> >
> >> >       ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, TPM2_EVENT_LOG_SIZE,
> >> >                               (void **)&event_log.buffer);
> >> > @@ -933,9 +942,11 @@ static efi_status_t efi_init_event_log(void)
> >> >        */
> >> >       elog.log = event_log.buffer;
> >> >       elog.log_size = TPM2_EVENT_LOG_SIZE;
> >> > -     ret = tcg2_log_prepare_buffer(dev, &elog, false);
> >> > -     if (ret != EFI_SUCCESS)
> >> > +     rc = tcg2_log_prepare_buffer(dev, &elog, false);
> >> > +     if (rc) {
> >> > +             ret = (rc == -ENOBUFS) ? EFI_BUFFER_TOO_SMALL : EFI_DEVICE_ERROR;
> >> >               goto free_pool;
> >> > +     }
> >> >
> >> >       event_log.pos = elog.log_position;
> >> >
> >> > @@ -1306,8 +1317,7 @@ efi_status_t efi_tcg2_measure_dtb(void *dtb)
> >> >       if (!is_tcg2_protocol_installed())
> >> >               return EFI_SUCCESS;
> >> >
> >> > -     ret = tcg2_platform_get_tpm2(&dev);
> >> > -     if (ret != EFI_SUCCESS)
> >> > +     if (tcg2_platform_get_tpm2(&dev))
> >> >               return EFI_SECURITY_VIOLATION;
> >> >
> >> >       rsvmap_size = size_of_rsvmap(dtb);
> >> > @@ -1356,8 +1366,7 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha
> >> >       if (tcg2_efi_app_invoked)
> >> >               return EFI_SUCCESS;
> >> >
> >> > -     ret = tcg2_platform_get_tpm2(&dev);
> >> > -     if (ret != EFI_SUCCESS)
> >> > +     if (tcg2_platform_get_tpm2(&dev))
> >> >               return EFI_SECURITY_VIOLATION;
> >> >
> >> >       ret = tcg2_measure_boot_variable(dev);
> >> > @@ -1406,9 +1415,8 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
> >> >       if (!is_tcg2_protocol_installed())
> >> >               return EFI_SUCCESS;
> >> >
> >> > -     ret = tcg2_platform_get_tpm2(&dev);
> >> > -     if (ret != EFI_SUCCESS)
> >> > -             return ret;
> >> > +     if (tcg2_platform_get_tpm2(&dev))
> >> > +             return EFI_SECURITY_VIOLATION;
> >> >
> >> >       ret = measure_event(dev, 4, EV_EFI_ACTION,
> >> >                           strlen(EFI_RETURNING_FROM_EFI_APPLICATION),
> >> > @@ -1437,9 +1445,10 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
> >> >               goto out;
> >> >       }
> >> >
> >> > -     ret = tcg2_platform_get_tpm2(&dev);
> >> > -     if (ret != EFI_SUCCESS)
> >> > +     if (tcg2_platform_get_tpm2(&dev)) {
> >> > +             ret = EFI_SECURITY_VIOLATION;
> >> >               goto out;
> >> > +     }
> >> >
> >> >       ret = measure_event(dev, 5, EV_EFI_ACTION,
> >> >                           strlen(EFI_EXIT_BOOT_SERVICES_INVOCATION),
> >> > @@ -1469,9 +1478,8 @@ efi_status_t efi_tcg2_notify_exit_boot_services_failed(void)
> >> >       if (!is_tcg2_protocol_installed())
> >> >               return EFI_SUCCESS;
> >> >
> >> > -     ret = tcg2_platform_get_tpm2(&dev);
> >> > -     if (ret != EFI_SUCCESS)
> >> > -             goto out;
> >> > +     if (tcg2_platform_get_tpm2(&dev))
> >> > +             return EFI_SECURITY_VIOLATION;
> >> >
> >> >       ret = measure_event(dev, 5, EV_EFI_ACTION,
> >> >                           strlen(EFI_EXIT_BOOT_SERVICES_INVOCATION),
> >> > @@ -1551,8 +1559,7 @@ efi_status_t efi_tcg2_do_initial_measurement(void)
> >> >       if (!is_tcg2_protocol_installed())
> >> >               return EFI_SUCCESS;
> >> >
> >> > -     ret = tcg2_platform_get_tpm2(&dev);
> >> > -     if (ret != EFI_SUCCESS)
> >> > +     if (tcg2_platform_get_tpm2(&dev))
> >> >               return EFI_SECURITY_VIOLATION;
> >> >
> >> >       ret = tcg2_measure_secure_boot_variable(dev);
> >> > @@ -1577,8 +1584,7 @@ efi_status_t efi_tcg2_register(void)
> >> >       struct efi_event *event;
> >> >       u32 err;
> >> >
> >> > -     ret = tcg2_platform_get_tpm2(&dev);
> >> > -     if (ret != EFI_SUCCESS) {
> >> > +     if (tcg2_platform_get_tpm2(&dev)) {
> >> >               log_warning("Missing TPMv2 device for EFI_TCG_PROTOCOL\n");
> >> >               return EFI_SUCCESS;
> >> >       }
> >> > @@ -1586,6 +1592,7 @@ efi_status_t efi_tcg2_register(void)
> >> >       /* initialize the TPM as early as possible. */
> >> >       err = tpm_auto_start(dev);
> >> >       if (err) {
> >> > +             ret = EFI_DEVICE_ERROR;
> >> >               log_err("TPM startup failed\n");
> >> >               goto fail;
> >> >       }
> >>
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index d56bd5657c8a..10c09caac35a 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -257,8 +257,8 @@  efi_tcg2_get_capability(struct efi_tcg2_protocol *this,
 	capability->protocol_version.major = 1;
 	capability->protocol_version.minor = 1;
 
-	efi_ret = tcg2_platform_get_tpm2(&dev);
-	if (efi_ret != EFI_SUCCESS) {
+	ret = tcg2_platform_get_tpm2(&dev);
+	if (ret) {
 		capability->supported_event_logs = 0;
 		capability->hash_algorithm_bitmap = 0;
 		capability->tpm_present_flag = false;
@@ -353,8 +353,7 @@  efi_tcg2_get_eventlog(struct efi_tcg2_protocol *this,
 		goto out;
 	}
 
-	ret = tcg2_platform_get_tpm2(&dev);
-	if (ret != EFI_SUCCESS) {
+	if (tcg2_platform_get_tpm2(&dev)) {
 		event_log_location = NULL;
 		event_log_last_entry = NULL;
 		*event_log_truncated = false;
@@ -389,7 +388,7 @@  static efi_status_t tcg2_hash_pe_image(void *efi, u64 efi_size,
 	void *new_efi = NULL;
 	u8 hash[TPM2_SHA512_DIGEST_SIZE];
 	struct udevice *dev;
-	efi_status_t ret;
+	efi_status_t ret = EFI_SUCCESS;
 	u32 active;
 	int i;
 
@@ -404,12 +403,13 @@  static efi_status_t tcg2_hash_pe_image(void *efi, u64 efi_size,
 		goto out;
 	}
 
-	ret = tcg2_platform_get_tpm2(&dev);
-	if (ret != EFI_SUCCESS)
+	if (tcg2_platform_get_tpm2(&dev)) {
+		ret = EFI_DEVICE_ERROR;
 		goto out;
+	}
 
-	ret = tcg2_get_active_pcr_banks(dev, &active);
-	if (ret != EFI_SUCCESS) {
+	if (tcg2_get_active_pcr_banks(dev, &active)) {
+		ret = EFI_DEVICE_ERROR;
 		goto out;
 	}
 
@@ -473,12 +473,12 @@  efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
 	IMAGE_DOS_HEADER *dos;
 	IMAGE_NT_HEADERS32 *nt;
 	struct efi_handler *handler;
+	int rc;
 
 	if (!is_tcg2_protocol_installed())
 		return EFI_SUCCESS;
 
-	ret = tcg2_platform_get_tpm2(&dev);
-	if (ret != EFI_SUCCESS)
+	if (tcg2_platform_get_tpm2(&dev))
 		return EFI_SECURITY_VIOLATION;
 
 	switch (handle->image_type) {
@@ -502,9 +502,9 @@  efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
 	if (ret != EFI_SUCCESS)
 		return ret;
 
-	ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
-	if (ret != EFI_SUCCESS)
-		return ret;
+	rc = tcg2_pcr_extend(dev, pcr_index, &digest_list);
+	if (rc)
+		return EFI_DEVICE_ERROR;
 
 	ret = efi_search_protocol(&handle->header,
 				  &efi_guid_loaded_image_device_path, &handler);
@@ -574,9 +574,10 @@  efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags,
 			       struct efi_tcg2_event *efi_tcg_event)
 {
 	struct udevice *dev;
-	efi_status_t ret;
+	efi_status_t ret = EFI_SUCCESS;
 	u32 event_type, pcr_index, event_size;
 	struct tpml_digest_values digest_list;
+	int rc = 0;
 
 	EFI_ENTRY("%p, %llu, %llu, %llu, %p", this, flags, data_to_hash,
 		  data_to_hash_len, efi_tcg_event);
@@ -586,9 +587,10 @@  efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags,
 		goto out;
 	}
 
-	ret = tcg2_platform_get_tpm2(&dev);
-	if (ret != EFI_SUCCESS)
+	if (tcg2_platform_get_tpm2(&dev)) {
+		ret = EFI_DEVICE_ERROR;
 		goto out;
+	}
 
 	if (efi_tcg_event->size < efi_tcg_event->header.header_size +
 	    sizeof(u32)) {
@@ -621,8 +623,10 @@  efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags,
 		ret = tcg2_hash_pe_image((void *)(uintptr_t)data_to_hash,
 					 data_to_hash_len, &digest_list);
 	} else {
-		ret = tcg2_create_digest(dev, (u8 *)(uintptr_t)data_to_hash,
-					 data_to_hash_len, &digest_list);
+		rc = tcg2_create_digest(dev, (u8 *)(uintptr_t)data_to_hash,
+					data_to_hash_len, &digest_list);
+		if (rc)
+			ret = EFI_DEVICE_ERROR;
 	}
 
 	if (ret != EFI_SUCCESS)
@@ -631,9 +635,11 @@  efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags,
 	pcr_index = efi_tcg_event->header.pcr_index;
 	event_type = efi_tcg_event->header.event_type;
 
-	ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
-	if (ret != EFI_SUCCESS)
+	rc = tcg2_pcr_extend(dev, pcr_index, &digest_list);
+	if (rc) {
+		ret = EFI_DEVICE_ERROR;
 		goto out;
+	}
 
 	if (flags & EFI_TCG2_EXTEND_ONLY) {
 		if (event_log.truncated)
@@ -672,7 +678,7 @@  efi_tcg2_submit_command(struct efi_tcg2_protocol *this,
 			u8 *output_param_block)
 {
 	struct udevice *dev;
-	efi_status_t ret;
+	efi_status_t ret = EFI_SUCCESS;
 	u32 rc;
 	size_t resp_buf_size = output_param_block_size;
 
@@ -684,9 +690,10 @@  efi_tcg2_submit_command(struct efi_tcg2_protocol *this,
 		goto out;
 	}
 
-	ret = tcg2_platform_get_tpm2(&dev);
-	if (ret != EFI_SUCCESS)
+	if (tcg2_platform_get_tpm2(&dev)) {
+		ret = EFI_DEVICE_ERROR;
 		goto out;
+	}
 
 	rc = tpm2_submit_command(dev, input_param_block,
 				 output_param_block, &resp_buf_size);
@@ -714,19 +721,20 @@  efi_tcg2_get_active_pcr_banks(struct efi_tcg2_protocol *this,
 			      u32 *active_pcr_banks)
 {
 	struct udevice *dev;
-	efi_status_t ret;
+	efi_status_t ret = EFI_INVALID_PARAMETER;
 
 	EFI_ENTRY("%p, %p", this, active_pcr_banks);
 
-	if (!this || !active_pcr_banks) {
-		ret = EFI_INVALID_PARAMETER;
+	if (!this || !active_pcr_banks)
 		goto out;
-	}
-	ret = tcg2_platform_get_tpm2(&dev);
-	if (ret != EFI_SUCCESS)
+
+	if (tcg2_platform_get_tpm2(&dev))
+		goto out;
+
+	if (tcg2_get_active_pcr_banks(dev, active_pcr_banks))
 		goto out;
 
-	ret = tcg2_get_active_pcr_banks(dev, active_pcr_banks);
+	ret = EFI_SUCCESS;
 
 out:
 	return EFI_EXIT(ret);
@@ -852,14 +860,15 @@  static efi_status_t measure_event(struct udevice *dev, u32 pcr_index,
 				  u32 event_type, u32 size, u8 event[])
 {
 	struct tpml_digest_values digest_list;
-	efi_status_t ret;
+	efi_status_t ret = EFI_DEVICE_ERROR;
+	int rc;
 
-	ret = tcg2_create_digest(dev, event, size, &digest_list);
-	if (ret != EFI_SUCCESS)
+	rc = tcg2_create_digest(dev, event, size, &digest_list);
+	if (rc)
 		goto out;
 
-	ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
-	if (ret != EFI_SUCCESS)
+	rc = tcg2_pcr_extend(dev, pcr_index, &digest_list);
+	if (rc)
 		goto out;
 
 	ret = tcg2_agile_log_append(pcr_index, event_type, &digest_list,
@@ -901,10 +910,10 @@  static efi_status_t efi_init_event_log(void)
 	struct tcg2_event_log elog;
 	struct udevice *dev;
 	efi_status_t ret;
+	int rc;
 
-	ret = tcg2_platform_get_tpm2(&dev);
-	if (ret != EFI_SUCCESS)
-		return ret;
+	if (tcg2_platform_get_tpm2(&dev))
+		return EFI_DEVICE_ERROR;
 
 	ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, TPM2_EVENT_LOG_SIZE,
 				(void **)&event_log.buffer);
@@ -933,9 +942,11 @@  static efi_status_t efi_init_event_log(void)
 	 */
 	elog.log = event_log.buffer;
 	elog.log_size = TPM2_EVENT_LOG_SIZE;
-	ret = tcg2_log_prepare_buffer(dev, &elog, false);
-	if (ret != EFI_SUCCESS)
+	rc = tcg2_log_prepare_buffer(dev, &elog, false);
+	if (rc) {
+		ret = (rc == -ENOBUFS) ? EFI_BUFFER_TOO_SMALL : EFI_DEVICE_ERROR;
 		goto free_pool;
+	}
 
 	event_log.pos = elog.log_position;
 
@@ -1306,8 +1317,7 @@  efi_status_t efi_tcg2_measure_dtb(void *dtb)
 	if (!is_tcg2_protocol_installed())
 		return EFI_SUCCESS;
 
-	ret = tcg2_platform_get_tpm2(&dev);
-	if (ret != EFI_SUCCESS)
+	if (tcg2_platform_get_tpm2(&dev))
 		return EFI_SECURITY_VIOLATION;
 
 	rsvmap_size = size_of_rsvmap(dtb);
@@ -1356,8 +1366,7 @@  efi_status_t efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha
 	if (tcg2_efi_app_invoked)
 		return EFI_SUCCESS;
 
-	ret = tcg2_platform_get_tpm2(&dev);
-	if (ret != EFI_SUCCESS)
+	if (tcg2_platform_get_tpm2(&dev))
 		return EFI_SECURITY_VIOLATION;
 
 	ret = tcg2_measure_boot_variable(dev);
@@ -1406,9 +1415,8 @@  efi_status_t efi_tcg2_measure_efi_app_exit(void)
 	if (!is_tcg2_protocol_installed())
 		return EFI_SUCCESS;
 
-	ret = tcg2_platform_get_tpm2(&dev);
-	if (ret != EFI_SUCCESS)
-		return ret;
+	if (tcg2_platform_get_tpm2(&dev))
+		return EFI_SECURITY_VIOLATION;
 
 	ret = measure_event(dev, 4, EV_EFI_ACTION,
 			    strlen(EFI_RETURNING_FROM_EFI_APPLICATION),
@@ -1437,9 +1445,10 @@  efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
 		goto out;
 	}
 
-	ret = tcg2_platform_get_tpm2(&dev);
-	if (ret != EFI_SUCCESS)
+	if (tcg2_platform_get_tpm2(&dev)) {
+		ret = EFI_SECURITY_VIOLATION;
 		goto out;
+	}
 
 	ret = measure_event(dev, 5, EV_EFI_ACTION,
 			    strlen(EFI_EXIT_BOOT_SERVICES_INVOCATION),
@@ -1469,9 +1478,8 @@  efi_status_t efi_tcg2_notify_exit_boot_services_failed(void)
 	if (!is_tcg2_protocol_installed())
 		return EFI_SUCCESS;
 
-	ret = tcg2_platform_get_tpm2(&dev);
-	if (ret != EFI_SUCCESS)
-		goto out;
+	if (tcg2_platform_get_tpm2(&dev))
+		return EFI_SECURITY_VIOLATION;
 
 	ret = measure_event(dev, 5, EV_EFI_ACTION,
 			    strlen(EFI_EXIT_BOOT_SERVICES_INVOCATION),
@@ -1551,8 +1559,7 @@  efi_status_t efi_tcg2_do_initial_measurement(void)
 	if (!is_tcg2_protocol_installed())
 		return EFI_SUCCESS;
 
-	ret = tcg2_platform_get_tpm2(&dev);
-	if (ret != EFI_SUCCESS)
+	if (tcg2_platform_get_tpm2(&dev))
 		return EFI_SECURITY_VIOLATION;
 
 	ret = tcg2_measure_secure_boot_variable(dev);
@@ -1577,8 +1584,7 @@  efi_status_t efi_tcg2_register(void)
 	struct efi_event *event;
 	u32 err;
 
-	ret = tcg2_platform_get_tpm2(&dev);
-	if (ret != EFI_SUCCESS) {
+	if (tcg2_platform_get_tpm2(&dev)) {
 		log_warning("Missing TPMv2 device for EFI_TCG_PROTOCOL\n");
 		return EFI_SUCCESS;
 	}
@@ -1586,6 +1592,7 @@  efi_status_t efi_tcg2_register(void)
 	/* initialize the TPM as early as possible. */
 	err = tpm_auto_start(dev);
 	if (err) {
+		ret = EFI_DEVICE_ERROR;
 		log_err("TPM startup failed\n");
 		goto fail;
 	}