[PATCH v3 15/17] efi_loader: use memory based variable storage

[ilias.apalodimas at linaro.org]

Hi Heinrich

On Wed, Jul 08, 2020 at 06:29:34PM +0200, Heinrich Schuchardt wrote:
> Saving UEFI variable as encoded U-Boot environment variables does not allow
> implement run-time support.
> 
> Use a memory buffer for storing UEFI variables.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>  lib/efi_loader/efi_variable.c | 556 ++++++----------------------------
>  1 file changed, 93 insertions(+), 463 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index f2f787fc8d..13123e7e41 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -30,145 +30,6 @@ static bool efi_secure_boot;
>  static enum efi_secure_mode efi_secure_mode;
> -}

[...]

> -
>  /**
>  }
>  #endif /* CONFIG_EFI_SECURE_BOOT */
> 
> -efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor,
> -				  u32 *attributes, efi_uintn_t *data_size,
> -				  void *data, u64 *timep)
> +efi_status_t __efi_runtime
> +efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor,
> +		     u32 *attributes, efi_uintn_t *data_size, void *data,
> +		     u64 *timep)
>  {
> -	char *native_name;
> -	efi_status_t ret;
> -	unsigned long in_size;
> -	const char *val = NULL, *s;
> -	u64 time = 0;
> -	u32 attr;
> +	efi_uintn_t old_size;
> +	struct efi_var_entry *var;
> +	u16 *pdata;
> 
>  	if (!variable_name || !vendor || !data_size)
>  		return EFI_INVALID_PARAMETER;
> -
> -	ret = efi_to_native(&native_name, variable_name, vendor);
> -	if (ret)
> -		return ret;
> -
> -	EFI_PRINT("get '%s'\n", native_name);
> -
> -	val = env_get(native_name);
> -	free(native_name);
> -	if (!val)
> +	var = efi_var_mem_find(vendor, variable_name, NULL);
> +	if (!var)
>  		return EFI_NOT_FOUND;
> 
> -	val = parse_attr(val, &attr, &time);
> -
> -	if (timep)
> -		*timep = time;
> -
> -	in_size = *data_size;
> -
> -	if ((s = prefix(val, "(blob)"))) {
> -		size_t len = strlen(s);
> -
> -		/* number of hexadecimal digits must be even */
> -		if (len & 1)
> -			return EFI_DEVICE_ERROR;
> -
> -		/* two characters per byte: */
> -		len /= 2;
> -		*data_size = len;
> -
> -		if (in_size < len) {
> -			ret = EFI_BUFFER_TOO_SMALL;
> -			goto out;
> -		}
> -
> -		if (!data) {
> -			EFI_PRINT("Variable with no data shouldn't exist.\n");
> -			return EFI_INVALID_PARAMETER;
> -		}
> -
> -		if (hex2bin(data, s, len))
> -			return EFI_DEVICE_ERROR;
> -
> -		EFI_PRINT("got value: \"%s\"\n", s);
> -	} else if ((s = prefix(val, "(utf8)"))) {
> -		unsigned len = strlen(s) + 1;
> -
> -		*data_size = len;
> -
> -		if (in_size < len) {
> -			ret = EFI_BUFFER_TOO_SMALL;
> -			goto out;
> -		}
> -
> -		if (!data) {
> -			EFI_PRINT("Variable with no data shouldn't exist.\n");
> -			return EFI_INVALID_PARAMETER;
> -		}
> -
> -		memcpy(data, s, len);
> -		((char *)data)[len] = '\0';
> -
> -		EFI_PRINT("got value: \"%s\"\n", (char *)data);
> -	} else {
> -		EFI_PRINT("invalid value: '%s'\n", val);
> -		return EFI_DEVICE_ERROR;
> -	}
> -
> -out:
>  	if (attributes)
> -		*attributes = attr;
> -
> -	return ret;
> -}
> -
> -static char *efi_variables_list;
> -static char *efi_cur_variable;
> -
> -/**
> - * parse_uboot_variable() - parse a u-boot variable and get uefi-related
> - *			    information
> - * @variable:		whole data of u-boot variable (ie. name=value)
> - * @variable_name_size: size of variable_name buffer in byte
> - * @variable_name:	name of uefi variable in u16, null-terminated
> - * @vendor:		vendor's guid
> - * @attributes:		attributes
> - *
> - * A uefi variable is encoded into a u-boot variable as described above.
> - * This function parses such a u-boot variable and retrieve uefi-related
> - * information into respective parameters. In return, variable_name_size
> - * is the size of variable name including NULL.
> - *
> - * Return:		EFI_SUCCESS if parsing is OK, EFI_NOT_FOUND when
> - *			the entire variable list has been returned,
> - *			otherwise non-zero status code
> - */

