[PATCH 1/3] efi_loader: Add check for event log passed from firmware

Ilias Apalodimas ilias.apalodimas at linaro.org
Mon Nov 22 12:32:44 CET 2021


Hi Ruchika, 
>  

[...]

> +static efi_status_t efi_init_event_log(struct udevice *dev)
>  {
>  	/*
>  	 * vendor_info_size is currently set to 0, we need to change the length
>  	 * and allocate the flexible array member if this changes
>  	 */
>  	struct tcg_pcr_event *event_header = NULL;
> -	struct udevice *dev;
> +	struct tpml_digest_values digest_list;
>  	size_t spec_event_size;
>  	efi_status_t ret;
> +	u32 pcr, pos;
> +	u64 base;
> +	u32 sz;
>  
>  	ret = platform_get_tpm2_device(&dev);
>  	if (ret != EFI_SUCCESS)
> @@ -1266,26 +1507,65 @@ static efi_status_t efi_init_event_log(void)
>  	 * last log entry
>  	 */
>  	memset(event_log.buffer, 0xff, TPM2_EVENT_LOG_SIZE);
> +
> +	/*
> +	 * The log header is defined to be in SHA1 event log entry format.
> +	 * Setup event header
> +	 */
> +	event_header =  (struct tcg_pcr_event *)event_log.buffer;
>  	event_log.pos = 0;
>  	event_log.last_event_size = 0;
>  	event_log.get_event_called = false;
>  	event_log.truncated = false;
>  
>  	/*
> -	 * The log header is defined to be in SHA1 event log entry format.
> -	 * Setup event header
> +	 * Check if earlier firmware have passed any eventlog. Different
> +	 * platforms can use different ways to do so
>  	 */
> -	event_header =  (struct tcg_pcr_event *)event_log.buffer;
> -	put_unaligned_le32(0, &event_header->pcr_index);
> -	put_unaligned_le32(EV_NO_ACTION, &event_header->event_type);
> -	memset(&event_header->digest, 0, sizeof(event_header->digest));
> -	ret = create_specid_event(dev, (void *)((uintptr_t)event_log.buffer + sizeof(*event_header)),
> -				  &spec_event_size);
> -	if (ret != EFI_SUCCESS)
> -		goto free_pool;
> -	put_unaligned_le32(spec_event_size, &event_header->event_size);
> -	event_log.pos = spec_event_size + sizeof(*event_header);
> -	event_log.last_event_size = event_log.pos;
> +	ret = platform_get_eventlog(dev, &base, &sz);

I think we can refactor this slightly and make it easier to read. 
Can we merge the logic of getting + validating the eventlog in 
platform_get_eventlog(). Then just return EFI_XXXXX and continue from
there.

> +	if (ret == EFI_SUCCESS && sz < TPM2_EVENT_LOG_SIZE) {
> +		void *buffer = (void *)base;
> +
> +		pos = 0;
> +		/* Parse the eventlog to check for its validity */
> +		ret = parse_event_log_header(buffer, sz, &pos);
> +		if (ret || pos > sz) {
> +			ret = EFI_COMPROMISED_DATA;
> +			goto free_pool;
> +		}
> +
> +		ret = parse_specid_event(dev, buffer, sz, &pos, &digest_list);
> +		if (ret || pos > sz) {
> +			log_err("Error parsing SPEC ID Event\n");
> +			ret = EFI_COMPROMISED_DATA;
> +			goto free_pool;
> +		}
> +
> +		while (pos < sz) {
> +			ret = tcg2_parse_event(dev, buffer, sz, &pos,
> +					       &digest_list, &pcr);
> +			if (ret) {
> +				log_err("Error parsing event\n");
> +				goto free_pool;
> +			}
> +		}
> +
> +		memcpy(event_log.buffer, buffer, sz);
> +		event_log.pos = sz;
> +	} else {
> +		put_unaligned_le32(0, &event_header->pcr_index);
> +		put_unaligned_le32(EV_NO_ACTION, &event_header->event_type);
> +		memset(&event_header->digest, 0, sizeof(event_header->digest));
> +		ret = create_specid_event(dev,
> +					  (void *)((uintptr_t)event_log.buffer +
> +						   sizeof(*event_header)),
> +					  &spec_event_size);
> +		if (ret != EFI_SUCCESS)
> +			goto free_pool;
> +		put_unaligned_le32(spec_event_size, &event_header->event_size);
> +		event_log.pos = spec_event_size + sizeof(*event_header);
> +		event_log.last_event_size = event_log.pos;
> +	}
>  
>  	ret = create_final_event();
>  	if (ret != EFI_SUCCESS)
> @@ -1664,7 +1944,7 @@ efi_status_t efi_tcg2_register(void)
>  		return EFI_SUCCESS;
>  	}
>  
> -	ret = efi_init_event_log();
> +	ret = efi_init_event_log(dev);
>  	if (ret != EFI_SUCCESS)
>  		goto fail;
>  
> -- 
> 2.25.1
> 


Thanks
/Ilias


More information about the U-Boot mailing list