[PATCH v2 3/4] efi_loader: add an EFI variable with the file contents
Heinrich Schuchardt
heinrich.schuchardt at canonical.com
Wed Apr 17 15:04:10 CEST 2024
On 17.04.24 12:19, Ilias Apalodimas wrote:
> Previous patches enabled SetVariableRT using a RAM backend.
> Although EBBR [0] defines a variable format we can teach userspace tools
> and write the altered variables, it's better if we skip the ABI
> requirements completely.
>
> So let's add a new variable, in its own namespace called "VarToFile"
> which contains a binary dump of the updated RT, BS and, NV variables
> and will be updated when GetVariable is called.
>
> Some adjustments are needed to do that.
> Currently we discard BS-only variables in EBS(). We need to preserve
> those on the RAM backend that exposes the variables. Since BS-only
> variables can't appear at runtime we need to move the memory masking
> checks from efi_var_collect() to efi_get_next_variable_name_mem()/
> efi_get_variable_mem() and do the filtering at runtime.
>
> We also need an efi_var_collect() variant available at runtime, in order
> to construct the "VarToFile" buffer on the fly.
>
> All users and applications (for linux) have to do when updating a variable
> is dd that variable in the file described by "RTStorageVolatile".
>
> Linux efivarfs uses a first 4 bytes of the output to represent attributes
> in little-endian format. So, storing variables works like this:
>
> $~ efibootmgr -n 0001
> $~ dd if=/sys/firmware/efi/efivars/VarToFile-b2ac5fc9-92b7-4acd-aeac-11e818c3130c of=/boot/efi/ubootefi.var skip=4 bs=1
>
> [0] https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-storage
>
> Co-developed-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
> include/efi_variable.h | 14 ++-
> lib/charset.c | 2 +-
> lib/efi_loader/efi_runtime.c | 19 ++++
> lib/efi_loader/efi_var_common.c | 6 +-
> lib/efi_loader/efi_var_mem.c | 146 ++++++++++++++++++------------
> lib/efi_loader/efi_variable.c | 6 +-
> lib/efi_loader/efi_variable_tee.c | 5 -
> 7 files changed, 130 insertions(+), 68 deletions(-)
>
> diff --git a/include/efi_variable.h b/include/efi_variable.h
> index 42a2b7c52bef..b545a36aac50 100644
> --- a/include/efi_variable.h
> +++ b/include/efi_variable.h
> @@ -271,13 +271,16 @@ const efi_guid_t *efi_auth_var_get_guid(const u16 *name);
> *
> * @variable_name_size: size of variable_name buffer in bytes
> * @variable_name: name of uefi variable's name in u16
> + * @mask: bitmask with required attributes of variables to be collected.
> + * variables are only collected if all of the required
> + * attributes match. Use 0 to skip matching
> * @vendor: vendor's guid
> *
> * Return: status code
> */
> efi_status_t __efi_runtime
> efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, u16 *variable_name,
> - efi_guid_t *vendor);
> + efi_guid_t *vendor, u32 mask);
> /**
> * efi_get_variable_mem() - Runtime common code across efi variable
> * implementations for GetVariable() from
> @@ -289,12 +292,15 @@ efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, u16 *variable_na
> * @data_size: size of the buffer to which the variable value is copied
> * @data: buffer to which the variable value is copied
> * @timep: authentication time (seconds since start of epoch)
> + * @mask: bitmask with required attributes of variables to be collected.
> + * variables are only collected if all of the required
> + * attributes match. Use 0 to skip matching
> * Return: status code
> */
> efi_status_t __efi_runtime
> efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor,
> u32 *attributes, efi_uintn_t *data_size, void *data,
> - u64 *timep);
> + u64 *timep, u32 mask);
>
> /**
> * efi_get_variable_runtime() - runtime implementation of GetVariable()
> @@ -334,4 +340,8 @@ efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size,
> */
> void efi_var_buf_update(struct efi_var_file *var_buf);
>
> +efi_status_t __efi_runtime efi_var_collect_mem(struct efi_var_file *buf,
> + efi_uintn_t *lenp,
> + u32 check_attr_mask);
> +
> #endif
> diff --git a/lib/charset.c b/lib/charset.c
> index df4f04074852..182c92a50c48 100644
> --- a/lib/charset.c
> +++ b/lib/charset.c
> @@ -387,7 +387,7 @@ int u16_strcasecmp(const u16 *s1, const u16 *s2)
> * > 0 if the first different u16 in s1 is greater than the
> * corresponding u16 in s2
> */
> -int u16_strncmp(const u16 *s1, const u16 *s2, size_t n)
> +int __efi_runtime u16_strncmp(const u16 *s1, const u16 *s2, size_t n)
> {
> int ret = 0;
>
> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> index c8f7a88ba8db..99ad1f35d8f1 100644
> --- a/lib/efi_loader/efi_runtime.c
> +++ b/lib/efi_loader/efi_runtime.c
> @@ -130,6 +130,8 @@ efi_status_t efi_init_runtime_supported(void)
> EFI_RT_SUPPORTED_CONVERT_POINTER;
>
> if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
> + int s = 0;
u8 s = 0;
should be enough.
> +
> ret = efi_set_variable_int(u"RTStorageVolatile",
> &efi_guid_efi_rt_var_file,
> EFI_VARIABLE_BOOTSERVICE_ACCESS |
> @@ -141,6 +143,23 @@ efi_status_t efi_init_runtime_supported(void)
> log_err("Failed to set RTStorageVolatile\n");
> return ret;
> }
> + ret = efi_set_variable_int(u"VarToFile",
> + &efi_guid_efi_rt_var_file,
> + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> + EFI_VARIABLE_RUNTIME_ACCESS,
Why is the variable set here? A comment would be helpful.
If you set it for protection, shouldn't it be EFI_VARIABLE_READ_ONLY?
An alternative would be to return EFI_INVALID_PARAMETER in
efi_set_variable_int for any attempt to set a variable with GUID
efi_guid_efi_rt_var_file.
> + sizeof(s),
> + &s, false);
> + if (ret != EFI_SUCCESS) {
> + log_err("Failed to set VarToFile\n");
> + efi_set_variable_int(u"RTStorageVolatile",
> + &efi_guid_efi_rt_var_file,
> + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> + EFI_VARIABLE_RUNTIME_ACCESS |
> + EFI_VARIABLE_READ_ONLY,
> + 0, NULL, false);
> +
> + return ret;
> + }
> rt_table->runtime_services_supported |= EFI_RT_SUPPORTED_SET_VARIABLE;
> }
>
> diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
> index aa8feffd3ec1..7862f2d6ce8a 100644
> --- a/lib/efi_loader/efi_var_common.c
> +++ b/lib/efi_loader/efi_var_common.c
> @@ -182,7 +182,8 @@ efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *guid,
> {
> efi_status_t ret;
>
> - ret = efi_get_variable_mem(variable_name, guid, attributes, data_size, data, NULL);
> + ret = efi_get_variable_mem(variable_name, guid, attributes, data_size,
> + data, NULL, EFI_VARIABLE_RUNTIME_ACCESS);
>
> /* Remove EFI_VARIABLE_READ_ONLY flag */
> if (attributes)
> @@ -195,7 +196,8 @@ efi_status_t __efi_runtime EFIAPI
> efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size,
> u16 *variable_name, efi_guid_t *guid)
> {
> - return efi_get_next_variable_name_mem(variable_name_size, variable_name, guid);
> + return efi_get_next_variable_name_mem(variable_name_size, variable_name,
> + guid, EFI_VARIABLE_RUNTIME_ACCESS);
> }
>
> /**
> diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c
> index 6c21cec5d457..65ab858c926e 100644
> --- a/lib/efi_loader/efi_var_mem.c
> +++ b/lib/efi_loader/efi_var_mem.c
> @@ -36,9 +36,11 @@ efi_var_mem_compare(struct efi_var_entry *var, const efi_guid_t *guid,
> const u16 *data, *var_name;
> bool match = true;
>
> - for (guid1 = (u8 *)&var->guid, guid2 = (u8 *)guid, i = 0;
> - i < sizeof(efi_guid_t) && match; ++i)
> - match = (guid1[i] == guid2[i]);
> + if (guid) {
> + for (guid1 = (u8 *)&var->guid, guid2 = (u8 *)guid, i = 0;
> + i < sizeof(efi_guid_t) && match; ++i)
> + match = (guid1[i] == guid2[i]);
> + }
>
> for (data = var->name, var_name = name;; ++data) {
> if (match)
> @@ -184,53 +186,6 @@ u64 __efi_runtime efi_var_mem_free(void)
> sizeof(struct efi_var_entry);
> }
>
> -/**
> - * efi_var_mem_bs_del() - delete boot service only variables
> - */
> -static void efi_var_mem_bs_del(void)
> -{
> - struct efi_var_entry *var = efi_var_buf->var;
> -
> - for (;;) {
> - struct efi_var_entry *last;
> -
> - last = (struct efi_var_entry *)
> - ((uintptr_t)efi_var_buf + efi_var_buf->length);
> - if (var >= last)
> - break;
> - if (var->attr & EFI_VARIABLE_RUNTIME_ACCESS) {
> - u16 *data;
> -
> - /* skip variable */
> - for (data = var->name; *data; ++data)
> - ;
> - ++data;
> - var = (struct efi_var_entry *)
> - ALIGN((uintptr_t)data + var->length, 8);
> - } else {
> - /* delete variable */
> - efi_var_mem_del(var);
> - }
> - }
> -}
> -
> -/**
> - * efi_var_mem_notify_exit_boot_services() - ExitBootService callback
> - *
> - * @event: callback event
> - * @context: callback context
> - */
> -static void EFIAPI
> -efi_var_mem_notify_exit_boot_services(struct efi_event *event, void *context)
> -{
> - EFI_ENTRY("%p, %p", event, context);
> -
> - /* Delete boot service only variables */
> - efi_var_mem_bs_del();
> -
> - EFI_EXIT(EFI_SUCCESS);
> -}
> -
> /**
> * efi_var_mem_notify_exit_boot_services() - SetVirtualMemoryMap callback
> *
> @@ -261,11 +216,7 @@ efi_status_t efi_var_mem_init(void)
> efi_var_buf->magic = EFI_VAR_FILE_MAGIC;
> efi_var_buf->length = (uintptr_t)efi_var_buf->var -
> (uintptr_t)efi_var_buf;
> - /* crc32 for 0 bytes = 0 */
>
> - ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK,
> - efi_var_mem_notify_exit_boot_services, NULL,
> - NULL, &event);
> if (ret != EFI_SUCCESS)
> return ret;
> ret = efi_create_event(EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE, TPL_CALLBACK,
> @@ -276,10 +227,75 @@ efi_status_t efi_var_mem_init(void)
> return ret;
> }
>
> +/**
> + * efi_var_collect_mem() - Copy EFI variables matching attributes mask from
> + * efi_var_buf
> + *
> + * @buf: buffer containing variable collection
> + * @lenp: buffer length
> + * @mask: mask of matched attributes
> + *
> + * Return: Status code
> + */
> +efi_status_t __efi_runtime
> +efi_var_collect_mem(struct efi_var_file *buf, efi_uintn_t *lenp, u32 mask)
> +{
> + static struct efi_var_file __efi_runtime_data hdr = {
> + .magic = EFI_VAR_FILE_MAGIC,
> + };
> + struct efi_var_entry *last, *var, *var_to;
> +
> + hdr.length = sizeof(struct efi_var_file);
> +
> + var = efi_var_buf->var;
> + last = (struct efi_var_entry *)
> + ((uintptr_t)efi_var_buf + efi_var_buf->length);
> + if (buf)
> + var_to = buf->var;
> +
> + while (var < last) {
> + u32 len;
> + struct efi_var_entry *var_next;
> +
> + efi_var_mem_compare(var, NULL, u"", &var_next);
On IRC you suggested to make u16_strlen __efi_runtime_code to replace
this efi_var_mem_compare() call and avoid the modifications of said
function. That might be more readable.
Best regards
Heinrich
> + len = (uintptr_t)var_next - (uintptr_t)var;
> +
> + if ((var->attr & mask) != mask) {
> + var = (void *)var + len;
> + continue;
> + }
> +
> + hdr.length += len;
> +
> + if (buf && hdr.length <= *lenp) {
> + efi_memcpy_runtime(var_to, var, len);
> + var_to = (void *)var_to + len;
> + }
> + var = (void *)var + len;
> + }
> +
> + if (!buf && hdr.length <= *lenp) {
> + *lenp = hdr.length;
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if (!buf || hdr.length > *lenp) {
> + *lenp = hdr.length;
> + return EFI_BUFFER_TOO_SMALL;
> + }
> + hdr.crc32 = crc32(0, (u8 *)buf->var,
> + hdr.length - sizeof(struct efi_var_file));
> +
> + efi_memcpy_runtime(buf, &hdr, sizeof(hdr));
> + *lenp = hdr.length;
> +
> + return EFI_SUCCESS;
> +}
> +
> efi_status_t __efi_runtime
> efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor,
> u32 *attributes, efi_uintn_t *data_size, void *data,
> - u64 *timep)
> + u64 *timep, u32 mask)
> {
> efi_uintn_t old_size;
> struct efi_var_entry *var;
> @@ -291,11 +307,22 @@ efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor,
> if (!var)
> return EFI_NOT_FOUND;
>
> + /*
> + * This function is used at runtime to dump EFI variables.
> + * The memory backend we keep around has BS-only variables as
> + * well. At runtime we filter them here
> + */
> + if (mask && !((var->attr & mask) == mask))
> + return EFI_NOT_FOUND;
> +
> if (attributes)
> *attributes = var->attr;
> if (timep)
> *timep = var->time;
>
> + if (!u16_strcmp(variable_name, u"VarToFile"))
> + return efi_var_collect_mem(data, data_size, EFI_VARIABLE_NON_VOLATILE);
> +
> old_size = *data_size;
> *data_size = var->length;
> if (old_size < var->length)
> @@ -315,7 +342,8 @@ efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor,
>
> efi_status_t __efi_runtime
> efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size,
> - u16 *variable_name, efi_guid_t *vendor)
> + u16 *variable_name, efi_guid_t *vendor,
> + u32 mask)
> {
> struct efi_var_entry *var;
> efi_uintn_t len, old_size;
> @@ -324,6 +352,7 @@ efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size,
> if (!variable_name_size || !variable_name || !vendor)
> return EFI_INVALID_PARAMETER;
>
> +skip:
> len = *variable_name_size >> 1;
> if (u16_strnlen(variable_name, len) == len)
> return EFI_INVALID_PARAMETER;
> @@ -347,6 +376,11 @@ efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size,
> efi_memcpy_runtime(variable_name, var->name, *variable_name_size);
> efi_memcpy_runtime(vendor, &var->guid, sizeof(efi_guid_t));
>
> + if (mask && !((var->attr & mask) == mask)) {
> + *variable_name_size = old_size;
> + goto skip;
> + }
> +
> return EFI_SUCCESS;
> }
>
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index abc2a3402f42..4aaa05a617d7 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -209,14 +209,16 @@ efi_get_variable_int(const u16 *variable_name, const efi_guid_t *vendor,
> u32 *attributes, efi_uintn_t *data_size, void *data,
> u64 *timep)
> {
> - return efi_get_variable_mem(variable_name, vendor, attributes, data_size, data, timep);
> + return efi_get_variable_mem(variable_name, vendor, attributes, data_size,
> + data, timep, 0);
> }
>
> efi_status_t __efi_runtime
> efi_get_next_variable_name_int(efi_uintn_t *variable_name_size,
> u16 *variable_name, efi_guid_t *vendor)
> {
> - return efi_get_next_variable_name_mem(variable_name_size, variable_name, vendor);
> + return efi_get_next_variable_name_mem(variable_name_size, variable_name,
> + vendor, 0);
> }
>
> /**
> diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> index dde135fd9f81..4f1aa298da13 100644
> --- a/lib/efi_loader/efi_variable_tee.c
> +++ b/lib/efi_loader/efi_variable_tee.c
> @@ -959,11 +959,6 @@ void efi_variables_boot_exit_notify(void)
> log_err("Unable to notify the MM partition for ExitBootServices\n");
> free(comm_buf);
>
> - /*
> - * Populate the list for runtime variables.
> - * asking EFI_VARIABLE_RUNTIME_ACCESS is redundant, since
> - * efi_var_mem_notify_exit_boot_services will clean those, but that's fine
> - */
> ret = efi_var_collect(&var_buf, &len, EFI_VARIABLE_RUNTIME_ACCESS);
> if (ret != EFI_SUCCESS)
> log_err("Can't populate EFI variables. No runtime variables will be available\n");
More information about the U-Boot
mailing list