[PATCH 10/16] efi_loader: UEFI variable persistence

[Heinrich Schuchardt]

On 3/27/20 9:07 AM, Punit Agrawal wrote:
> Heinrich Schuchardt <xypron.glpk at gmx.de> writes:
>
>> Persist non-volatile UEFI variables in a file on the EFI system partition.
>>
>> The file is written:
>>
>> * whenever a non-volatile UEFI variable is changed after initialization
>>    of the UEFI sub-system.
>> * upon ExitBootServices()
>
> I might be missing something but how does this cope with the ESP being
> on a storage medium access to which is owned by the OS at runtime? e.g.,
> partition on eMMC or SATA drive.

This development does not guard against manipulation by the OS.

Ilias is cureently working on a solution for ATF based devices that will
provide secure storage for variables.

>
>>
>> The file is read during the UEFI sub-system initialization to restore
>> non-volatile UEFI variables.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>>   include/efi_variable.h              |  36 +++++
>>   lib/efi_loader/Kconfig              |   8 +
>>   lib/efi_loader/Makefile             |   1 +
>>   lib/efi_loader/efi_variable.c       |  12 +-
>>   lib/efi_loader/efi_variables_file.c | 235 ++++++++++++++++++++++++++++
>>   5 files changed, 291 insertions(+), 1 deletion(-)
>>   create mode 100644 include/efi_variable.h
>>   create mode 100644 lib/efi_loader/efi_variables_file.c
>>
>
> [...]
>
>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>> index e10ca05549..41705fc252 100644
>> --- a/lib/efi_loader/Kconfig
>> +++ b/lib/efi_loader/Kconfig
>> @@ -27,6 +27,14 @@ config EFI_LOADER
>>
>>   if EFI_LOADER
>>
>> +config EFI_VARIABLE_FILE_STORE
>> +	bool "Store non-volatile UEFI variables as file"
>> +	depends on FAT_WRITE
>> +	default y
>> +	help
>> +	  Select tis option if you want non-volatile UEFI variables to be stored
>
>                   this
>
>> +	  as file /ubootefi.var on the EFI system partition.
>> +
>>   config EFI_GET_TIME
>>   	bool "GetTime() runtime service"
>>   	depends on DM_RTC
>
> [...]
>
>> diff --git a/lib/efi_loader/efi_variables_file.c b/lib/efi_loader/efi_variables_file.c
>> new file mode 100644
>> index 0000000000..4a918d3fde
>> --- /dev/null
>> +++ b/lib/efi_loader/efi_variables_file.c
>> @@ -0,0 +1,235 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * File interface for UEFI variables
>> + *
>> + * Copyright (c) 2020, Heinrich Schuchardt
>> + */
>> +
>> +#include <common.h>
>> +#include <charset.h>
>> +#include <fs.h>
>> +#include <malloc.h>
>> +#include <mapmem.h>
>> +#include <efi_loader.h>
>> +#include <efi_variable.h>
>> +#include <u-boot/crc.h>
>> +
>> +#define PART_STR_LEN 10
>> +
>> +/**
>> + * efi_set_blk_dev_to_system_partition() - select EFI system partition
>> + *
>> + * Set the EFI system partition as current block device.
>> + *
>> + * Return:	status code
>> + */
>> +static efi_status_t __maybe_unused efi_set_blk_dev_to_system_partition(void)
>> +{
>> +	char part_str[PART_STR_LEN];
>> +	int r;
>> +
>> +	if (!efi_system_partition.if_type)
>> +		return EFI_NOT_FOUND;
>> +	snprintf(part_str, PART_STR_LEN, "%u:%u",
>> +		 efi_system_partition.devnum, efi_system_partition.part);
>> +	r = fs_set_blk_dev(blk_get_if_type_name(efi_system_partition.if_type),
>> +			   part_str, FS_TYPE_ANY);
>> +	if (r) {
>> +		printf("Cannot read EFI system partition\n");
>> +		return EFI_DEVICE_ERROR;
>> +	}
>> +	return EFI_SUCCESS;
>> +}
>> +
>> +/**
>> + * efi_var_collect() - collect non-volatile variables in buffer
>> + *
>> + * A buffer is allocated and filled with all non-volatile variables in a
>> + * format ready to be written to disk.
>> + *
>> + * @bufp:	pointer to pointer of buffer with collected variables
>> + * @lenp:	pointer to length of buffer
>> + * Return:	status code
>> + */
>> +static efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp,
>> +						   loff_t *lenp)
>> +{
>> +	size_t len = EFI_VAR_BUF_SIZE;
>> +	struct efi_var_file *buf;
>> +	struct efi_var_entry *var, *old_var;
>> +	size_t old_var_name_length = 2;
>> +
>> +	*bufp = NULL; /* Avoid double free() */
>> +	buf = calloc(1, len);
>> +	if (!buf)
>> +		return EFI_OUT_OF_RESOURCES;
>> +	var = buf->var;
>> +	old_var = var;
>> +	for (;;) {
>> +		efi_uintn_t data_length, var_name_length;
>> +		u8 *data;
>> +		efi_status_t ret;
>> +
>> +		if ((uintptr_t)buf + len <=
>> +		    (uintptr_t)var->name + old_var_name_length)
>> +			return EFI_BUFFER_TOO_SMALL;
>> +
>> +		var_name_length = (uintptr_t)buf + len - (uintptr_t)var->name;
>> +		memcpy(var->name, old_var->name, old_var_name_length);
>> +		guidcpy(&var->guid, &old_var->guid);
>> +		ret = efi_get_next_variable_name_int(
>> +				&var_name_length, var->name, &var->guid);
>> +		if (ret == EFI_NOT_FOUND)
>> +			break;
>> +		if (ret != EFI_SUCCESS) {
>> +			free(buf);
>> +			return ret;
>> +		}
>> +		old_var_name_length = var_name_length;
>> +		old_var = var;
>> +
>> +		data = (u8 *)var->name + old_var_name_length;
>> +		data_length = (uintptr_t)buf + len - (uintptr_t)data;
>> +		ret = efi_get_variable_int(var->name, &var->guid,
>> +					   &var->attr, &data_length, data);
>> +		if (ret != EFI_SUCCESS) {
>> +			free(buf);
>> +			return ret;
>> +		}
>> +		if (!(var->attr & EFI_VARIABLE_NON_VOLATILE))
>> +			continue;
>> +		var->length = data_length;
>> +		var = (struct efi_var_entry *)
>> +		      ALIGN((uintptr_t)data + data_length, 8);
>> +	}
>> +
>> +	buf->reserved = 0;
>> +	buf->magic = EFI_VAR_FILE_MAGIC;
>> +	len = (uintptr_t)var - (uintptr_t)buf;
>> +	buf->crc32 = crc32(0, (u8 *)buf->var,
>> +			   len - sizeof(struct efi_var_file));
>> +	buf->length = len;
>> +	*bufp = buf;
>> +	*lenp = len;
>> +
>> +	return EFI_SUCCESS;
>> +}
>> +
>> +/**
>> + * efi_var_to_file() - save non-volatile variables as file
>> + *
>> + * File ubootefi.var is created on the EFI system partion.
>> + *
>> + * Return:	status code
>> + */
>
> The return value doesn't seem to be used in this patch. Is it really
> needed?

It can be remove.

Thanks for reviewing.

Best regards

Heinrich

>
> Thanks,
> Punit
>
>> +efi_status_t efi_var_to_file(void)
>> +{
>> +#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
>> +	efi_status_t ret;
>> +	struct efi_var_file *buf;
>> +	loff_t len;
>> +	loff_t actlen;
>> +	int r;
>> +
>> +	ret = efi_var_collect(&buf, &len);
>> +	if (ret != EFI_SUCCESS)
>> +		goto error;
>> +
>> +	ret = efi_set_blk_dev_to_system_partition();
>> +	if (ret != EFI_SUCCESS)
>> +		goto error;
>> +
>> +	r = fs_write(EFI_VAR_FILE_NAME, map_to_sysmem(buf), 0, len, &actlen);
>> +	if (r || len != actlen)
>> +		ret =  EFI_DEVICE_ERROR;
>> +
>> +error:
>> +	if (ret != EFI_SUCCESS)
>> +		printf("Failed to persist EFI variables\n");
>> +	free(buf);
>> +	return ret;
>> +#else
>> +	return EFI_SUCCESS;
>> +#endif
>> +}
>> +
>
> [...]
>