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

Heinrich Schuchardt xypron.glpk at gmx.de
Sat Jun 22 17:55:17 CEST 2024


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 at 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;
>   	}



More information about the U-Boot mailing list