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

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