Re: [PATCH 3/5] efi_loader: add boot variable measurement

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Kojima-san,
> +{

[...]

> +	u16 *boot_order;
> +	u16 var_name[] = L"BootOrder";
> +	u16 boot_name[] = L"Boot0000";
> +	u16 hexmap[] = L"0123456789ABCDEF";
> +	u8 *bootvar;
> +	efi_uintn_t var_data_size;
> +	u32 count, i;
> +	efi_status_t ret;
> +
> +	boot_order = efi_get_var(var_name, &efi_global_variable_guid,
> +				 &var_data_size);
> +	if (!boot_order) {
> +		log_info("BootOrder not defined\n");
> +		ret = EFI_NOT_FOUND;
> +		goto error;
> +	}
> +
> +	ret = tcg2_measure_variable(dev, 1, EV_EFI_VARIABLE_BOOT2, var_name,
> +				    &efi_global_variable_guid, var_data_size,
> +				    (u8 *)boot_order);
> +	if (ret != EFI_SUCCESS)
> +		goto error;
> +
> +	count = var_data_size / sizeof(*boot_order);
> +	for (i = 0; i < count; i++) {
> +		boot_name[4] = hexmap[(boot_order[i] & 0xf000) >> 12];
> +		boot_name[5] = hexmap[(boot_order[i] & 0x0f00) >> 8];
> +		boot_name[6] = hexmap[(boot_order[i] & 0x00f0) >> 4];
> +		boot_name[7] = hexmap[(boot_order[i] & 0x000f)];

Can you use efi_create_indexed_name() instead?

[...]
> +	for (pcr_index = 0; pcr_index <= 7; pcr_index++) {
> +		ret = tcg2_measure_event(dev, pcr_index, EV_SEPARATOR,
> +					 sizeof(event), (u8 *)&event);

I assume adding a separator event on all these PCRs is described on the
standard?

> +		if (ret != EFI_SUCCESS)
> +			goto out;
> +	}
> +
> +	tcg2_efi_app_invoked = true;
> +out:
> +	return ret;
> +}
> +
> +/**
> + * efi_tcg2_measure_efi_app_exit() - measure efi app exit
> + *
> + * Return:	status code
> + */
> +efi_status_t EFIAPI efi_tcg2_measure_efi_app_exit(void)
> +{
> +	efi_status_t ret;
> +	struct udevice *dev;
> +
> +	ret = platform_get_tpm2_device(&dev);
> +	if (ret != EFI_SUCCESS)
> +		return ret;
> +
> +	ret = tcg2_measure_event(dev, 4, EV_EFI_ACTION,
> +				 strlen(EFI_RETURNING_FROM_EFI_APPLICATION),

Do we need a NUL terminator on this string or not?


Regards
/Ilias