[PATCH] efi_loader: move efi_var_collect to common functions

Heinrich Schuchardt xypron.glpk at gmx.de
Fri Apr 5 08:27:55 CEST 2024


On 4/4/24 09:48, Ilias Apalodimas wrote:
> From: Ilias Apalodimas <apalos at gmail.com>
>
> efi_var_collect() was initially placed in efi_var_file.c, since back
> then we only supported efi variables stored in a file. Since then we
> support variables stored in an RPMB as well and use that function to
> collect variables that should be present at runtime.

Thanks for cleaning this up.

In the CONFIG_EFI_MM_COMM_TEE=y case we seem not to need efi_var_file.c
anymore. Can we add this diff:

--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -72,11 +72,11 @@ obj-y += efi_string.o
  obj-$(CONFIG_EFI_UNICODE_COLLATION_PROTOCOL2) += efi_unicode_collation.o
  obj-y += efi_var_common.o
  obj-y += efi_var_mem.o
-obj-y += efi_var_file.o
  ifeq ($(CONFIG_EFI_MM_COMM_TEE),y)
  obj-y += efi_variable_tee.o
  else
  obj-y += efi_variable.o
+obj-y += efi_var_file.o
  obj-$(CONFIG_EFI_VARIABLES_PRESEED) += efi_var_seed.o
  endif
  obj-y += efi_watchdog.o

Another comment below.

>
> So let's move it around in efi_var_common.c which makes more sense
>
> Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> Signed-off-by: Ilias Apalodimas <apalos at gmail.com>
> ---
>   lib/efi_loader/efi_var_common.c | 76 +++++++++++++++++++++++++++++++++
>   lib/efi_loader/efi_var_file.c   | 64 ---------------------------
>   2 files changed, 76 insertions(+), 64 deletions(-)
>
> diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
> index 16b2c3d48828..07b9603d49f3 100644
> --- a/lib/efi_loader/efi_var_common.c
> +++ b/lib/efi_loader/efi_var_common.c
> @@ -9,6 +9,7 @@
>   #include <efi_loader.h>
>   #include <efi_variable.h>
>   #include <stdlib.h>
> +#include <u-boot/crc.h>
>
>   enum efi_secure_mode {
>   	EFI_MODE_SETUP,
> @@ -416,3 +417,78 @@ void *efi_get_var(const u16 *name, const efi_guid_t *vendor, efi_uintn_t *size)
>
>   	return buf;
>   }
> +
> +/**
> + * efi_var_collect() - Copy EFI variables mstching attributes mask
> + *
> + * @bufp:	buffer containing variable collection
> + * @lenp:	buffer length
> + * @attr_mask:	mask of matched attributes
> + *
> + * Return:	Status code
> + */
> +efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *lenp,
> +					    u32 check_attr_mask)
> +{
> +	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,
> +					   &var->time);
> +		if (ret != EFI_SUCCESS) {
> +			free(buf);
> +			return ret;
> +		}
> +		if ((var->attr & check_attr_mask) == check_attr_mask) {
> +			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;
> +}
> +
> +

This produces a warning

.git/rebase-apply/patch:95: new blank line at EOF.

Could you, please, remove the blank lines at the end of the file.

Best regards

Heinrich

> diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> index 532b6b40eefe..413e1794e88c 100644
> --- a/lib/efi_loader/efi_var_file.c
> +++ b/lib/efi_loader/efi_var_file.c
> @@ -52,70 +52,6 @@ static efi_status_t __maybe_unused efi_set_blk_dev_to_system_partition(void)
>   	return EFI_SUCCESS;
>   }
>
> -efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *lenp,
> -					    u32 check_attr_mask)
> -{
> -	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,
> -					   &var->time);
> -		if (ret != EFI_SUCCESS) {
> -			free(buf);
> -			return ret;
> -		}
> -		if ((var->attr & check_attr_mask) == check_attr_mask) {
> -			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
>    *



More information about the U-Boot mailing list