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

[Ilias Apalodimas]

Hi,

> +	ret = tcg2_get_fw_eventlog(dev, event_log.buffer, &event_log.pos);
> +	/*
> +	 * If earlier firmware hasn't passed any eventlog, go ahead and
> +	 * create the eventlog header
> +	 */
> +	if (ret == EFI_NOT_FOUND) {
> +		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;
> +	}
> +
>  	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)
> -- 
> 2.25.1
> 

I think Kojima-san is right.  I don't think it's a good idea to turn our
startup code into a 'PC client spec' parser and try to reason about
previous events. The Spec is pretty clear on "3.2.3.1 Firmware Boot Block
SRTM".  Specifically it says:

"In this architecture, the Platform Firmware is composed of a Boot Block 
(SEC/PEI/IBB) and a UEFI firmware. Each of these is an independent component
and each can be updated independent of the other. In this architecture, the
Boot Block is the SRTM while the UEFI Firmware is not but is a measured 
component of chain of trust".

So in our case the (SEC/PEI/IBB) is basically TF-A.  So TF-A should compose
the S_CRTM_VERSION event.  If it doesn't we need to go back to TF-A and
fix it.  I think unconditionally removing it from u-boot makes more sense,
rather that parse events and try to figure out if the previous loader has a
bug....


Regards
/Ilias