[PATCH v2 1/3] efi_var_file: refactor to move buffer functions
Heinrich Schuchardt
xypron.glpk at gmx.de
Wed Nov 29 18:41:43 CET 2023
On 24.11.23 12:35, Shantur Rathore wrote:
> Currently efi_var_file.c has functions to store/read
> EFI variables to/from memory buffer. These functions
> can be used with other EFI variable stores so move
> them out to efi_var_common.c
The moved functions relate to filling the memory buffer of variables.
Wouldn't lib/efi_loader/efi_var_mem.c be more appropriate as target file?
Best regards
Heinrich
>
> Signed-off-by: Shantur Rathore <i at shantur.com>
> ---
>
> (no changes since v1)
>
> include/efi_variable.h | 5 ++
> lib/efi_loader/Makefile | 2 +-
> lib/efi_loader/efi_var_common.c | 109 ++++++++++++++++++++++++++++
> lib/efi_loader/efi_var_file.c | 121 --------------------------------
> lib/efi_loader/efi_variable.c | 8 ++-
> 5 files changed, 122 insertions(+), 123 deletions(-)
>
> diff --git a/include/efi_variable.h b/include/efi_variable.h
> index 805e6c5f1e..bd0a31fc3e 100644
> --- a/include/efi_variable.h
> +++ b/include/efi_variable.h
> @@ -161,6 +161,11 @@ efi_status_t efi_var_to_file(void);
> efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *lenp,
> u32 check_attr_mask);
>
> +/* GUID used by Shim to store the MOK database */
> +#define SHIM_LOCK_GUID \
> + EFI_GUID(0x605dab50, 0xe046, 0x4300, \
> + 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
> +
> /**
> * efi_var_restore() - restore EFI variables from buffer
> *
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 8d31fc61c6..33b1910249 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -66,7 +66,7 @@ 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
> +obj-$(CONFIG_EFI_VARIABLE_FILE_STORE) += efi_var_file.o
> ifeq ($(CONFIG_EFI_MM_COMM_TEE),y)
> obj-y += efi_variable_tee.o
> else
> diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
> index ad50bffd2b..7509c30b5a 100644
> --- a/lib/efi_loader/efi_var_common.c
> +++ b/lib/efi_loader/efi_var_common.c
> @@ -10,6 +10,7 @@
> #include <efi_loader.h>
> #include <efi_variable.h>
> #include <stdlib.h>
> +#include <u-boot/crc.h>
>
> enum efi_secure_mode {
> EFI_MODE_SETUP,
> @@ -40,6 +41,7 @@ static const struct efi_auth_var_name_type name_type[] = {
>
> static bool efi_secure_boot;
> static enum efi_secure_mode efi_secure_mode;
> +static const efi_guid_t shim_lock_guid = SHIM_LOCK_GUID;
>
> /**
> * efi_efi_get_variable() - retrieve value of a UEFI variable
> @@ -417,3 +419,110 @@ void *efi_get_var(const u16 *name, const efi_guid_t *vendor, efi_uintn_t *size)
>
> return buf;
> }
> +
> +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_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
> +{
> + struct efi_var_entry *var, *last_var;
> + u16 *data;
> + efi_status_t ret;
> +
> + if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
> + buf->crc32 != crc32(0, (u8 *)buf->var,
> + buf->length - sizeof(struct efi_var_file))) {
> + log_err("Invalid EFI variables file\n");
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
> + for (var = buf->var; var < last_var;
> + var = (struct efi_var_entry *)
> + ALIGN((uintptr_t)data + var->length, 8)) {
> +
> + data = var->name + u16_strlen(var->name) + 1;
> +
> + /*
> + * Secure boot related and volatile variables shall only be
> + * restored from U-Boot's preseed.
> + */
> + if (!safe &&
> + (efi_auth_var_get_type(var->name, &var->guid) !=
> + EFI_AUTH_VAR_NONE ||
> + !guidcmp(&var->guid, &shim_lock_guid) ||
> + !(var->attr & EFI_VARIABLE_NON_VOLATILE)))
> + continue;
> + if (!var->length)
> + continue;
> + if (efi_var_mem_find(&var->guid, var->name, NULL))
> + continue;
> + ret = efi_var_mem_ins(var->name, &var->guid, var->attr,
> + var->length, data, 0, NULL,
> + var->time);
> + if (ret != EFI_SUCCESS)
> + log_err("Failed to set EFI variable %ls\n", var->name);
> + }
> + return EFI_SUCCESS;
> +}
> diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> index d7dba05071..166501a355 100644
> --- a/lib/efi_loader/efi_var_file.c
> +++ b/lib/efi_loader/efi_var_file.c
> @@ -15,17 +15,9 @@
> #include <mapmem.h>
> #include <efi_loader.h>
> #include <efi_variable.h>
> -#include <u-boot/crc.h>
>
> #define PART_STR_LEN 10
>
> -/* GUID used by Shim to store the MOK database */
> -#define SHIM_LOCK_GUID \
> - EFI_GUID(0x605dab50, 0xe046, 0x4300, \
> - 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
> -
> -static const efi_guid_t shim_lock_guid = SHIM_LOCK_GUID;
> -
> /**
> * efi_set_blk_dev_to_system_partition() - select EFI system partition
> *
> @@ -53,70 +45,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
> *
> @@ -126,7 +54,6 @@ efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *
> */
> 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;
> @@ -150,52 +77,6 @@ error:
> log_err("Failed to persist EFI variables\n");
> free(buf);
> return ret;
> -#else
> - return EFI_SUCCESS;
> -#endif
> -}
> -
> -efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
> -{
> - struct efi_var_entry *var, *last_var;
> - u16 *data;
> - efi_status_t ret;
> -
> - if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
> - buf->crc32 != crc32(0, (u8 *)buf->var,
> - buf->length - sizeof(struct efi_var_file))) {
> - log_err("Invalid EFI variables file\n");
> - return EFI_INVALID_PARAMETER;
> - }
> -
> - last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
> - for (var = buf->var; var < last_var;
> - var = (struct efi_var_entry *)
> - ALIGN((uintptr_t)data + var->length, 8)) {
> -
> - data = var->name + u16_strlen(var->name) + 1;
> -
> - /*
> - * Secure boot related and volatile variables shall only be
> - * restored from U-Boot's preseed.
> - */
> - if (!safe &&
> - (efi_auth_var_get_type(var->name, &var->guid) !=
> - EFI_AUTH_VAR_NONE ||
> - !guidcmp(&var->guid, &shim_lock_guid) ||
> - !(var->attr & EFI_VARIABLE_NON_VOLATILE)))
> - continue;
> - if (!var->length)
> - continue;
> - if (efi_var_mem_find(&var->guid, var->name, NULL))
> - continue;
> - ret = efi_var_mem_ins(var->name, &var->guid, var->attr,
> - var->length, data, 0, NULL,
> - var->time);
> - if (ret != EFI_SUCCESS)
> - log_err("Failed to set EFI variable %ls\n", var->name);
> - }
> - return EFI_SUCCESS;
> }
>
> /**
> @@ -214,7 +95,6 @@ efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
> */
> efi_status_t efi_var_from_file(void)
> {
> -#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
> struct efi_var_file *buf;
> loff_t len;
> efi_status_t ret;
> @@ -239,6 +119,5 @@ efi_status_t efi_var_from_file(void)
> log_err("Invalid EFI variables file\n");
> error:
> free(buf);
> -#endif
> return EFI_SUCCESS;
> }
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index be95ed44e6..adc5ac6a80 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -357,8 +357,11 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
> * Write non-volatile EFI variables to file
> * TODO: check if a value change has occured to avoid superfluous writes
> */
> - if (attributes & EFI_VARIABLE_NON_VOLATILE)
> +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> + if (attributes & EFI_VARIABLE_NON_VOLATILE) {
> efi_var_to_file();
> + }
> +#endif
>
> return EFI_SUCCESS;
> }
> @@ -466,9 +469,12 @@ efi_status_t efi_init_variables(void)
> if (ret != EFI_SUCCESS)
> return ret;
>
> +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> ret = efi_var_from_file();
> if (ret != EFI_SUCCESS)
> return ret;
> +#endif
> +
> if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
> ret = efi_var_restore((struct efi_var_file *)
> __efi_var_file_begin, true);
More information about the U-Boot
mailing list