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 <[email protected]>
Signed-off-by: Heinrich Schuchardt <[email protected]>
Signed-off-by: Ilias Apalodimas <[email protected]>
---
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");
