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

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