[...]

>  }
> 
> -efi_status_t efi_get_next_variable_name_int(efi_uintn_t *variable_name_size,
> -					    u16 *variable_name,
> -					    efi_guid_t *vendor)
> +efi_status_t __efi_runtime
> +efi_get_next_variable_name_int(efi_uintn_t *variable_name_size,
> +			       u16 *variable_name, efi_guid_t *vendor)
>  {
> -	char *native_name, *variable;
> -	ssize_t name_len, list_len;
> -	char regex[256];
> -	char * const regexlist[] = {regex};
> -	u32 attributes;
> -	int i;
> -	efi_status_t ret;
> +	struct efi_var_entry *var;
> +	efi_uintn_t old_size;
> +	u16 *pdata;
> 
>  	if (!variable_name_size || !variable_name || !vendor)
>  		return EFI_INVALID_PARAMETER;
> 
> -	if (variable_name[0]) {
> -		/* check null-terminated string */
> -		for (i = 0; i < *variable_name_size; i++)
> -			if (!variable_name[i])
> -				break;
> -		if (i >= *variable_name_size)
> -			return EFI_INVALID_PARAMETER;
> -
> -		/* search for the last-returned variable */
> -		ret = efi_to_native(&native_name, variable_name, vendor);
> -		if (ret)
> -			return ret;
> +	efi_var_mem_find(vendor, variable_name, &var);
> 
> -		name_len = strlen(native_name);
> -		for (variable = efi_variables_list; variable && *variable;) {
> -			if (!strncmp(variable, native_name, name_len) &&
> -			    variable[name_len] == '=')
> -				break;
> +	if (!var)
> +		return EFI_NOT_FOUND;
> 
> -			variable = strchr(variable, '\n');
> -			if (variable)
> -				variable++;
> -		}
> +	for (pdata = var->name; *pdata; ++pdata)
> +		;
> +	++pdata;
> 
> -		free(native_name);
> -		if (!(variable && *variable))
> -			return EFI_INVALID_PARAMETER;
> +	old_size = *variable_name_size;
> +	*variable_name_size = (uintptr_t)pdata - (uintptr_t)var->name;
> 
> -		/* next variable */
> -		variable = strchr(variable, '\n');
> -		if (variable)
> -			variable++;
> -		if (!(variable && *variable))
> -			return EFI_NOT_FOUND;
> -	} else {
> -		/*
> -		 *new search: free a list used in the previous search
> -		 */
> -		free(efi_variables_list);
> -		efi_variables_list = NULL;
> -		efi_cur_variable = NULL;
> -
> -		snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");
> -		list_len = hexport_r(&env_htab, '\n',
> -				     H_MATCH_REGEX | H_MATCH_KEY,
> -				     &efi_variables_list, 0, 1, regexlist);
> -
> -		if (list_len <= 1)
> -			return EFI_NOT_FOUND;
> -
> -		variable = efi_variables_list;
> -	}
> +	if (old_size < *variable_name_size)
> +		return EFI_BUFFER_TOO_SMALL;
> 
> -	ret = parse_uboot_variable(variable, variable_name_size, variable_name,
> -				   vendor, &attributes);
> +	efi_memcpy_runtime(variable_name, var->name, *variable_name_size);
> +	efi_memcpy_runtime(vendor, &var->guid, sizeof(efi_guid_t));
> 
> -	return ret;
> +	return EFI_SUCCESS;
>  }
> 

In order to provide memory backed runtime variable support on the tee-backed
variables, I'll have to use identical functions for efi_get_variable_int() and 
efi_get_next_variable_name_int(). 
I think it would make more sense to move all the variants that involve talking
to the memory based backend to efi_var_common.c. That way you can call the
common function in your non-runtime code (as well), while I can call that after
EBS, on all my *_runtime variants.

Cheers
/Ilias