[PATCH v1 0/4] Enable SetvariableRT
Hi all,
This is an updated version of [0].
When EFI variables are stored on file we don't allow SetVariableRT, since the OS doesn't know how to access or write that file. At the same time keeping the U-Boot drivers alive in runtime sections and performing writes from the firmware is dangerous -- if at all possible.
For GetVariableRT we copy runtime variables in RAM and expose them to the OS. Add a Kconfig option and provide SetVariableRT using the same memory back end. The OS will be responsible for syncing the RAM contents to the file, otherwise any changes made during runtime won't persist reboots.
It's worth noting that the variable store format is defined in EBBR [1] and authenticated variables are explicitly prohibited, since they have to be stored on a medium that's tamper and rollback protected.
The original RFC was just enabling the memory back end. This is a more complete version and we should be able, with small adjustments of userspace tools, fix SetVariableRT. If enabled the firmware will add two new RO EFI variables after ExitBootServices.
RTStorageVolatile -- contains the filename, relative to the ESP VarToFile -- an EFI variable that holds all the BS,RT, NV variables and can be copied to the file defined by RTStorageVolatile.
If any errors occur during the variable init, the firmware will delete them and adjust the RT_PROP table accordingly, disabling SetvarRT.
- pre-patch $~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (ro,nosuid,nodev,noexec,relatime)
$~ efibootmgr -n 0001 Could not set BootNext: Read-only file system
- post-patch $~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (rw,nosuid,nodev,noexec,relatime) , disabling SetvarRT. $~ efibootmgr -n 0001 BootNext: 0001 BootCurrent: 0000 BootOrder: 0000,0001 Boot0000* debian HD(1,GPT,bdae5610-3331-4e4d-9466-acb5caf0b4a6,0x800,0x100000)/File(EFI\debian\grubaa64.efi) Boot0001* virtio 0 VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,0000000000000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,850000001f000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,1600850000000000){auto_created_boot_option}
$~ efivar -p -n 8be4df61-93ca-11d2-aa0d-00e098032b8c-BootNext GUID: 8be4df61-93ca-11d2-aa0d-00e098032b8c Name: "BootNext" Attributes: Non-Volatile Boot Service Access Runtime Service Access Value: 00000000 01 00
[0] https://lore.kernel.org/u-boot/20240329071929.2461441-1-ilias.apalodimas@lin... [1] https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-st...
Changes since the RFC: - Return EFI_INVALID_PARAM if attributes are not volatile - Add EFI_WRITE_PROTECTED checks for BS, RT *only* variables - Add 2 EFI variables and allow userspace to write the file - Add selftests
Ilias Apalodimas (4): efi_loader: conditionally enable SetvariableRT efi_loader: Add OS notifications for SetVariableRT in RAM efi_loader: add an EFI variable with the variable file contents efi_selftest: add tests for setvariableRT
include/efi_loader.h | 4 + include/efi_variable.h | 15 +- lib/efi_loader/Kconfig | 16 ++ lib/efi_loader/efi_boottime.c | 2 + lib/efi_loader/efi_runtime.c | 1 + lib/efi_loader/efi_var_common.c | 43 ++-- lib/efi_loader/efi_var_file.c | 1 - lib/efi_loader/efi_var_mem.c | 90 +++----- lib/efi_loader/efi_variable.c | 210 +++++++++++++++++- lib/efi_loader/efi_variable_tee.c | 1 - .../efi_selftest_variables_runtime.c | 116 +++++++++- 11 files changed, 401 insertions(+), 98 deletions(-)
-- 2.37.2
When EFI variables are stored on file we don't allow SetVariableRT, since the OS doesn't know how to access or write that file. At the same time keeping the U-Boot drivers alive in runtime sections and performing writes from the firmware is dangerous -- if at all possible.
For GetVariableRT we copy runtime variables in RAM and expose them to the OS. Add a Kconfig option and provide SetVariableRT using the same memory backend. The OS will be responsible for syncing the RAM contents to the file, otherwise any changes made during runtime won't persist reboots.
It's worth noting that the variable store format is defined in EBBR [0] and authenticated variables are explicitly prohibited, since they have to be stored on a medium that's tamper and rollback protected.
- pre-patch $~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (ro,nosuid,nodev,noexec,relatime)
$~ efibootmgr -n 0001 Could not set BootNext: Read-only file system
- post-patch $~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (rw,nosuid,nodev,noexec,relatime)
$~ efibootmgr -n 0001 BootNext: 0001 BootCurrent: 0000 BootOrder: 0000,0001 Boot0000* debian HD(1,GPT,bdae5610-3331-4e4d-9466-acb5caf0b4a6,0x800,0x100000)/File(EFI\debian\grubaa64.efi) Boot0001* virtio 0 VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,0000000000000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,850000001f000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,1600850000000000){auto_created_boot_option}
$~ efivar -p -n 8be4df61-93ca-11d2-aa0d-00e098032b8c-BootNext GUID: 8be4df61-93ca-11d2-aa0d-00e098032b8c Name: "BootNext" Attributes: Non-Volatile Boot Service Access Runtime Service Access Value: 00000000 01 00
[0] https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-st...
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- lib/efi_loader/Kconfig | 16 +++ lib/efi_loader/efi_runtime.c | 5 + lib/efi_loader/efi_variable.c | 114 ++++++++++++++++-- .../efi_selftest_variables_runtime.c | 13 +- 4 files changed, 135 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index a7c3e05c13a0..b210ceea6d64 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -63,6 +63,22 @@ config EFI_VARIABLE_FILE_STORE Select this option if you want non-volatile UEFI variables to be stored as file /ubootefi.var on the EFI system partition.
+config EFI_RT_VOLATILE_STORE + bool "Allow variable runtime services in volatile storage (e.g RAM)" + depends on EFI_VARIABLE_FILE_STORE + help + When EFI variables are stored on file we don't allow SetVariableRT, + since the OS doesn't know how to write that file. At he same time + we copy runtime variables in DRAM and support GetVariableRT + + Enable this option to allow SetVariableRT on the RAM backend of + the EFI variable storage. The OS will be responsible for syncing + the RAM contents to the file, otherwise any changes made during + runtime won't persist reboots. + Authenticated variables are not supported. Note that this will + violate the EFI spec since writing auth variables will return + EFI_INVALID_PARAMETER + config EFI_MM_COMM_TEE bool "UEFI variables storage service via the trusted world" depends on OPTEE diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 18da6892e796..8ebbea7e5c69 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -10,6 +10,7 @@ #include <dm.h> #include <elf.h> #include <efi_loader.h> +#include <efi_variable.h> #include <log.h> #include <malloc.h> #include <rtc.h> @@ -126,6 +127,10 @@ efi_status_t efi_init_runtime_supported(void) EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP | EFI_RT_SUPPORTED_CONVERT_POINTER;
+ if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) + rt_table->runtime_services_supported |= + EFI_RT_SUPPORTED_SET_VARIABLE; + /* * This value must be synced with efi_runtime_detach_list * as well as efi_runtime_services. diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 6fe3792a12a5..f79041e6bedd 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -218,17 +218,20 @@ efi_get_next_variable_name_int(efi_uintn_t *variable_name_size, return efi_get_next_variable_name_mem(variable_name_size, variable_name, vendor); }
-efi_status_t efi_set_variable_int(const u16 *variable_name, - const efi_guid_t *vendor, - u32 attributes, efi_uintn_t data_size, - const void *data, bool ro_check) +/** + * setvariable_allowed() - checks defined by the UEFI spec for setvariable + * + * @variable_name: name of the variable + * @vendor: vendor GUID + * @attributes: attributes of the variable + * @data_size: size of the buffer with the variable value + * @data: buffer with the variable value + * Return: status code + */ +static efi_status_t __efi_runtime +setvariable_allowed(const u16 *variable_name, const efi_guid_t *vendor, + u32 attributes, efi_uintn_t data_size, const void *data) { - struct efi_var_entry *var; - efi_uintn_t ret; - bool append, delete; - u64 time = 0; - enum efi_auth_var_type var_type; - if (!variable_name || !*variable_name || !vendor) return EFI_INVALID_PARAMETER;
@@ -260,6 +263,25 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) return EFI_INVALID_PARAMETER;
+ return EFI_SUCCESS; +} + +efi_status_t efi_set_variable_int(const u16 *variable_name, + const efi_guid_t *vendor, + u32 attributes, efi_uintn_t data_size, + const void *data, bool ro_check) +{ + struct efi_var_entry *var; + efi_uintn_t ret; + bool append, delete; + u64 time = 0; + enum efi_auth_var_type var_type; + + ret = setvariable_allowed(variable_name, vendor, attributes, data_size, + data); + if (ret != EFI_SUCCESS) + return ret; + /* check if a variable exists */ var = efi_var_mem_find(vendor, variable_name, NULL); append = !!(attributes & EFI_VARIABLE_APPEND_WRITE); @@ -452,6 +474,78 @@ efi_set_variable_runtime(u16 *variable_name, const efi_guid_t *vendor, u32 attributes, efi_uintn_t data_size, const void *data) { +if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { + struct efi_var_entry *var; + efi_uintn_t ret; + bool append, delete; + u64 time = 0; + + /* + * Authenticated variables are not supported the rest of the checks + * are in setvariable_allowed() + */ + if (attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) + return EFI_INVALID_PARAMETER; + /* BS only variables are hidden deny writing them */ + if (!(attributes & EFI_VARIABLE_RUNTIME_ACCESS)) + return EFI_INVALID_PARAMETER; + + ret = setvariable_allowed(variable_name, vendor, attributes, data_size, + data); + if (ret != EFI_SUCCESS) + return ret; + + /* check if a variable exists */ + var = efi_var_mem_find(vendor, variable_name, NULL); + append = !!(attributes & EFI_VARIABLE_APPEND_WRITE); + attributes &= ~EFI_VARIABLE_APPEND_WRITE; + delete = !append && (!data_size || !attributes); + + if (var) { + if (var->attr & EFI_VARIABLE_READ_ONLY || + !(var->attr & EFI_VARIABLE_NON_VOLATILE)) + return EFI_WRITE_PROTECTED; + + /* attributes won't be changed */ + if (!delete && (((var->attr & ~EFI_VARIABLE_READ_ONLY) != + (attributes & ~EFI_VARIABLE_READ_ONLY)))) + return EFI_INVALID_PARAMETER; + time = var->time; + } else { + if (!(attributes & EFI_VARIABLE_NON_VOLATILE)) + return EFI_INVALID_PARAMETER; + if (append && !data_size) + return EFI_SUCCESS; + if (delete) + return EFI_NOT_FOUND; + } + + if (delete) { + /* EFI_NOT_FOUND has been handled before */ + attributes = var->attr; + ret = EFI_SUCCESS; + } else if (append && var) { + u16 *old_data = var->name; + + for (; *old_data; ++old_data) + ; + ++old_data; + ret = efi_var_mem_ins(variable_name, vendor, attributes, + var->length, old_data, data_size, data, + time); + } else { + ret = efi_var_mem_ins(variable_name, vendor, attributes, + data_size, data, 0, NULL, time); + } + + if (ret != EFI_SUCCESS) + return ret; + /* We are always inserting new variables, get rid of the old copy */ + efi_var_mem_del(var); + + return EFI_SUCCESS; +} else + return EFI_UNSUPPORTED; }
diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c index 4700d9424105..4c9405c0a7c7 100644 --- a/lib/efi_selftest/efi_selftest_variables_runtime.c +++ b/lib/efi_selftest/efi_selftest_variables_runtime.c @@ -62,9 +62,16 @@ static int execute(void) EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS, 3, v + 4); - if (ret != EFI_UNSUPPORTED) { - efi_st_error("SetVariable failed\n"); - return EFI_ST_FAILURE; + if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { + if (ret != EFI_INVALID_PARAMETER) { + efi_st_error("SetVariable failed\n"); + return EFI_ST_FAILURE; + } + } else { + if (ret != EFI_UNSUPPORTED) { + efi_st_error("SetVariable failed\n"); + return EFI_ST_FAILURE; + } } len = EFI_ST_MAX_DATA_SIZE; ret = runtime->get_variable(u"PlatformLangCodes", &guid_vendor0,
On 4/6/24 16:01, Ilias Apalodimas wrote:
When EFI variables are stored on file we don't allow SetVariableRT,
Neither in the UEFI standard nor in U-Boot we have a function SetVariableRT.
%s/SetVariableRT/SetVariable at runtime/
since the OS doesn't know how to access or write that file. At the same time keeping the U-Boot drivers alive in runtime sections and performing writes from the firmware is dangerous -- if at all possible.
For GetVariableRT we copy runtime variables in RAM and expose them to
%s/GetVariabeRT/GetVariable at runtime/
the OS. Add a Kconfig option and provide SetVariableRT using the same memory backend. The OS will be responsible for syncing the RAM contents to the file, otherwise any changes made during runtime won't persist reboots.
It's worth noting that the variable store format is defined in EBBR [0] and authenticated variables are explicitly prohibited, since they have to be stored on a medium that's tamper and rollback protected.
- pre-patch
$~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (ro,nosuid,nodev,noexec,relatime)
$~ efibootmgr -n 0001 Could not set BootNext: Read-only file system
- post-patch
$~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (rw,nosuid,nodev,noexec,relatime)
$~ efibootmgr -n 0001 BootNext: 0001 BootCurrent: 0000 BootOrder: 0000,0001 Boot0000* debian HD(1,GPT,bdae5610-3331-4e4d-9466-acb5caf0b4a6,0x800,0x100000)/File(EFI\debian\grubaa64.efi) Boot0001* virtio 0 VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,0000000000000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,850000001f000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,1600850000000000){auto_created_boot_option}
$~ efivar -p -n 8be4df61-93ca-11d2-aa0d-00e098032b8c-BootNext GUID: 8be4df61-93ca-11d2-aa0d-00e098032b8c Name: "BootNext" Attributes: Non-Volatile Boot Service Access Runtime Service Access Value: 00000000 01 00
[0] https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-st...
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
lib/efi_loader/Kconfig | 16 +++ lib/efi_loader/efi_runtime.c | 5 + lib/efi_loader/efi_variable.c | 114 ++++++++++++++++-- .../efi_selftest_variables_runtime.c | 13 +- 4 files changed, 135 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index a7c3e05c13a0..b210ceea6d64 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -63,6 +63,22 @@ config EFI_VARIABLE_FILE_STORE Select this option if you want non-volatile UEFI variables to be stored as file /ubootefi.var on the EFI system partition.
+config EFI_RT_VOLATILE_STORE
- bool "Allow variable runtime services in volatile storage (e.g RAM)"
- depends on EFI_VARIABLE_FILE_STORE
- help
When EFI variables are stored on file we don't allow SetVariableRT,since the OS doesn't know how to write that file. At he same timewe copy runtime variables in DRAM and support GetVariableRTEnable this option to allow SetVariableRT on the RAM backend ofthe EFI variable storage. The OS will be responsible for syncingthe RAM contents to the file, otherwise any changes made duringruntime won't persist reboots.Authenticated variables are not supported. Note that this willviolate the EFI spec since writing auth variables will returnEFI_INVALID_PARAMETER- config EFI_MM_COMM_TEE bool "UEFI variables storage service via the trusted world" depends on OPTEE
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 18da6892e796..8ebbea7e5c69 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -10,6 +10,7 @@ #include <dm.h> #include <elf.h> #include <efi_loader.h> +#include <efi_variable.h> #include <log.h> #include <malloc.h> #include <rtc.h> @@ -126,6 +127,10 @@ efi_status_t efi_init_runtime_supported(void) EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP | EFI_RT_SUPPORTED_CONVERT_POINTER;
- if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE))
rt_table->runtime_services_supported |=EFI_RT_SUPPORTED_SET_VARIABLE;- /*
- This value must be synced with efi_runtime_detach_list
- as well as efi_runtime_services.
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 6fe3792a12a5..f79041e6bedd 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -218,17 +218,20 @@ efi_get_next_variable_name_int(efi_uintn_t *variable_name_size, return efi_get_next_variable_name_mem(variable_name_size, variable_name, vendor); }
-efi_status_t efi_set_variable_int(const u16 *variable_name,
const efi_guid_t *vendor,u32 attributes, efi_uintn_t data_size,const void *data, bool ro_check)+/**
- setvariable_allowed() - checks defined by the UEFI spec for setvariable
- @variable_name: name of the variable
- @vendor: vendor GUID
- @attributes: attributes of the variable
- @data_size: size of the buffer with the variable value
- @data: buffer with the variable value
- Return: status code
- */
+static efi_status_t __efi_runtime +setvariable_allowed(const u16 *variable_name, const efi_guid_t *vendor,
u32 attributes, efi_uintn_t data_size, const void *data)
Can we reuse this function in efi_query_variable_info_int()?
{
- struct efi_var_entry *var;
- efi_uintn_t ret;
- bool append, delete;
- u64 time = 0;
- enum efi_auth_var_type var_type;
- if (!variable_name || !*variable_name || !vendor) return EFI_INVALID_PARAMETER;
@@ -260,6 +263,25 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) return EFI_INVALID_PARAMETER;
- return EFI_SUCCESS;
+}
+efi_status_t efi_set_variable_int(const u16 *variable_name,
const efi_guid_t *vendor,u32 attributes, efi_uintn_t data_size,const void *data, bool ro_check)+{
- struct efi_var_entry *var;
- efi_uintn_t ret;
- bool append, delete;
- u64 time = 0;
- enum efi_auth_var_type var_type;
- ret = setvariable_allowed(variable_name, vendor, attributes, data_size,
data);- if (ret != EFI_SUCCESS)
return ret;- /* check if a variable exists */ var = efi_var_mem_find(vendor, variable_name, NULL); append = !!(attributes & EFI_VARIABLE_APPEND_WRITE);
@@ -452,6 +474,78 @@ efi_set_variable_runtime(u16 *variable_name, const efi_guid_t *vendor, u32 attributes, efi_uintn_t data_size, const void *data) { +if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
This line should be indented by one tab.
Can we put the EFI_UNSUPPORTED exit on top to avoid a very long if branch:
if (!IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) return EFI_UNSUPPORTED.
If we implement SetVariable() at runtime, we should also enable QueryVariableInfo().
- struct efi_var_entry *var;
- efi_uintn_t ret;
- bool append, delete;
- u64 time = 0;
- /*
* Authenticated variables are not supported the rest of the checks* are in setvariable_allowed()
Please, describe here why they are not implemented, yet.
*/- if (attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)
return EFI_INVALID_PARAMETER;- /* BS only variables are hidden deny writing them */
- if (!(attributes & EFI_VARIABLE_RUNTIME_ACCESS))
return EFI_INVALID_PARAMETER;
Isn't this duplicating logic from efi_set_variable_int()? Can we carve out more common code?
Best regards
Heinrich
- ret = setvariable_allowed(variable_name, vendor, attributes, data_size,
data);- if (ret != EFI_SUCCESS)
return ret;- /* check if a variable exists */
- var = efi_var_mem_find(vendor, variable_name, NULL);
- append = !!(attributes & EFI_VARIABLE_APPEND_WRITE);
- attributes &= ~EFI_VARIABLE_APPEND_WRITE;
- delete = !append && (!data_size || !attributes);
- if (var) {
if (var->attr & EFI_VARIABLE_READ_ONLY ||!(var->attr & EFI_VARIABLE_NON_VOLATILE))return EFI_WRITE_PROTECTED;/* attributes won't be changed */if (!delete && (((var->attr & ~EFI_VARIABLE_READ_ONLY) !=(attributes & ~EFI_VARIABLE_READ_ONLY))))return EFI_INVALID_PARAMETER;time = var->time;- } else {
if (!(attributes & EFI_VARIABLE_NON_VOLATILE))return EFI_INVALID_PARAMETER;if (append && !data_size)return EFI_SUCCESS;if (delete)return EFI_NOT_FOUND;- }
- if (delete) {
/* EFI_NOT_FOUND has been handled before */attributes = var->attr;ret = EFI_SUCCESS;- } else if (append && var) {
u16 *old_data = var->name;for (; *old_data; ++old_data);++old_data;ret = efi_var_mem_ins(variable_name, vendor, attributes,var->length, old_data, data_size, data,time);- } else {
ret = efi_var_mem_ins(variable_name, vendor, attributes,data_size, data, 0, NULL, time);- }
- if (ret != EFI_SUCCESS)
return ret;- /* We are always inserting new variables, get rid of the old copy */
- efi_var_mem_del(var);
- return EFI_SUCCESS;
+} else
- return EFI_UNSUPPORTED; }
diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c index 4700d9424105..4c9405c0a7c7 100644 --- a/lib/efi_selftest/efi_selftest_variables_runtime.c +++ b/lib/efi_selftest/efi_selftest_variables_runtime.c @@ -62,9 +62,16 @@ static int execute(void) EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS, 3, v + 4);
- if (ret != EFI_UNSUPPORTED) {
efi_st_error("SetVariable failed\n");return EFI_ST_FAILURE;
- if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
if (ret != EFI_INVALID_PARAMETER) {efi_st_error("SetVariable failed\n");return EFI_ST_FAILURE;}- } else {
if (ret != EFI_UNSUPPORTED) {efi_st_error("SetVariable failed\n");return EFI_ST_FAILURE;}
Previous patches enable SetVariableRT using a volatile storage backend using EFI_RUNTIME_SERVICES_DATA allocared memory. Since there's no recommendation from the spec on how to notify the OS, add a volatile EFI variable that contains the filename relative to the ESP. OS'es can use that file and update it at runtime
$~ efivar -p -n b2ac5fc9-92b7-4acd-aeac-11e818c3130c-RTStorageVolatile GUID: b2ac5fc9-92b7-4acd-aeac-11e818c3130c Name: "RTStorageVolatile" Attributes: Boot Service Access Runtime Service Access Value: 00000000 75 62 6f 6f 74 65 66 69 2e 76 61 72 00 |ubootefi.var. |
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- include/efi_loader.h | 4 ++++ lib/efi_loader/efi_runtime.c | 4 ---- lib/efi_loader/efi_variable.c | 20 ++++++++++++++++++++ 3 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 7daca0afba2b..25551fe18948 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -159,6 +159,10 @@ static inline void efi_set_bootdev(const char *dev, const char *devnr, #define EFICONFIG_AUTO_GENERATED_ENTRY_GUID \ EFI_GUID(0x8108ac4e, 0x9f11, 0x4d59, \ 0x85, 0x0e, 0xe2, 0x1a, 0x52, 0x2c, 0x59, 0xb2) +#define U_BOOT_EFI_RT_VAR_FILE_GUID \ + EFI_GUID(0xb2ac5fc9, 0x92b7, 0x4acd, \ + 0xae, 0xac, 0x11, 0xe8, 0x18, 0xc3, 0x13, 0x0c) +
/* Use internal device tree when starting UEFI application */ #define EFI_FDT_USE_INTERNAL NULL diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 8ebbea7e5c69..d898ba6c268f 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -127,10 +127,6 @@ efi_status_t efi_init_runtime_supported(void) EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP | EFI_RT_SUPPORTED_CONVERT_POINTER;
- if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) - rt_table->runtime_services_supported |= - EFI_RT_SUPPORTED_SET_VARIABLE; - /* * This value must be synced with efi_runtime_detach_list * as well as efi_runtime_services. diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index f79041e6bedd..f97c8c57f75c 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -554,6 +554,26 @@ if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { */ void efi_variables_boot_exit_notify(void) { + const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID; + const efi_guid_t rt_prop_guid = EFI_RT_PROPERTIES_TABLE_GUID; + efi_status_t ret; + + if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { + struct efi_rt_properties_table *rt_prop = + efi_get_configuration_table(&rt_prop_guid); + + ret = efi_set_variable_int(u"RTStorageVolatile", + &efi_guid_efi_rt_var_file, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_READ_ONLY, + sizeof(EFI_VAR_FILE_NAME), + EFI_VAR_FILE_NAME, false); + if (ret != EFI_SUCCESS) + rt_prop->runtime_services_supported |= ~EFI_RT_SUPPORTED_SET_VARIABLE; + else + log_err("Can't RTStorage. SetVariableRT won't be available\n"); + } /* Switch variable services functions to runtime version */ efi_runtime_services.get_variable = efi_get_variable_runtime; efi_runtime_services.get_next_variable_name =
On 4/6/24 16:01, Ilias Apalodimas wrote:
Previous patches enable SetVariableRT using a volatile storage backend using EFI_RUNTIME_SERVICES_DATA allocared memory. Since there's no recommendation from the spec on how to notify the OS, add a volatile EFI variable that contains the filename relative to the ESP. OS'es can use that file and update it at runtime
$~ efivar -p -n b2ac5fc9-92b7-4acd-aeac-11e818c3130c-RTStorageVolatile GUID: b2ac5fc9-92b7-4acd-aeac-11e818c3130c Name: "RTStorageVolatile" Attributes: Boot Service Access Runtime Service Access Value: 00000000 75 62 6f 6f 74 65 66 69 2e 76 61 72 00 |ubootefi.var. |
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
include/efi_loader.h | 4 ++++ lib/efi_loader/efi_runtime.c | 4 ---- lib/efi_loader/efi_variable.c | 20 ++++++++++++++++++++ 3 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 7daca0afba2b..25551fe18948 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -159,6 +159,10 @@ static inline void efi_set_bootdev(const char *dev, const char *devnr, #define EFICONFIG_AUTO_GENERATED_ENTRY_GUID \ EFI_GUID(0x8108ac4e, 0x9f11, 0x4d59, \ 0x85, 0x0e, 0xe2, 0x1a, 0x52, 0x2c, 0x59, 0xb2) +#define U_BOOT_EFI_RT_VAR_FILE_GUID \
EFI_GUID(0xb2ac5fc9, 0x92b7, 0x4acd, \
0xae, 0xac, 0x11, 0xe8, 0x18, 0xc3, 0x13, 0x0c)/* Use internal device tree when starting UEFI application */ #define EFI_FDT_USE_INTERNAL NULL
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 8ebbea7e5c69..d898ba6c268f 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -127,10 +127,6 @@ efi_status_t efi_init_runtime_supported(void) EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP | EFI_RT_SUPPORTED_CONVERT_POINTER;
- if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE))
rt_table->runtime_services_supported |=EFI_RT_SUPPORTED_SET_VARIABLE;
Why do you want to remove this flag?
Best regards
Heinrich
/* * This value must be synced with efi_runtime_detach_list * as well as efi_runtime_services. diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index f79041e6bedd..f97c8c57f75c 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -554,6 +554,26 @@ if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { */ void efi_variables_boot_exit_notify(void) {
- const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID;
- const efi_guid_t rt_prop_guid = EFI_RT_PROPERTIES_TABLE_GUID;
- efi_status_t ret;
- if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
struct efi_rt_properties_table *rt_prop =efi_get_configuration_table(&rt_prop_guid);ret = efi_set_variable_int(u"RTStorageVolatile",&efi_guid_efi_rt_var_file,EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS |EFI_VARIABLE_READ_ONLY,sizeof(EFI_VAR_FILE_NAME),EFI_VAR_FILE_NAME, false);if (ret != EFI_SUCCESS)rt_prop->runtime_services_supported |= ~EFI_RT_SUPPORTED_SET_VARIABLE;elselog_err("Can't RTStorage. SetVariableRT won't be available\n");- } /* Switch variable services functions to runtime version */ efi_runtime_services.get_variable = efi_get_variable_runtime; efi_runtime_services.get_next_variable_name =
Hi Heinrich,
/* Use internal device tree when starting UEFI application */ #define EFI_FDT_USE_INTERNAL NULL
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 8ebbea7e5c69..d898ba6c268f 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -127,10 +127,6 @@ efi_status_t efi_init_runtime_supported(void) EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP | EFI_RT_SUPPORTED_CONVERT_POINTER;
if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE))rt_table->runtime_services_supported |=EFI_RT_SUPPORTED_SET_VARIABLE;
[...]
Why do you want to remove this flag?
I don't, I messed this up during my rebase before sending the patches.
The code in EBS() was supposed to re-enable it. It's all fixed in patch #3, but this patch needs a change as well rt_prop->runtime_services_supported |= ~EFI_RT_SUPPORTED_SET_VARIABLE; should be rt_prop->runtime_services_supported |= EFI_RT_SUPPORTED_SET_VARIABLE; in efi_variables_boot_exit_notify()
Thanks /Ilias
Best regards
Heinrich
/* * This value must be synced with efi_runtime_detach_list * as well as efi_runtime_services.diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index f79041e6bedd..f97c8c57f75c 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -554,6 +554,26 @@ if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { */ void efi_variables_boot_exit_notify(void) {
const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID;const efi_guid_t rt_prop_guid = EFI_RT_PROPERTIES_TABLE_GUID;efi_status_t ret;if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {struct efi_rt_properties_table *rt_prop =efi_get_configuration_table(&rt_prop_guid);ret = efi_set_variable_int(u"RTStorageVolatile",&efi_guid_efi_rt_var_file,EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS |EFI_VARIABLE_READ_ONLY,sizeof(EFI_VAR_FILE_NAME),EFI_VAR_FILE_NAME, false);if (ret != EFI_SUCCESS)rt_prop->runtime_services_supported |= ~EFI_RT_SUPPORTED_SET_VARIABLE;elselog_err("Can't RTStorage. SetVariableRT won't be available\n");} /* Switch variable services functions to runtime version */ efi_runtime_services.get_variable = efi_get_variable_runtime; efi_runtime_services.get_next_variable_name =
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.
Some adjustments are needed to do that. Currently we discard BS-only variables in EBS(). We need to preserve those on the OS RAM backend that exposes the variables. Since BS-only variables can't appear at RT 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 to make efi_var_collect() available at runtime, in order to construct the "VarToFile" buffer with BS, RT & NV variables.
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-st...
Suggested-by:Ard Biesheuvel ardb@kernel.org # dumping all variables to a variable Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- include/efi_variable.h | 15 +++- lib/efi_loader/efi_boottime.c | 2 + lib/efi_loader/efi_var_common.c | 43 +++++------ lib/efi_loader/efi_var_file.c | 1 - lib/efi_loader/efi_var_mem.c | 90 ++++++++++------------- lib/efi_loader/efi_variable.c | 118 ++++++++++++++++++++++++------ lib/efi_loader/efi_variable_tee.c | 1 - 7 files changed, 164 insertions(+), 106 deletions(-)
diff --git a/include/efi_variable.h b/include/efi_variable.h index 42a2b7c52bef..8963339b9bb6 100644 --- a/include/efi_variable.h +++ b/include/efi_variable.h @@ -271,13 +271,15 @@ 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 * @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 +291,14 @@ 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 * 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 +338,11 @@ efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size, */ void efi_var_buf_update(struct efi_var_file *var_buf);
+/** + * efi_prealloced_rt_memory() - Get a pointer to preallocated EFI memory + * available at runtime + * + * Return: pointer to preallocated runtime usable buffer + */ +void __efi_runtime *efi_prealloced_rt_memory(void); #endif diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 1951291747cd..39481c89a688 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -97,6 +97,8 @@ const efi_guid_t efi_guid_load_file_protocol = EFI_LOAD_FILE_PROTOCOL_GUID; const efi_guid_t efi_guid_load_file2_protocol = EFI_LOAD_FILE2_PROTOCOL_GUID; /* GUID of the SMBIOS table */ const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID; +/* used by special U-Boot variables during SetVariableRT */ +const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID;
static efi_status_t EFIAPI efi_disconnect_controller( efi_handle_t controller_handle, diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c index 07b9603d49f3..4abc90e411e7 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); }
/** @@ -427,18 +429,15 @@ void *efi_get_var(const u16 *name, const efi_guid_t *vendor, efi_uintn_t *size) * * Return: Status code */ -efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *lenp, - u32 check_attr_mask) +efi_status_t __efi_runtime +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; + buf = (struct efi_var_file *)efi_prealloced_rt_memory(); var = buf->var; old_var = var; for (;;) { @@ -451,32 +450,26 @@ efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t * 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); + efi_memcpy_runtime(var->name, old_var->name, old_var_name_length); + efi_memcpy_runtime(&var->guid, &old_var->guid, sizeof(efi_guid_t)); + ret = efi_get_next_variable_name_mem(&var_name_length, var->name, + &var->guid, check_attr_mask); if (ret == EFI_NOT_FOUND) break; - if (ret != EFI_SUCCESS) { - free(buf); + if (ret != EFI_SUCCESS) 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, + ret = efi_get_variable_mem(var->name, &var->guid, &var->attr, &data_length, data, - &var->time); - if (ret != EFI_SUCCESS) { - free(buf); + &var->time, check_attr_mask); + if (ret != EFI_SUCCESS) 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); - } + var->length = data_length; + var = (struct efi_var_entry *)ALIGN((uintptr_t)data + data_length, 8); }
buf->reserved = 0; @@ -490,5 +483,3 @@ efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *
return EFI_SUCCESS; } - - diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c index 413e1794e88c..8614e3d34706 100644 --- a/lib/efi_loader/efi_var_file.c +++ b/lib/efi_loader/efi_var_file.c @@ -83,7 +83,6 @@ efi_status_t efi_var_to_file(void) error: if (ret != EFI_SUCCESS) log_err("Failed to persist EFI variables\n"); - free(buf); return ret; #else return EFI_SUCCESS; diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c index 6c21cec5d457..a7af0604733e 100644 --- a/lib/efi_loader/efi_var_mem.c +++ b/lib/efi_loader/efi_var_mem.c @@ -16,6 +16,7 @@ * relocation during SetVirtualAddressMap(). */ static struct efi_var_file __efi_runtime_data *efi_var_buf; +static void __efi_runtime_data *efi_rt_prealloced; static struct efi_var_entry __efi_runtime_data *efi_current_var;
/** @@ -184,53 +185,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 * @@ -241,6 +195,7 @@ static void EFIAPI __efi_runtime efi_var_mem_notify_virtual_address_map(struct efi_event *event, void *context) { efi_convert_pointer(0, (void **)&efi_var_buf); + efi_convert_pointer(0, (void **)&efi_rt_prealloced); efi_current_var = NULL; }
@@ -261,13 +216,21 @@ 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); + /* + * efi_var_collect() needs to run at runtime and provide us + * copies of variables used for the VarToFile variable. + * Preallocate memory equal to the variable storage and + * preserve it to copy variables around + */ + ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, + EFI_RUNTIME_SERVICES_DATA, + efi_size_in_pages(EFI_VAR_BUF_SIZE), + &memory); if (ret != EFI_SUCCESS) return ret; + efi_rt_prealloced = (void *)(uintptr_t)memory; + ret = efi_create_event(EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE, TPL_CALLBACK, efi_var_mem_notify_virtual_address_map, NULL, NULL, &event); @@ -279,7 +242,7 @@ efi_status_t efi_var_mem_init(void) 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,6 +254,9 @@ efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor, if (!var) return EFI_NOT_FOUND;
+ if (mask && !((var->attr & mask) == mask)) + return EFI_NOT_FOUND; + if (attributes) *attributes = var->attr; if (timep) @@ -315,7 +281,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 +291,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 +315,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; }
@@ -354,3 +327,14 @@ void efi_var_buf_update(struct efi_var_file *var_buf) { memcpy(efi_var_buf, var_buf, EFI_VAR_BUF_SIZE); } + +void __efi_runtime *efi_prealloced_rt_memory(void) +{ + char *s; + int count = EFI_VAR_BUF_SIZE; + + s = (char *)efi_rt_prealloced; + while (count--) + *s++ = 0; + return efi_rt_prealloced; +} diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index f97c8c57f75c..4f529169ea54 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -22,6 +22,8 @@ #include <u-boot/crc.h> #include <asm/sections.h>
+static const efi_guid_t __efi_runtime_data efi_guid_efi_rt_var_file = + U_BOOT_EFI_RT_VAR_FILE_GUID; #ifdef CONFIG_EFI_SECURE_BOOT
/** @@ -208,14 +210,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); }
/** @@ -479,6 +483,8 @@ if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { efi_uintn_t ret; bool append, delete; u64 time = 0; + struct efi_var_file *buf; + loff_t len;
/* * Authenticated variables are not supported the rest of the checks @@ -520,30 +526,60 @@ if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { return EFI_NOT_FOUND; }
- if (delete) { + if (!delete) { + /* + * We always insert new variabes and delete the old one when + * appending + */ + len = 2 * (u16_strlen(variable_name) + 1) + data_size + + sizeof(struct efi_var_entry); + if (var && append) + len += 2 * var->length; + /* + * We will copy the variable update into VarToFile, + * account for it twice + */ + len *= 2; + if (len > efi_var_mem_free()) + return EFI_OUT_OF_RESOURCES; + if (append && var) { + u16 *old_data = var->name; + + for (; *old_data; ++old_data) + ; + ++old_data; + ret = efi_var_mem_ins(variable_name, vendor, attributes, + var->length, old_data, data_size, + data, time); + } else { + ret = efi_var_mem_ins(variable_name, vendor, attributes, + data_size, data, 0, NULL, time); + } + } else { /* EFI_NOT_FOUND has been handled before */ attributes = var->attr; ret = EFI_SUCCESS; - } else if (append && var) { - u16 *old_data = var->name; - - for (; *old_data; ++old_data) - ; - ++old_data; - ret = efi_var_mem_ins(variable_name, vendor, attributes, - var->length, old_data, data_size, data, - time); - } else { - ret = efi_var_mem_ins(variable_name, vendor, attributes, - data_size, data, 0, NULL, time); } - if (ret != EFI_SUCCESS) return ret; /* We are always inserting new variables, get rid of the old copy */ efi_var_mem_del(var);
- return EFI_SUCCESS; + /* + * Create a volatile variable that userspace apps can dd and + * update the file contents + */ + ret = efi_var_collect(&buf, &len, EFI_VARIABLE_NON_VOLATILE); + if (ret != EFI_SUCCESS) + return ret; + var = efi_var_mem_find(&efi_guid_efi_rt_var_file, u"VarToFile", NULL); + if (var) + efi_var_mem_del(var); + + ret = efi_var_mem_ins(u"VarToFile", &efi_guid_efi_rt_var_file, + EFI_VARIABLE_RUNTIME_ACCESS, len, buf, 0, + NULL, time); + return ret; } else
return EFI_UNSUPPORTED; @@ -557,11 +593,11 @@ void efi_variables_boot_exit_notify(void) const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID; const efi_guid_t rt_prop_guid = EFI_RT_PROPERTIES_TABLE_GUID; efi_status_t ret; + struct efi_var_file *buf; + loff_t len; + bool fail = false;
if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { - struct efi_rt_properties_table *rt_prop = - efi_get_configuration_table(&rt_prop_guid); - ret = efi_set_variable_int(u"RTStorageVolatile", &efi_guid_efi_rt_var_file, EFI_VARIABLE_BOOTSERVICE_ACCESS | @@ -569,11 +605,47 @@ void efi_variables_boot_exit_notify(void) EFI_VARIABLE_READ_ONLY, sizeof(EFI_VAR_FILE_NAME), EFI_VAR_FILE_NAME, false); + if (ret != EFI_SUCCESS) { + fail = true; + goto out; + } + + ret = efi_var_collect(&buf, &len, EFI_VARIABLE_NON_VOLATILE); + if (ret != EFI_SUCCESS) { + fail = true; + goto out; + } + + ret = efi_set_variable_int(u"VarToFile", + &efi_guid_efi_rt_var_file, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, + len, + buf, false); if (ret != EFI_SUCCESS) - rt_prop->runtime_services_supported |= ~EFI_RT_SUPPORTED_SET_VARIABLE; - else - log_err("Can't RTStorage. SetVariableRT won't be available\n"); + fail = true; +out: + if (fail) { + 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, 0, + false); + efi_set_variable_int(u"VarToFile", + &efi_guid_efi_rt_var_file, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, 0, 0, + false); + } else { + struct efi_rt_properties_table *rt_prop = + efi_get_configuration_table(&rt_prop_guid); + + rt_prop->runtime_services_supported |= + EFI_RT_SUPPORTED_SET_VARIABLE; + } } + /* Switch variable services functions to runtime version */ efi_runtime_services.get_variable = efi_get_variable_runtime; efi_runtime_services.get_next_variable_name = diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index dde135fd9f81..9d0e270591ea 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -969,7 +969,6 @@ void efi_variables_boot_exit_notify(void) log_err("Can't populate EFI variables. No runtime variables will be available\n"); else efi_var_buf_update(var_buf); - free(var_buf);
/* Update runtime service table */ efi_runtime_services.query_variable_info = -- 2.37.2
On 4/6/24 16:01, 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.
Some adjustments are needed to do that. Currently we discard BS-only variables in EBS(). We need to preserve those on the OS RAM backend that exposes the variables. Since BS-only variables can't appear at RT 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 to make efi_var_collect() available at runtime, in order to construct the "VarToFile" buffer with BS, RT & NV variables.
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-st...
This change actually doubles the amount of memory needed to store a variable at runtime. How do you reflect this in QueryVariableInfo()?
Suggested-by:Ard Biesheuvel ardb@kernel.org # dumping all variables to a variable Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
include/efi_variable.h | 15 +++- lib/efi_loader/efi_boottime.c | 2 + lib/efi_loader/efi_var_common.c | 43 +++++------ lib/efi_loader/efi_var_file.c | 1 - lib/efi_loader/efi_var_mem.c | 90 ++++++++++------------- lib/efi_loader/efi_variable.c | 118 ++++++++++++++++++++++++------ lib/efi_loader/efi_variable_tee.c | 1 - 7 files changed, 164 insertions(+), 106 deletions(-)
diff --git a/include/efi_variable.h b/include/efi_variable.h index 42a2b7c52bef..8963339b9bb6 100644 --- a/include/efi_variable.h +++ b/include/efi_variable.h @@ -271,13 +271,15 @@ 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.
*/ efi_status_t __efi_runtime efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, u16 *variable_name,
variables are only collected if all of the required- @vendor: vendor's guid
- Return: status code
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 +291,14 @@ 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
This line looks incomplete.
- 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 +338,11 @@ efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size, */ void efi_var_buf_update(struct efi_var_file *var_buf);
+/**
- efi_prealloced_rt_memory() - Get a pointer to preallocated EFI memory
available at runtime
- Return: pointer to preallocated runtime usable buffer
- */
+void __efi_runtime *efi_prealloced_rt_memory(void); #endif diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 1951291747cd..39481c89a688 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -97,6 +97,8 @@ const efi_guid_t efi_guid_load_file_protocol = EFI_LOAD_FILE_PROTOCOL_GUID; const efi_guid_t efi_guid_load_file2_protocol = EFI_LOAD_FILE2_PROTOCOL_GUID; /* GUID of the SMBIOS table */ const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID; +/* used by special U-Boot variables during SetVariableRT */ +const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID;
static efi_status_t EFIAPI efi_disconnect_controller( efi_handle_t controller_handle, diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c index 07b9603d49f3..4abc90e411e7 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);}
/**
@@ -427,18 +429,15 @@ void *efi_get_var(const u16 *name, const efi_guid_t *vendor, efi_uintn_t *size)
- Return: Status code
*/ -efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *lenp,
u32 check_attr_mask)+efi_status_t __efi_runtime +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;
- buf = (struct efi_var_file *)efi_prealloced_rt_memory(); var = buf->var; old_var = var; for (;;) {
@@ -451,32 +450,26 @@ efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t * 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);
efi_memcpy_runtime(var->name, old_var->name, old_var_name_length);efi_memcpy_runtime(&var->guid, &old_var->guid, sizeof(efi_guid_t));ret = efi_get_next_variable_name_mem(&var_name_length, var->name,&var->guid, check_attr_mask);
if (ret != EFI_SUCCESS) {free(buf);
if (ret != EFI_SUCCESS) 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,
ret = efi_get_variable_mem(var->name, &var->guid, &var->attr, &data_length, data,
&var->time);if (ret != EFI_SUCCESS) {free(buf);
&var->time, check_attr_mask);if (ret != EFI_SUCCESS) 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);}
var->length = data_length;var = (struct efi_var_entry *)ALIGN((uintptr_t)data + data_length, 8);}
buf->reserved = 0;
@@ -490,5 +483,3 @@ efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *
return EFI_SUCCESS; }
diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c index 413e1794e88c..8614e3d34706 100644 --- a/lib/efi_loader/efi_var_file.c +++ b/lib/efi_loader/efi_var_file.c @@ -83,7 +83,6 @@ efi_status_t efi_var_to_file(void) error: if (ret != EFI_SUCCESS) log_err("Failed to persist EFI variables\n");
- free(buf); return ret; #else return EFI_SUCCESS;
diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c index 6c21cec5d457..a7af0604733e 100644 --- a/lib/efi_loader/efi_var_mem.c +++ b/lib/efi_loader/efi_var_mem.c @@ -16,6 +16,7 @@
- relocation during SetVirtualAddressMap().
*/ static struct efi_var_file __efi_runtime_data *efi_var_buf; +static void __efi_runtime_data *efi_rt_prealloced; static struct efi_var_entry __efi_runtime_data *efi_current_var;
/** @@ -184,53 +185,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
@@ -241,6 +195,7 @@ static void EFIAPI __efi_runtime efi_var_mem_notify_virtual_address_map(struct efi_event *event, void *context) { efi_convert_pointer(0, (void **)&efi_var_buf);
- efi_convert_pointer(0, (void **)&efi_rt_prealloced); efi_current_var = NULL; }
@@ -261,13 +216,21 @@ 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);
- /*
* efi_var_collect() needs to run at runtime and provide us* copies of variables used for the VarToFile variable.* Preallocate memory equal to the variable storage and* preserve it to copy variables around*/- ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
EFI_RUNTIME_SERVICES_DATA,efi_size_in_pages(EFI_VAR_BUF_SIZE),&memory);- efi_rt_prealloced = (void *)(uintptr_t)memory;
- ret = efi_create_event(EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE, TPL_CALLBACK, efi_var_mem_notify_virtual_address_map, NULL, NULL, &event);
@@ -279,7 +242,7 @@ efi_status_t efi_var_mem_init(void) 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)@@ -291,6 +254,9 @@ efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor, if (!var) return EFI_NOT_FOUND;
- if (mask && !((var->attr & mask) == mask))
return EFI_NOT_FOUND;- if (attributes) *attributes = var->attr; if (timep)
@@ -315,7 +281,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)@@ -324,6 +291,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 +315,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; }
@@ -354,3 +327,14 @@ void efi_var_buf_update(struct efi_var_file *var_buf) { memcpy(efi_var_buf, var_buf, EFI_VAR_BUF_SIZE); }
+void __efi_runtime *efi_prealloced_rt_memory(void) +{
- char *s;
- int count = EFI_VAR_BUF_SIZE;
- s = (char *)efi_rt_prealloced;
- while (count--)
*s++ = 0;- return efi_rt_prealloced;
+} diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index f97c8c57f75c..4f529169ea54 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -22,6 +22,8 @@ #include <u-boot/crc.h> #include <asm/sections.h>
+static const efi_guid_t __efi_runtime_data efi_guid_efi_rt_var_file =
U_BOOT_EFI_RT_VAR_FILE_GUID;#ifdef CONFIG_EFI_SECURE_BOOT
/**
@@ -208,14 +210,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);}
/**
@@ -479,6 +483,8 @@ if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { efi_uintn_t ret; bool append, delete; u64 time = 0;
struct efi_var_file *buf;
loff_t len;
/*
- Authenticated variables are not supported the rest of the checks
@@ -520,30 +526,60 @@ if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { return EFI_NOT_FOUND; }
- if (delete) {
- if (!delete) {
/** We always insert new variabes and delete the old one when* appending*/len = 2 * (u16_strlen(variable_name) + 1) + data_size +sizeof(struct efi_var_entry);if (var && append)len += 2 * var->length;/** We will copy the variable update into VarToFile,* account for it twice*/len *= 2;if (len > efi_var_mem_free())return EFI_OUT_OF_RESOURCES;if (append && var) {u16 *old_data = var->name;for (; *old_data; ++old_data);++old_data;ret = efi_var_mem_ins(variable_name, vendor, attributes,var->length, old_data, data_size,data, time);} else {ret = efi_var_mem_ins(variable_name, vendor, attributes,data_size, data, 0, NULL, time);}- } else { /* EFI_NOT_FOUND has been handled before */ attributes = var->attr; ret = EFI_SUCCESS;
} else if (append && var) {
u16 *old_data = var->name;for (; *old_data; ++old_data);++old_data;ret = efi_var_mem_ins(variable_name, vendor, attributes,var->length, old_data, data_size, data,time);} else {
ret = efi_var_mem_ins(variable_name, vendor, attributes,data_size, data, 0, NULL, time);}
if (ret != EFI_SUCCESS) return ret; /* We are always inserting new variables, get rid of the old copy */ efi_var_mem_del(var);
return EFI_SUCCESS;
/*
* Create a volatile variable that userspace apps can dd and* update the file contents*/ret = efi_var_collect(&buf, &len, EFI_VARIABLE_NON_VOLATILE);
if (ret != EFI_SUCCESS)
return ret;var = efi_var_mem_find(&efi_guid_efi_rt_var_file, u"VarToFile", NULL);
if (var)
efi_var_mem_del(var);ret = efi_var_mem_ins(u"VarToFile", &efi_guid_efi_rt_var_file,
EFI_VARIABLE_RUNTIME_ACCESS, len, buf, 0,NULL, time);return ret; } else
return EFI_UNSUPPORTED;
@@ -557,11 +593,11 @@ void efi_variables_boot_exit_notify(void) const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID; const efi_guid_t rt_prop_guid = EFI_RT_PROPERTIES_TABLE_GUID; efi_status_t ret;
struct efi_var_file *buf;
loff_t len;
bool fail = false;
if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
struct efi_rt_properties_table *rt_prop =efi_get_configuration_table(&rt_prop_guid);- ret = efi_set_variable_int(u"RTStorageVolatile", &efi_guid_efi_rt_var_file, EFI_VARIABLE_BOOTSERVICE_ACCESS |
@@ -569,11 +605,47 @@ void efi_variables_boot_exit_notify(void) EFI_VARIABLE_READ_ONLY, sizeof(EFI_VAR_FILE_NAME), EFI_VAR_FILE_NAME, false);
if (ret != EFI_SUCCESS) {fail = true;goto out;}ret = efi_var_collect(&buf, &len, EFI_VARIABLE_NON_VOLATILE);if (ret != EFI_SUCCESS) {fail = true;goto out;}ret = efi_set_variable_int(u"VarToFile",&efi_guid_efi_rt_var_file,EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS,len,buf, false);
rt_prop->runtime_services_supported |= ~EFI_RT_SUPPORTED_SET_VARIABLE;elselog_err("Can't RTStorage. SetVariableRT won't be available\n");
fail = true;+out:
if (fail) {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, 0,false);
Not providing VarToFile because the memory buffer is more than half filled before ExitBootServices() is unexpected behavior. This needs rework.
It is not necessary to create VarToFile in the internal memory buffer. You could generate it when the user calls GetVariable() in the user provided buffer.
Best regards
Heinrich
efi_set_variable_int(u"VarToFile",&efi_guid_efi_rt_var_file,EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS, 0, 0,false);} else {struct efi_rt_properties_table *rt_prop =efi_get_configuration_table(&rt_prop_guid);rt_prop->runtime_services_supported |=EFI_RT_SUPPORTED_SET_VARIABLE;}- /* Switch variable services functions to runtime version */ efi_runtime_services.get_variable = efi_get_variable_runtime; efi_runtime_services.get_next_variable_name =
diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index dde135fd9f81..9d0e270591ea 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -969,7 +969,6 @@ void efi_variables_boot_exit_notify(void) log_err("Can't populate EFI variables. No runtime variables will be available\n"); else efi_var_buf_update(var_buf);
free(var_buf);
/* Update runtime service table */ efi_runtime_services.query_variable_info =
-- 2.37.2
Hi Heinrich
On Mon, 8 Apr 2024 at 09:41, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 4/6/24 16:01, 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.
Some adjustments are needed to do that. Currently we discard BS-only variables in EBS(). We need to preserve those on the OS RAM backend that exposes the variables. Since BS-only variables can't appear at RT 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 to make efi_var_collect() available at runtime, in order to construct the "VarToFile" buffer with BS, RT & NV variables.
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-st...
This change actually doubles the amount of memory needed to store a variable at runtime. How do you reflect this in QueryVariableInfo()?
It doesn't double it. Only BS, RT, NV variables are added to VarToFile. QueryVariableInfo isn't supported at runtime, but with the current code we have at boot time the VarToFile would be included in the calculations.
Suggested-by:Ard Biesheuvel ardb@kernel.org # dumping all variables to a variable Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
include/efi_variable.h | 15 +++- lib/efi_loader/efi_boottime.c | 2 + lib/efi_loader/efi_var_common.c | 43 +++++------ lib/efi_loader/efi_var_file.c | 1 - lib/efi_loader/efi_var_mem.c | 90 ++++++++++------------- lib/efi_loader/efi_variable.c | 118 ++++++++++++++++++++++++------
[...]
- @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 requiredThis line looks incomplete.
Yes, fixed
- 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);/**
[...]
return EFI_UNSUPPORTED;@@ -557,11 +593,11 @@ void efi_variables_boot_exit_notify(void) const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID; const efi_guid_t rt_prop_guid = EFI_RT_PROPERTIES_TABLE_GUID; efi_status_t ret;
struct efi_var_file *buf;loff_t len;bool fail = false; if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
struct efi_rt_properties_table *rt_prop =efi_get_configuration_table(&rt_prop_guid);ret = efi_set_variable_int(u"RTStorageVolatile", &efi_guid_efi_rt_var_file, EFI_VARIABLE_BOOTSERVICE_ACCESS |@@ -569,11 +605,47 @@ void efi_variables_boot_exit_notify(void) EFI_VARIABLE_READ_ONLY, sizeof(EFI_VAR_FILE_NAME), EFI_VAR_FILE_NAME, false);
if (ret != EFI_SUCCESS) {fail = true;goto out;}ret = efi_var_collect(&buf, &len, EFI_VARIABLE_NON_VOLATILE);if (ret != EFI_SUCCESS) {fail = true;goto out;}ret = efi_set_variable_int(u"VarToFile",&efi_guid_efi_rt_var_file,EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS,len,buf, false); if (ret != EFI_SUCCESS)
rt_prop->runtime_services_supported |= ~EFI_RT_SUPPORTED_SET_VARIABLE;elselog_err("Can't RTStorage. SetVariableRT won't be available\n");
fail = true;+out:
if (fail) {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, 0,false);Not providing VarToFile because the memory buffer is more than half filled before ExitBootServices() is unexpected behavior. This needs rework.
Why? We should add the SetVariable at runtime to the documentation.
It is not necessary to create VarToFile in the internal memory buffer. You could generate it when the user calls GetVariable() in the user provided buffer.
Yes, but that would make some functions look a bit messier, since we have to inject code specifically looking for VarToFile. OTOH it would make SetVariable at runtime easier to read since all the size calculations and variable creation would go away. But in any case, we have to add the variable at runtime (perhaps with a value of 0?) -- users need to know it's there to be able to read it. But if you are fine with the special conditions in GetVariable at runtime, I can easily move the logic to GetVariable.
Thanks /Ilias
Best regards
Heinrich
efi_set_variable_int(u"VarToFile",&efi_guid_efi_rt_var_file,EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS, 0, 0,false);} else {struct efi_rt_properties_table *rt_prop =efi_get_configuration_table(&rt_prop_guid);rt_prop->runtime_services_supported |=EFI_RT_SUPPORTED_SET_VARIABLE;} }/* Switch variable services functions to runtime version */ efi_runtime_services.get_variable = efi_get_variable_runtime; efi_runtime_services.get_next_variable_name =diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index dde135fd9f81..9d0e270591ea 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -969,7 +969,6 @@ void efi_variables_boot_exit_notify(void) log_err("Can't populate EFI variables. No runtime variables will be available\n"); else efi_var_buf_update(var_buf);
free(var_buf); /* Update runtime service table */ efi_runtime_services.query_variable_info =-- 2.37.2
On 4/8/24 10:12, Ilias Apalodimas wrote:
Hi Heinrich
On Mon, 8 Apr 2024 at 09:41, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 4/6/24 16:01, 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.
Some adjustments are needed to do that. Currently we discard BS-only variables in EBS(). We need to preserve those on the OS RAM backend that exposes the variables. Since BS-only variables can't appear at RT 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 to make efi_var_collect() available at runtime, in order to construct the "VarToFile" buffer with BS, RT & NV variables.
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-st...
This change actually doubles the amount of memory needed to store a variable at runtime. How do you reflect this in QueryVariableInfo()?
It doesn't double it. Only BS, RT, NV variables are added to VarToFile. QueryVariableInfo isn't supported at runtime, but with the current code we have at boot time the VarToFile would be included in the calculations.
VarToFile does not exist at boot time. Nothing stops the user from writing NV variables to fill up the memory buffer. VarToFile will be as large as the sum of NV variables and will not fit into the buffer when efi_variables_boot_exit_notify() is invoked.
Suggested-by:Ard Biesheuvel ardb@kernel.org # dumping all variables to a variable Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
include/efi_variable.h | 15 +++- lib/efi_loader/efi_boottime.c | 2 + lib/efi_loader/efi_var_common.c | 43 +++++------ lib/efi_loader/efi_var_file.c | 1 - lib/efi_loader/efi_var_mem.c | 90 ++++++++++------------- lib/efi_loader/efi_variable.c | 118 ++++++++++++++++++++++++------
[...]
* @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 requiredThis line looks incomplete.
Yes, fixed
* 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);/**
[...]
return EFI_UNSUPPORTED;@@ -557,11 +593,11 @@ void efi_variables_boot_exit_notify(void) const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID; const efi_guid_t rt_prop_guid = EFI_RT_PROPERTIES_TABLE_GUID; efi_status_t ret;
struct efi_var_file *buf;loff_t len;bool fail = false; if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
struct efi_rt_properties_table *rt_prop =efi_get_configuration_table(&rt_prop_guid);ret = efi_set_variable_int(u"RTStorageVolatile", &efi_guid_efi_rt_var_file, EFI_VARIABLE_BOOTSERVICE_ACCESS |@@ -569,11 +605,47 @@ void efi_variables_boot_exit_notify(void) EFI_VARIABLE_READ_ONLY, sizeof(EFI_VAR_FILE_NAME), EFI_VAR_FILE_NAME, false);
if (ret != EFI_SUCCESS) {fail = true;goto out;}ret = efi_var_collect(&buf, &len, EFI_VARIABLE_NON_VOLATILE);if (ret != EFI_SUCCESS) {fail = true;goto out;}ret = efi_set_variable_int(u"VarToFile",&efi_guid_efi_rt_var_file,EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS,len,buf, false); if (ret != EFI_SUCCESS)
rt_prop->runtime_services_supported |= ~EFI_RT_SUPPORTED_SET_VARIABLE;elselog_err("Can't RTStorage. SetVariableRT won't be available\n");
fail = true;+out:
if (fail) {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, 0,false);Not providing VarToFile because the memory buffer is more than half filled before ExitBootServices() is unexpected behavior. This needs rework.
Why? We should add the SetVariable at runtime to the documentation.
Depending on the number of variables existing at boot time support you will be creating VarToFile or not. I can't see that users will fancy this.
Best regards
Heinrich
It is not necessary to create VarToFile in the internal memory buffer. You could generate it when the user calls GetVariable() in the user provided buffer.
Yes, but that would make some functions look a bit messier, since we have to inject code specifically looking for VarToFile. OTOH it would make SetVariable at runtime easier to read since all the size calculations and variable creation would go away. But in any case, we have to add the variable at runtime (perhaps with a value of 0?) -- users need to know it's there to be able to read it. But if you are fine with the special conditions in GetVariable at runtime, I can easily move the logic to GetVariable.
Thanks /Ilias
Best regards
Heinrich
efi_set_variable_int(u"VarToFile",&efi_guid_efi_rt_var_file,EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS, 0, 0,false);} else {struct efi_rt_properties_table *rt_prop =efi_get_configuration_table(&rt_prop_guid);rt_prop->runtime_services_supported |=EFI_RT_SUPPORTED_SET_VARIABLE;} }/* Switch variable services functions to runtime version */ efi_runtime_services.get_variable = efi_get_variable_runtime; efi_runtime_services.get_next_variable_name =diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index dde135fd9f81..9d0e270591ea 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -969,7 +969,6 @@ void efi_variables_boot_exit_notify(void) log_err("Can't populate EFI variables. No runtime variables will be available\n"); else efi_var_buf_update(var_buf);
free(var_buf); /* Update runtime service table */ efi_runtime_services.query_variable_info =-- 2.37.2
On Mon, 8 Apr 2024 at 11:29, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 4/8/24 10:12, Ilias Apalodimas wrote:
Hi Heinrich
On Mon, 8 Apr 2024 at 09:41, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 4/6/24 16:01, 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.
Some adjustments are needed to do that. Currently we discard BS-only variables in EBS(). We need to preserve those on the OS RAM backend that exposes the variables. Since BS-only variables can't appear at RT 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 to make efi_var_collect() available at runtime, in order to construct the "VarToFile" buffer with BS, RT & NV variables.
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-st...
This change actually doubles the amount of memory needed to store a variable at runtime. How do you reflect this in QueryVariableInfo()?
It doesn't double it. Only BS, RT, NV variables are added to VarToFile. QueryVariableInfo isn't supported at runtime, but with the current code we have at boot time the VarToFile would be included in the calculations.
VarToFile does not exist at boot time. Nothing stops the user from writing NV variables to fill up the memory buffer. VarToFile will be as large as the sum of NV variables and will not fit into the buffer when efi_variables_boot_exit_notify() is invoked.
Yes, tbh when I was initially writing this, I was allocating 2x the variable size buffer to get away from this limitation. But I eventually decided variables for embedded would be small enough to not suffer from this that much. Anyway, I can either allocate 2x size and adjust some code size checks or move the variable filling in GetVariable as you suggested earlier. Any preference?
Thanks /Ilias
Suggested-by:Ard Biesheuvel ardb@kernel.org # dumping all variables to a variable Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
include/efi_variable.h | 15 +++- lib/efi_loader/efi_boottime.c | 2 + lib/efi_loader/efi_var_common.c | 43 +++++------ lib/efi_loader/efi_var_file.c | 1 - lib/efi_loader/efi_var_mem.c | 90 ++++++++++------------- lib/efi_loader/efi_variable.c | 118 ++++++++++++++++++++++++------
[...]
* @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 requiredThis line looks incomplete.
Yes, fixed
* 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);/**
[...]
return EFI_UNSUPPORTED;@@ -557,11 +593,11 @@ void efi_variables_boot_exit_notify(void) const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID; const efi_guid_t rt_prop_guid = EFI_RT_PROPERTIES_TABLE_GUID; efi_status_t ret;
struct efi_var_file *buf;loff_t len;bool fail = false; if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
struct efi_rt_properties_table *rt_prop =efi_get_configuration_table(&rt_prop_guid);ret = efi_set_variable_int(u"RTStorageVolatile", &efi_guid_efi_rt_var_file, EFI_VARIABLE_BOOTSERVICE_ACCESS |@@ -569,11 +605,47 @@ void efi_variables_boot_exit_notify(void) EFI_VARIABLE_READ_ONLY, sizeof(EFI_VAR_FILE_NAME), EFI_VAR_FILE_NAME, false);
if (ret != EFI_SUCCESS) {fail = true;goto out;}ret = efi_var_collect(&buf, &len, EFI_VARIABLE_NON_VOLATILE);if (ret != EFI_SUCCESS) {fail = true;goto out;}ret = efi_set_variable_int(u"VarToFile",&efi_guid_efi_rt_var_file,EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS,len,buf, false); if (ret != EFI_SUCCESS)
rt_prop->runtime_services_supported |= ~EFI_RT_SUPPORTED_SET_VARIABLE;elselog_err("Can't RTStorage. SetVariableRT won't be available\n");
fail = true;+out:
if (fail) {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, 0,false);Not providing VarToFile because the memory buffer is more than half filled before ExitBootServices() is unexpected behavior. This needs rework.
Why? We should add the SetVariable at runtime to the documentation.
Depending on the number of variables existing at boot time support you will be creating VarToFile or not. I can't see that users will fancy this.
Best regards
Heinrich
It is not necessary to create VarToFile in the internal memory buffer. You could generate it when the user calls GetVariable() in the user provided buffer.
Yes, but that would make some functions look a bit messier, since we have to inject code specifically looking for VarToFile. OTOH it would make SetVariable at runtime easier to read since all the size calculations and variable creation would go away. But in any case, we have to add the variable at runtime (perhaps with a value of 0?) -- users need to know it's there to be able to read it. But if you are fine with the special conditions in GetVariable at runtime, I can easily move the logic to GetVariable.
Thanks /Ilias
Best regards
Heinrich
efi_set_variable_int(u"VarToFile",&efi_guid_efi_rt_var_file,EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS, 0, 0,false);} else {struct efi_rt_properties_table *rt_prop =efi_get_configuration_table(&rt_prop_guid);rt_prop->runtime_services_supported |=EFI_RT_SUPPORTED_SET_VARIABLE;} }/* Switch variable services functions to runtime version */ efi_runtime_services.get_variable = efi_get_variable_runtime; efi_runtime_services.get_next_variable_name =diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index dde135fd9f81..9d0e270591ea 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -969,7 +969,6 @@ void efi_variables_boot_exit_notify(void) log_err("Can't populate EFI variables. No runtime variables will be available\n"); else efi_var_buf_update(var_buf);
free(var_buf); /* Update runtime service table */ efi_runtime_services.query_variable_info =-- 2.37.2
Since we support SetVariableRT now add the relevant tests
- Search for the RTStorageVolatile and VarToFile variables after EBS - Try to update with invalid variales (BS, RT only) - Try to write a variable bigger than our backend storage - Write a variable that fits and check VarToFile has been updated correclty - Append to the variable and check VarToFile changes - Try to delete VarToFile which is write protected
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- .../efi_selftest_variables_runtime.c | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c index 4c9405c0a7c7..eb4820e7deab 100644 --- a/lib/efi_selftest/efi_selftest_variables_runtime.c +++ b/lib/efi_selftest/efi_selftest_variables_runtime.c @@ -10,6 +10,7 @@ */
#include <efi_selftest.h> +#include <efi_variable.h>
#define EFI_ST_MAX_DATA_SIZE 16 #define EFI_ST_MAX_VARNAME_SIZE 40 @@ -17,6 +18,8 @@ static struct efi_boot_services *boottime; static struct efi_runtime_services *runtime; static const efi_guid_t guid_vendor0 = EFI_GLOBAL_VARIABLE_GUID; +static const efi_guid_t __efi_runtime_data efi_rt_var_guid = + U_BOOT_EFI_RT_VAR_FILE_GUID;
/* * Setup unit test. @@ -45,11 +48,14 @@ static int execute(void) u32 attr; u8 v[16] = {0x5d, 0xd1, 0x5e, 0x51, 0x5a, 0x05, 0xc7, 0x0c, 0x35, 0x4a, 0xae, 0x87, 0xa5, 0xdf, 0x0f, 0x65,}; + u8 v2[CONFIG_EFI_VAR_BUF_SIZE]; u8 data[EFI_ST_MAX_DATA_SIZE]; + u8 data2[CONFIG_EFI_VAR_BUF_SIZE]; u16 varname[EFI_ST_MAX_VARNAME_SIZE]; efi_guid_t guid; u64 max_storage, rem_storage, max_size;
+ memset(v2, 0x1, sizeof(v2)); ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, &max_storage, &rem_storage, &max_size); @@ -63,10 +69,107 @@ static int execute(void) EFI_VARIABLE_RUNTIME_ACCESS, 3, v + 4); if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { + efi_uintn_t prev_len, delta; + if (ret != EFI_INVALID_PARAMETER) { efi_st_error("SetVariable failed\n"); return EFI_ST_FAILURE; } + + len = sizeof(data); + ret = runtime->get_variable(u"RTStorageVolatile", + &efi_rt_var_guid, + &attr, &len, data); + if (ret != EFI_SUCCESS) { + efi_st_error("GetVariable failed\n"); + return EFI_ST_FAILURE; + } + + len = sizeof(data2); + ret = runtime->get_variable(u"VarToFile", &efi_rt_var_guid, + &attr, &len, data2); + if (ret != EFI_SUCCESS) { + efi_st_error("GetVariable failed\n"); + return EFI_ST_FAILURE; + } + /* + * VarToFile will size must change once a variable is inserted + * Store it now, we'll use it later + */ + prev_len = len; + ret = runtime->set_variable(u"efi_st_var0", &guid_vendor0, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_NON_VOLATILE, + sizeof(v2) / 2, + v2 + (sizeof(v2) / 2)); + /* + * This will try to update VarToFile as well and must fail, + * without changing or deleting VarToFile + */ + if (ret != EFI_OUT_OF_RESOURCES) { + efi_st_error("SetVariable failed\n"); + return EFI_ST_FAILURE; + } + len = sizeof(data2); + ret = runtime->get_variable(u"VarToFile", &efi_rt_var_guid, + &attr, &len, data2); + if (ret != EFI_SUCCESS || prev_len != len) { + efi_st_error("Get/SetVariable failed\n"); + return EFI_ST_FAILURE; + } + + /* SetVariableRT updates VarToFile with efi_st_var0 */ + ret = runtime->set_variable(u"efi_st_var0", &guid_vendor0, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_NON_VOLATILE, + sizeof(v), v); + if (ret != EFI_SUCCESS) { + efi_st_error("SetVariable failed\n"); + return EFI_ST_FAILURE; + } + len = sizeof(data2); + delta = 2 * (u16_strlen(u"efi_st_var0") + 1) + sizeof(v) + + sizeof(struct efi_var_entry); + ret = runtime->get_variable(u"VarToFile", &efi_rt_var_guid, + &attr, &len, data2); + if (ret != EFI_SUCCESS || prev_len + delta != len) { + efi_st_error("Get/SetVariable failed\n"); + return EFI_ST_FAILURE; + } + + /* append on an existing variable will updateVarToFile */ + ret = runtime->set_variable(u"efi_st_var0", &guid_vendor0, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_APPEND_WRITE | + EFI_VARIABLE_NON_VOLATILE, + sizeof(v), v); + if (ret != EFI_SUCCESS) { + efi_st_error("SetVariable failed\n"); + return EFI_ST_FAILURE; + } + prev_len = len; + delta = sizeof(v); + len = sizeof(data2); + ret = runtime->get_variable(u"VarToFile", &efi_rt_var_guid, + &attr, &len, data2); + if (ret != EFI_SUCCESS || prev_len + delta != len) { + efi_st_error("Get/SetVariable failed\n"); + return EFI_ST_FAILURE; + } + + /* Variables that are BS, RT and volatile are RO after EBS */ + ret = runtime->set_variable(u"VarToFile", &efi_rt_var_guid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_NON_VOLATILE, + sizeof(v), v); + if (ret != EFI_WRITE_PROTECTED) { + efi_st_error("Get/SetVariable failed\n"); + return EFI_ST_FAILURE; + } } else { if (ret != EFI_UNSUPPORTED) { efi_st_error("SetVariable failed\n");
Hi all,
This is an updated version of [0].
When EFI variables are stored on file we don't allow SetVariableRT, since the OS doesn't know how to access or write that file. At the same time keeping the U-Boot drivers alive in runtime sections and performing writes from the firmware is dangerous -- if at all possible.
For GetVariableRT we copy runtime variables in RAM and expose them to the OS. Add a Kconfig option and provide SetVariableRT using the same memory back end. The OS will be responsible for syncing the RAM contents to the file, otherwise any changes made during runtime won't persist reboots.
It's worth noting that the variable store format is defined in EBBR [1] and authenticated variables are explicitly prohibited, since they have to be stored on a medium that's tamper and rollback protected.
The original RFC was just enabling the memory back end. This is a more complete version and we should be able, with small adjustments of userspace tools, fix SetVariableRT. If enabled the firmware will add two new RO EFI variables after ExitBootServices.
RTStorageVolatile -- contains the filename, relative to the ESP VarToFile -- an EFI variable that holds all the BS,RT, NV variables and can be copied to the file defined by RTStorageVolatile.
If any errors occur during the variable init, the firmware will delete them and adjust the RT_PROP table accordingly, disabling SetvarRT.
- pre-patch $~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (ro,nosuid,nodev,noexec,relatime)
$~ efibootmgr -n 0001 Could not set BootNext: Read-only file system
- post-patch $~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (rw,nosuid,nodev,noexec,relatime) , disabling SetvarRT. $~ efibootmgr -n 0001 BootNext: 0001 BootCurrent: 0000 BootOrder: 0000,0001 Boot0000* debian HD(1,GPT,bdae5610-3331-4e4d-9466-acb5caf0b4a6,0x800,0x100000)/File(EFI\debian\grubaa64.efi) Boot0001* virtio 0 VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,0000000000000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,850000001f000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,1600850000000000){auto_created_boot_option}
$~ efivar -p -n 8be4df61-93ca-11d2-aa0d-00e098032b8c-BootNext GUID: 8be4df61-93ca-11d2-aa0d-00e098032b8c Name: "BootNext" Attributes: Non-Volatile Boot Service Access Runtime Service Access Value: 00000000 01 00
[0] https://lore.kernel.org/u-boot/20240329071929.2461441-1-ilias.apalodimas@lin... [1] https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-st...
Changes since the RFC: - Return EFI_INVALID_PARAM if attributes are not volatile - Add EFI_WRITE_PROTECTED checks for BS, RT *only* variables - Add 2 EFI variables and allow userspace to write the file - Add selftests
Ilias Apalodimas (4): efi_loader: conditionally enable SetvariableRT efi_loader: Add OS notifications for SetVariableRT in RAM efi_loader: add an EFI variable with the variable file contents efi_selftest: add tests for setvariableRT
include/efi_loader.h | 4 + include/efi_variable.h | 15 +- lib/efi_loader/Kconfig | 16 ++ lib/efi_loader/efi_boottime.c | 2 + lib/efi_loader/efi_runtime.c | 1 + lib/efi_loader/efi_var_common.c | 43 ++-- lib/efi_loader/efi_var_file.c | 1 - lib/efi_loader/efi_var_mem.c | 90 +++----- lib/efi_loader/efi_variable.c | 210 +++++++++++++++++- lib/efi_loader/efi_variable_tee.c | 1 - .../efi_selftest_variables_runtime.c | 116 +++++++++- 11 files changed, 401 insertions(+), 98 deletions(-)
-- 2.37.2
When EFI variables are stored on file we don't allow SetVariableRT, since the OS doesn't know how to access or write that file. At the same time keeping the U-Boot drivers alive in runtime sections and performing writes from the firmware is dangerous -- if at all possible.
For GetVariableRT we copy runtime variables in RAM and expose them to the OS. Add a Kconfig option and provide SetVariableRT using the same memory backend. The OS will be responsible for syncing the RAM contents to the file, otherwise any changes made during runtime won't persist reboots.
It's worth noting that the variable store format is defined in EBBR [0] and authenticated variables are explicitly prohibited, since they have to be stored on a medium that's tamper and rollback protected.
- pre-patch $~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (ro,nosuid,nodev,noexec,relatime)
$~ efibootmgr -n 0001 Could not set BootNext: Read-only file system
- post-patch $~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (rw,nosuid,nodev,noexec,relatime)
$~ efibootmgr -n 0001 BootNext: 0001 BootCurrent: 0000 BootOrder: 0000,0001 Boot0000* debian HD(1,GPT,bdae5610-3331-4e4d-9466-acb5caf0b4a6,0x800,0x100000)/File(EFI\debian\grubaa64.efi) Boot0001* virtio 0 VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,0000000000000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,850000001f000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,1600850000000000){auto_created_boot_option}
$~ efivar -p -n 8be4df61-93ca-11d2-aa0d-00e098032b8c-BootNext GUID: 8be4df61-93ca-11d2-aa0d-00e098032b8c Name: "BootNext" Attributes: Non-Volatile Boot Service Access Runtime Service Access Value: 00000000 01 00
[0] https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-st...
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- lib/efi_loader/Kconfig | 16 +++ lib/efi_loader/efi_runtime.c | 5 + lib/efi_loader/efi_variable.c | 114 ++++++++++++++++-- .../efi_selftest_variables_runtime.c | 13 +- 4 files changed, 135 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index a7c3e05c13a0..b210ceea6d64 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -63,6 +63,22 @@ config EFI_VARIABLE_FILE_STORE Select this option if you want non-volatile UEFI variables to be stored as file /ubootefi.var on the EFI system partition.
+config EFI_RT_VOLATILE_STORE + bool "Allow variable runtime services in volatile storage (e.g RAM)" + depends on EFI_VARIABLE_FILE_STORE + help + When EFI variables are stored on file we don't allow SetVariableRT, + since the OS doesn't know how to write that file. At he same time + we copy runtime variables in DRAM and support GetVariableRT + + Enable this option to allow SetVariableRT on the RAM backend of + the EFI variable storage. The OS will be responsible for syncing + the RAM contents to the file, otherwise any changes made during + runtime won't persist reboots. + Authenticated variables are not supported. Note that this will + violate the EFI spec since writing auth variables will return + EFI_INVALID_PARAMETER + config EFI_MM_COMM_TEE bool "UEFI variables storage service via the trusted world" depends on OPTEE diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 18da6892e796..8ebbea7e5c69 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -10,6 +10,7 @@ #include <dm.h> #include <elf.h> #include <efi_loader.h> +#include <efi_variable.h> #include <log.h> #include <malloc.h> #include <rtc.h> @@ -126,6 +127,10 @@ efi_status_t efi_init_runtime_supported(void) EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP | EFI_RT_SUPPORTED_CONVERT_POINTER;
+ if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) + rt_table->runtime_services_supported |= + EFI_RT_SUPPORTED_SET_VARIABLE; + /* * This value must be synced with efi_runtime_detach_list * as well as efi_runtime_services. diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 6fe3792a12a5..f79041e6bedd 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -218,17 +218,20 @@ efi_get_next_variable_name_int(efi_uintn_t *variable_name_size, return efi_get_next_variable_name_mem(variable_name_size, variable_name, vendor); }
-efi_status_t efi_set_variable_int(const u16 *variable_name, - const efi_guid_t *vendor, - u32 attributes, efi_uintn_t data_size, - const void *data, bool ro_check) +/** + * setvariable_allowed() - checks defined by the UEFI spec for setvariable + * + * @variable_name: name of the variable + * @vendor: vendor GUID + * @attributes: attributes of the variable + * @data_size: size of the buffer with the variable value + * @data: buffer with the variable value + * Return: status code + */ +static efi_status_t __efi_runtime +setvariable_allowed(const u16 *variable_name, const efi_guid_t *vendor, + u32 attributes, efi_uintn_t data_size, const void *data) { - struct efi_var_entry *var; - efi_uintn_t ret; - bool append, delete; - u64 time = 0; - enum efi_auth_var_type var_type; - if (!variable_name || !*variable_name || !vendor) return EFI_INVALID_PARAMETER;
@@ -260,6 +263,25 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) return EFI_INVALID_PARAMETER;
+ return EFI_SUCCESS; +} + +efi_status_t efi_set_variable_int(const u16 *variable_name, + const efi_guid_t *vendor, + u32 attributes, efi_uintn_t data_size, + const void *data, bool ro_check) +{ + struct efi_var_entry *var; + efi_uintn_t ret; + bool append, delete; + u64 time = 0; + enum efi_auth_var_type var_type; + + ret = setvariable_allowed(variable_name, vendor, attributes, data_size, + data); + if (ret != EFI_SUCCESS) + return ret; + /* check if a variable exists */ var = efi_var_mem_find(vendor, variable_name, NULL); append = !!(attributes & EFI_VARIABLE_APPEND_WRITE); @@ -452,6 +474,78 @@ efi_set_variable_runtime(u16 *variable_name, const efi_guid_t *vendor, u32 attributes, efi_uintn_t data_size, const void *data) { +if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { + struct efi_var_entry *var; + efi_uintn_t ret; + bool append, delete; + u64 time = 0; + + /* + * Authenticated variables are not supported the rest of the checks + * are in setvariable_allowed() + */ + if (attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) + return EFI_INVALID_PARAMETER; + /* BS only variables are hidden deny writing them */ + if (!(attributes & EFI_VARIABLE_RUNTIME_ACCESS)) + return EFI_INVALID_PARAMETER; + + ret = setvariable_allowed(variable_name, vendor, attributes, data_size, + data); + if (ret != EFI_SUCCESS) + return ret; + + /* check if a variable exists */ + var = efi_var_mem_find(vendor, variable_name, NULL); + append = !!(attributes & EFI_VARIABLE_APPEND_WRITE); + attributes &= ~EFI_VARIABLE_APPEND_WRITE; + delete = !append && (!data_size || !attributes); + + if (var) { + if (var->attr & EFI_VARIABLE_READ_ONLY || + !(var->attr & EFI_VARIABLE_NON_VOLATILE)) + return EFI_WRITE_PROTECTED; + + /* attributes won't be changed */ + if (!delete && (((var->attr & ~EFI_VARIABLE_READ_ONLY) != + (attributes & ~EFI_VARIABLE_READ_ONLY)))) + return EFI_INVALID_PARAMETER; + time = var->time; + } else { + if (!(attributes & EFI_VARIABLE_NON_VOLATILE)) + return EFI_INVALID_PARAMETER; + if (append && !data_size) + return EFI_SUCCESS; + if (delete) + return EFI_NOT_FOUND; + } + + if (delete) { + /* EFI_NOT_FOUND has been handled before */ + attributes = var->attr; + ret = EFI_SUCCESS; + } else if (append && var) { + u16 *old_data = var->name; + + for (; *old_data; ++old_data) + ; + ++old_data; + ret = efi_var_mem_ins(variable_name, vendor, attributes, + var->length, old_data, data_size, data, + time); + } else { + ret = efi_var_mem_ins(variable_name, vendor, attributes, + data_size, data, 0, NULL, time); + } + + if (ret != EFI_SUCCESS) + return ret; + /* We are always inserting new variables, get rid of the old copy */ + efi_var_mem_del(var); + + return EFI_SUCCESS; +} else + return EFI_UNSUPPORTED; }
diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c index 4700d9424105..4c9405c0a7c7 100644 --- a/lib/efi_selftest/efi_selftest_variables_runtime.c +++ b/lib/efi_selftest/efi_selftest_variables_runtime.c @@ -62,9 +62,16 @@ static int execute(void) EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS, 3, v + 4); - if (ret != EFI_UNSUPPORTED) { - efi_st_error("SetVariable failed\n"); - return EFI_ST_FAILURE; + if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { + if (ret != EFI_INVALID_PARAMETER) { + efi_st_error("SetVariable failed\n"); + return EFI_ST_FAILURE; + } + } else { + if (ret != EFI_UNSUPPORTED) { + efi_st_error("SetVariable failed\n"); + return EFI_ST_FAILURE; + } } len = EFI_ST_MAX_DATA_SIZE; ret = runtime->get_variable(u"PlatformLangCodes", &guid_vendor0,
Previous patches enable SetVariableRT using a volatile storage backend using EFI_RUNTIME_SERVICES_DATA allocared memory. Since there's no recommendation from the spec on how to notify the OS, add a volatile EFI variable that contains the filename relative to the ESP. OS'es can use that file and update it at runtime
$~ efivar -p -n b2ac5fc9-92b7-4acd-aeac-11e818c3130c-RTStorageVolatile GUID: b2ac5fc9-92b7-4acd-aeac-11e818c3130c Name: "RTStorageVolatile" Attributes: Boot Service Access Runtime Service Access Value: 00000000 75 62 6f 6f 74 65 66 69 2e 76 61 72 00 |ubootefi.var. |
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- include/efi_loader.h | 4 ++++ lib/efi_loader/efi_runtime.c | 4 ---- lib/efi_loader/efi_variable.c | 20 ++++++++++++++++++++ 3 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 7daca0afba2b..25551fe18948 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -159,6 +159,10 @@ static inline void efi_set_bootdev(const char *dev, const char *devnr, #define EFICONFIG_AUTO_GENERATED_ENTRY_GUID \ EFI_GUID(0x8108ac4e, 0x9f11, 0x4d59, \ 0x85, 0x0e, 0xe2, 0x1a, 0x52, 0x2c, 0x59, 0xb2) +#define U_BOOT_EFI_RT_VAR_FILE_GUID \ + EFI_GUID(0xb2ac5fc9, 0x92b7, 0x4acd, \ + 0xae, 0xac, 0x11, 0xe8, 0x18, 0xc3, 0x13, 0x0c) +
/* Use internal device tree when starting UEFI application */ #define EFI_FDT_USE_INTERNAL NULL diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 8ebbea7e5c69..d898ba6c268f 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -127,10 +127,6 @@ efi_status_t efi_init_runtime_supported(void) EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP | EFI_RT_SUPPORTED_CONVERT_POINTER;
- if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) - rt_table->runtime_services_supported |= - EFI_RT_SUPPORTED_SET_VARIABLE; - /* * This value must be synced with efi_runtime_detach_list * as well as efi_runtime_services. diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index f79041e6bedd..f97c8c57f75c 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -554,6 +554,26 @@ if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { */ void efi_variables_boot_exit_notify(void) { + const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID; + const efi_guid_t rt_prop_guid = EFI_RT_PROPERTIES_TABLE_GUID; + efi_status_t ret; + + if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { + struct efi_rt_properties_table *rt_prop = + efi_get_configuration_table(&rt_prop_guid); + + ret = efi_set_variable_int(u"RTStorageVolatile", + &efi_guid_efi_rt_var_file, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_READ_ONLY, + sizeof(EFI_VAR_FILE_NAME), + EFI_VAR_FILE_NAME, false); + if (ret != EFI_SUCCESS) + rt_prop->runtime_services_supported |= ~EFI_RT_SUPPORTED_SET_VARIABLE; + else + log_err("Can't RTStorage. SetVariableRT won't be available\n"); + } /* Switch variable services functions to runtime version */ efi_runtime_services.get_variable = efi_get_variable_runtime; efi_runtime_services.get_next_variable_name =
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.
Some adjustments are needed to do that. Currently we discard BS-only variables in EBS(). We need to preserve those on the OS RAM backend that exposes the variables. Since BS-only variables can't appear at RT 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 to make efi_var_collect() available at runtime, in order to construct the "VarToFile" buffer with BS, RT & NV variables.
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-st...
Suggested-by:Ard Biesheuvel ardb@kernel.org # dumping all variables to a variable Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- include/efi_variable.h | 15 +++- lib/efi_loader/efi_boottime.c | 2 + lib/efi_loader/efi_var_common.c | 43 +++++------ lib/efi_loader/efi_var_file.c | 1 - lib/efi_loader/efi_var_mem.c | 90 ++++++++++------------- lib/efi_loader/efi_variable.c | 118 ++++++++++++++++++++++++------ lib/efi_loader/efi_variable_tee.c | 1 - 7 files changed, 164 insertions(+), 106 deletions(-)
diff --git a/include/efi_variable.h b/include/efi_variable.h index 42a2b7c52bef..8963339b9bb6 100644 --- a/include/efi_variable.h +++ b/include/efi_variable.h @@ -271,13 +271,15 @@ 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 * @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 +291,14 @@ 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 * 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 +338,11 @@ efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size, */ void efi_var_buf_update(struct efi_var_file *var_buf);
+/** + * efi_prealloced_rt_memory() - Get a pointer to preallocated EFI memory + * available at runtime + * + * Return: pointer to preallocated runtime usable buffer + */ +void __efi_runtime *efi_prealloced_rt_memory(void); #endif diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 1951291747cd..39481c89a688 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -97,6 +97,8 @@ const efi_guid_t efi_guid_load_file_protocol = EFI_LOAD_FILE_PROTOCOL_GUID; const efi_guid_t efi_guid_load_file2_protocol = EFI_LOAD_FILE2_PROTOCOL_GUID; /* GUID of the SMBIOS table */ const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID; +/* used by special U-Boot variables during SetVariableRT */ +const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID;
static efi_status_t EFIAPI efi_disconnect_controller( efi_handle_t controller_handle, diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c index 07b9603d49f3..4abc90e411e7 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); }
/** @@ -427,18 +429,15 @@ void *efi_get_var(const u16 *name, const efi_guid_t *vendor, efi_uintn_t *size) * * Return: Status code */ -efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *lenp, - u32 check_attr_mask) +efi_status_t __efi_runtime +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; + buf = (struct efi_var_file *)efi_prealloced_rt_memory(); var = buf->var; old_var = var; for (;;) { @@ -451,32 +450,26 @@ efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t * 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); + efi_memcpy_runtime(var->name, old_var->name, old_var_name_length); + efi_memcpy_runtime(&var->guid, &old_var->guid, sizeof(efi_guid_t)); + ret = efi_get_next_variable_name_mem(&var_name_length, var->name, + &var->guid, check_attr_mask); if (ret == EFI_NOT_FOUND) break; - if (ret != EFI_SUCCESS) { - free(buf); + if (ret != EFI_SUCCESS) 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, + ret = efi_get_variable_mem(var->name, &var->guid, &var->attr, &data_length, data, - &var->time); - if (ret != EFI_SUCCESS) { - free(buf); + &var->time, check_attr_mask); + if (ret != EFI_SUCCESS) 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); - } + var->length = data_length; + var = (struct efi_var_entry *)ALIGN((uintptr_t)data + data_length, 8); }
buf->reserved = 0; @@ -490,5 +483,3 @@ efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *
return EFI_SUCCESS; } - - diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c index 413e1794e88c..8614e3d34706 100644 --- a/lib/efi_loader/efi_var_file.c +++ b/lib/efi_loader/efi_var_file.c @@ -83,7 +83,6 @@ efi_status_t efi_var_to_file(void) error: if (ret != EFI_SUCCESS) log_err("Failed to persist EFI variables\n"); - free(buf); return ret; #else return EFI_SUCCESS; diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c index 6c21cec5d457..a7af0604733e 100644 --- a/lib/efi_loader/efi_var_mem.c +++ b/lib/efi_loader/efi_var_mem.c @@ -16,6 +16,7 @@ * relocation during SetVirtualAddressMap(). */ static struct efi_var_file __efi_runtime_data *efi_var_buf; +static void __efi_runtime_data *efi_rt_prealloced; static struct efi_var_entry __efi_runtime_data *efi_current_var;
/** @@ -184,53 +185,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 * @@ -241,6 +195,7 @@ static void EFIAPI __efi_runtime efi_var_mem_notify_virtual_address_map(struct efi_event *event, void *context) { efi_convert_pointer(0, (void **)&efi_var_buf); + efi_convert_pointer(0, (void **)&efi_rt_prealloced); efi_current_var = NULL; }
@@ -261,13 +216,21 @@ 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); + /* + * efi_var_collect() needs to run at runtime and provide us + * copies of variables used for the VarToFile variable. + * Preallocate memory equal to the variable storage and + * preserve it to copy variables around + */ + ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, + EFI_RUNTIME_SERVICES_DATA, + efi_size_in_pages(EFI_VAR_BUF_SIZE), + &memory); if (ret != EFI_SUCCESS) return ret; + efi_rt_prealloced = (void *)(uintptr_t)memory; + ret = efi_create_event(EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE, TPL_CALLBACK, efi_var_mem_notify_virtual_address_map, NULL, NULL, &event); @@ -279,7 +242,7 @@ efi_status_t efi_var_mem_init(void) 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,6 +254,9 @@ efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor, if (!var) return EFI_NOT_FOUND;
+ if (mask && !((var->attr & mask) == mask)) + return EFI_NOT_FOUND; + if (attributes) *attributes = var->attr; if (timep) @@ -315,7 +281,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 +291,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 +315,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; }
@@ -354,3 +327,14 @@ void efi_var_buf_update(struct efi_var_file *var_buf) { memcpy(efi_var_buf, var_buf, EFI_VAR_BUF_SIZE); } + +void __efi_runtime *efi_prealloced_rt_memory(void) +{ + char *s; + int count = EFI_VAR_BUF_SIZE; + + s = (char *)efi_rt_prealloced; + while (count--) + *s++ = 0; + return efi_rt_prealloced; +} diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index f97c8c57f75c..4f529169ea54 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -22,6 +22,8 @@ #include <u-boot/crc.h> #include <asm/sections.h>
+static const efi_guid_t __efi_runtime_data efi_guid_efi_rt_var_file = + U_BOOT_EFI_RT_VAR_FILE_GUID; #ifdef CONFIG_EFI_SECURE_BOOT
/** @@ -208,14 +210,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); }
/** @@ -479,6 +483,8 @@ if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { efi_uintn_t ret; bool append, delete; u64 time = 0; + struct efi_var_file *buf; + loff_t len;
/* * Authenticated variables are not supported the rest of the checks @@ -520,30 +526,60 @@ if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { return EFI_NOT_FOUND; }
- if (delete) { + if (!delete) { + /* + * We always insert new variabes and delete the old one when + * appending + */ + len = 2 * (u16_strlen(variable_name) + 1) + data_size + + sizeof(struct efi_var_entry); + if (var && append) + len += 2 * var->length; + /* + * We will copy the variable update into VarToFile, + * account for it twice + */ + len *= 2; + if (len > efi_var_mem_free()) + return EFI_OUT_OF_RESOURCES; + if (append && var) { + u16 *old_data = var->name; + + for (; *old_data; ++old_data) + ; + ++old_data; + ret = efi_var_mem_ins(variable_name, vendor, attributes, + var->length, old_data, data_size, + data, time); + } else { + ret = efi_var_mem_ins(variable_name, vendor, attributes, + data_size, data, 0, NULL, time); + } + } else { /* EFI_NOT_FOUND has been handled before */ attributes = var->attr; ret = EFI_SUCCESS; - } else if (append && var) { - u16 *old_data = var->name; - - for (; *old_data; ++old_data) - ; - ++old_data; - ret = efi_var_mem_ins(variable_name, vendor, attributes, - var->length, old_data, data_size, data, - time); - } else { - ret = efi_var_mem_ins(variable_name, vendor, attributes, - data_size, data, 0, NULL, time); } - if (ret != EFI_SUCCESS) return ret; /* We are always inserting new variables, get rid of the old copy */ efi_var_mem_del(var);
- return EFI_SUCCESS; + /* + * Create a volatile variable that userspace apps can dd and + * update the file contents + */ + ret = efi_var_collect(&buf, &len, EFI_VARIABLE_NON_VOLATILE); + if (ret != EFI_SUCCESS) + return ret; + var = efi_var_mem_find(&efi_guid_efi_rt_var_file, u"VarToFile", NULL); + if (var) + efi_var_mem_del(var); + + ret = efi_var_mem_ins(u"VarToFile", &efi_guid_efi_rt_var_file, + EFI_VARIABLE_RUNTIME_ACCESS, len, buf, 0, + NULL, time); + return ret; } else
return EFI_UNSUPPORTED; @@ -557,11 +593,11 @@ void efi_variables_boot_exit_notify(void) const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID; const efi_guid_t rt_prop_guid = EFI_RT_PROPERTIES_TABLE_GUID; efi_status_t ret; + struct efi_var_file *buf; + loff_t len; + bool fail = false;
if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { - struct efi_rt_properties_table *rt_prop = - efi_get_configuration_table(&rt_prop_guid); - ret = efi_set_variable_int(u"RTStorageVolatile", &efi_guid_efi_rt_var_file, EFI_VARIABLE_BOOTSERVICE_ACCESS | @@ -569,11 +605,47 @@ void efi_variables_boot_exit_notify(void) EFI_VARIABLE_READ_ONLY, sizeof(EFI_VAR_FILE_NAME), EFI_VAR_FILE_NAME, false); + if (ret != EFI_SUCCESS) { + fail = true; + goto out; + } + + ret = efi_var_collect(&buf, &len, EFI_VARIABLE_NON_VOLATILE); + if (ret != EFI_SUCCESS) { + fail = true; + goto out; + } + + ret = efi_set_variable_int(u"VarToFile", + &efi_guid_efi_rt_var_file, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, + len, + buf, false); if (ret != EFI_SUCCESS) - rt_prop->runtime_services_supported |= ~EFI_RT_SUPPORTED_SET_VARIABLE; - else - log_err("Can't RTStorage. SetVariableRT won't be available\n"); + fail = true; +out: + if (fail) { + 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, 0, + false); + efi_set_variable_int(u"VarToFile", + &efi_guid_efi_rt_var_file, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, 0, 0, + false); + } else { + struct efi_rt_properties_table *rt_prop = + efi_get_configuration_table(&rt_prop_guid); + + rt_prop->runtime_services_supported |= + EFI_RT_SUPPORTED_SET_VARIABLE; + } } + /* Switch variable services functions to runtime version */ efi_runtime_services.get_variable = efi_get_variable_runtime; efi_runtime_services.get_next_variable_name = diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index dde135fd9f81..9d0e270591ea 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -969,7 +969,6 @@ void efi_variables_boot_exit_notify(void) log_err("Can't populate EFI variables. No runtime variables will be available\n"); else efi_var_buf_update(var_buf); - free(var_buf);
/* Update runtime service table */ efi_runtime_services.query_variable_info = -- 2.37.2
Since we support SetVariableRT now add the relevant tests
- Search for the RTStorageVolatile and VarToFile variables after EBS - Try to update with invalid variales (BS, RT only) - Try to write a variable bigger than our backend storage - Write a variable that fits and check VarToFile has been updated correclty - Append to the variable and check VarToFile changes - Try to delete VarToFile which is write protected
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- .../efi_selftest_variables_runtime.c | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c index 4c9405c0a7c7..eb4820e7deab 100644 --- a/lib/efi_selftest/efi_selftest_variables_runtime.c +++ b/lib/efi_selftest/efi_selftest_variables_runtime.c @@ -10,6 +10,7 @@ */
#include <efi_selftest.h> +#include <efi_variable.h>
#define EFI_ST_MAX_DATA_SIZE 16 #define EFI_ST_MAX_VARNAME_SIZE 40 @@ -17,6 +18,8 @@ static struct efi_boot_services *boottime; static struct efi_runtime_services *runtime; static const efi_guid_t guid_vendor0 = EFI_GLOBAL_VARIABLE_GUID; +static const efi_guid_t __efi_runtime_data efi_rt_var_guid = + U_BOOT_EFI_RT_VAR_FILE_GUID;
/* * Setup unit test. @@ -45,11 +48,14 @@ static int execute(void) u32 attr; u8 v[16] = {0x5d, 0xd1, 0x5e, 0x51, 0x5a, 0x05, 0xc7, 0x0c, 0x35, 0x4a, 0xae, 0x87, 0xa5, 0xdf, 0x0f, 0x65,}; + u8 v2[CONFIG_EFI_VAR_BUF_SIZE]; u8 data[EFI_ST_MAX_DATA_SIZE]; + u8 data2[CONFIG_EFI_VAR_BUF_SIZE]; u16 varname[EFI_ST_MAX_VARNAME_SIZE]; efi_guid_t guid; u64 max_storage, rem_storage, max_size;
+ memset(v2, 0x1, sizeof(v2)); ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, &max_storage, &rem_storage, &max_size); @@ -63,10 +69,107 @@ static int execute(void) EFI_VARIABLE_RUNTIME_ACCESS, 3, v + 4); if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { + efi_uintn_t prev_len, delta; + if (ret != EFI_INVALID_PARAMETER) { efi_st_error("SetVariable failed\n"); return EFI_ST_FAILURE; } + + len = sizeof(data); + ret = runtime->get_variable(u"RTStorageVolatile", + &efi_rt_var_guid, + &attr, &len, data); + if (ret != EFI_SUCCESS) { + efi_st_error("GetVariable failed\n"); + return EFI_ST_FAILURE; + } + + len = sizeof(data2); + ret = runtime->get_variable(u"VarToFile", &efi_rt_var_guid, + &attr, &len, data2); + if (ret != EFI_SUCCESS) { + efi_st_error("GetVariable failed\n"); + return EFI_ST_FAILURE; + } + /* + * VarToFile will size must change once a variable is inserted + * Store it now, we'll use it later + */ + prev_len = len; + ret = runtime->set_variable(u"efi_st_var0", &guid_vendor0, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_NON_VOLATILE, + sizeof(v2) / 2, + v2 + (sizeof(v2) / 2)); + /* + * This will try to update VarToFile as well and must fail, + * without changing or deleting VarToFile + */ + if (ret != EFI_OUT_OF_RESOURCES) { + efi_st_error("SetVariable failed\n"); + return EFI_ST_FAILURE; + } + len = sizeof(data2); + ret = runtime->get_variable(u"VarToFile", &efi_rt_var_guid, + &attr, &len, data2); + if (ret != EFI_SUCCESS || prev_len != len) { + efi_st_error("Get/SetVariable failed\n"); + return EFI_ST_FAILURE; + } + + /* SetVariableRT updates VarToFile with efi_st_var0 */ + ret = runtime->set_variable(u"efi_st_var0", &guid_vendor0, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_NON_VOLATILE, + sizeof(v), v); + if (ret != EFI_SUCCESS) { + efi_st_error("SetVariable failed\n"); + return EFI_ST_FAILURE; + } + len = sizeof(data2); + delta = 2 * (u16_strlen(u"efi_st_var0") + 1) + sizeof(v) + + sizeof(struct efi_var_entry); + ret = runtime->get_variable(u"VarToFile", &efi_rt_var_guid, + &attr, &len, data2); + if (ret != EFI_SUCCESS || prev_len + delta != len) { + efi_st_error("Get/SetVariable failed\n"); + return EFI_ST_FAILURE; + } + + /* append on an existing variable will updateVarToFile */ + ret = runtime->set_variable(u"efi_st_var0", &guid_vendor0, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_APPEND_WRITE | + EFI_VARIABLE_NON_VOLATILE, + sizeof(v), v); + if (ret != EFI_SUCCESS) { + efi_st_error("SetVariable failed\n"); + return EFI_ST_FAILURE; + } + prev_len = len; + delta = sizeof(v); + len = sizeof(data2); + ret = runtime->get_variable(u"VarToFile", &efi_rt_var_guid, + &attr, &len, data2); + if (ret != EFI_SUCCESS || prev_len + delta != len) { + efi_st_error("Get/SetVariable failed\n"); + return EFI_ST_FAILURE; + } + + /* Variables that are BS, RT and volatile are RO after EBS */ + ret = runtime->set_variable(u"VarToFile", &efi_rt_var_guid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_NON_VOLATILE, + sizeof(v), v); + if (ret != EFI_WRITE_PROTECTED) { + efi_st_error("Get/SetVariable failed\n"); + return EFI_ST_FAILURE; + } } else { if (ret != EFI_UNSUPPORTED) { efi_st_error("SetVariable failed\n");
From: Ilias Apalodimas ilias.apalodimas@linaro.org
Hi all,
This is an updated version of [0].
When EFI variables are stored on file we don't allow SetVariableRT, since the OS doesn't know how to access or write that file. At the same time keeping the U-Boot drivers alive in runtime sections and performing writes from the firmware is dangerous -- if at all possible.
For GetVariableRT we copy runtime variables in RAM and expose them to the OS. Add a Kconfig option and provide SetVariableRT using the same memory back end. The OS will be responsible for syncing the RAM contents to the file, otherwise any changes made during runtime won't persist reboots.
It's worth noting that the variable store format is defined in EBBR [1] and authenticated variables are explicitly prohibited, since they have to be stored on a medium that's tamper and rollback protected.
The original RFC was just enabling the memory back end. This is a more complete version and we should be able, with small adjustments of userspace tools, fix SetVariableRT. If enabled the firmware will add two new RO EFI variables after ExitBootServices.
RTStorageVolatile -- contains the filename, relative to the ESP VarToFile -- an EFI variable that holds all the BS,RT, NV variables and can be copied to the file defined by RTStorageVolatile.
If any errors occur during the variable init, the firmware will delete them and adjust the RT_PROP table accordingly, disabling SetvarRT.
- pre-patch
$~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (ro,nosuid,nodev,noexec,relatime)
$~ efibootmgr -n 0001 Could not set BootNext: Read-only file system
- post-patch
$~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (rw,nosuid,nodev,noexec,relatime) , disabling SetvarRT. $~ efibootmgr -n 0001 BootNext: 0001 BootCurrent: 0000 BootOrder: 0000,0001 Boot0000* debian HD(1,GPT,bdae5610-3331-4e4d-9466-acb5caf0b4a6,0x800,0x100000)/File(EFI\debian\grubaa64.efi) Boot0001* virtio 0 VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,0000000000000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,850000001f000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,1600850000000000){auto_created_boot_option}
$~ efivar -p -n 8be4df61-93ca-11d2-aa0d-00e098032b8c-BootNext GUID: 8be4df61-93ca-11d2-aa0d-00e098032b8c Name: "BootNext" Attributes: Non-Volatile Boot Service Access Runtime Service Access Value: 00000000 01 00
[0] https://lore.kernel.org/u-boot/20240329071929.2461441-1-ilias.apalodimas@lin... [1] https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-st...
Changes since the RFC:
- Return EFI_INVALID_PARAM if attributes are not volatile
- Add EFI_WRITE_PROTECTED checks for BS, RT *only* variables
- Add 2 EFI variables and allow userspace to write the file
- Add selftests
Ilias Apalodimas (4): efi_loader: conditionally enable SetvariableRT efi_loader: Add OS notifications for SetVariableRT in RAM efi_loader: add an EFI variable with the variable file contents efi_selftest: add tests for setvariableRT
include/efi_loader.h | 4 + include/efi_variable.h | 15 +- lib/efi_loader/Kconfig | 16 ++ lib/efi_loader/efi_boottime.c | 2 + lib/efi_loader/efi_runtime.c | 1 + lib/efi_loader/efi_var_common.c | 43 ++-- lib/efi_loader/efi_var_file.c | 1 - lib/efi_loader/efi_var_mem.c | 90 +++----- lib/efi_loader/efi_variable.c | 210 +++++++++++++++++- lib/efi_loader/efi_variable_tee.c | 1 - .../efi_selftest_variables_runtime.c | 116 +++++++++- 11 files changed, 401 insertions(+), 98 deletions(-)
I can't get patch 0003 from this series to apply on master or next.
Hi Mark
On Mon, 8 Apr 2024 at 00:53, Mark Kettenis mark.kettenis@xs4all.nl wrote:
From: Ilias Apalodimas ilias.apalodimas@linaro.org
Hi all,
This is an updated version of [0].
When EFI variables are stored on file we don't allow SetVariableRT, since the OS doesn't know how to access or write that file. At the same time keeping the U-Boot drivers alive in runtime sections and performing writes from the firmware is dangerous -- if at all possible.
For GetVariableRT we copy runtime variables in RAM and expose them to the OS. Add a Kconfig option and provide SetVariableRT using the same memory back end. The OS will be responsible for syncing the RAM contents to the file, otherwise any changes made during runtime won't persist reboots.
It's worth noting that the variable store format is defined in EBBR [1] and authenticated variables are explicitly prohibited, since they have to be stored on a medium that's tamper and rollback protected.
The original RFC was just enabling the memory back end. This is a more complete version and we should be able, with small adjustments of userspace tools, fix SetVariableRT. If enabled the firmware will add two new RO EFI variables after ExitBootServices.
RTStorageVolatile -- contains the filename, relative to the ESP VarToFile -- an EFI variable that holds all the BS,RT, NV variables and can be copied to the file defined by RTStorageVolatile.
If any errors occur during the variable init, the firmware will delete them and adjust the RT_PROP table accordingly, disabling SetvarRT.
- pre-patch
$~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (ro,nosuid,nodev,noexec,relatime)
$~ efibootmgr -n 0001 Could not set BootNext: Read-only file system
- post-patch
$~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (rw,nosuid,nodev,noexec,relatime) , disabling SetvarRT. $~ efibootmgr -n 0001 BootNext: 0001 BootCurrent: 0000 BootOrder: 0000,0001 Boot0000* debian HD(1,GPT,bdae5610-3331-4e4d-9466-acb5caf0b4a6,0x800,0x100000)/File(EFI\debian\grubaa64.efi) Boot0001* virtio 0 VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,0000000000000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,850000001f000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,1600850000000000){auto_created_boot_option}
$~ efivar -p -n 8be4df61-93ca-11d2-aa0d-00e098032b8c-BootNext GUID: 8be4df61-93ca-11d2-aa0d-00e098032b8c Name: "BootNext" Attributes: Non-Volatile Boot Service Access Runtime Service Access Value: 00000000 01 00
[0] https://lore.kernel.org/u-boot/20240329071929.2461441-1-ilias.apalodimas@lin... [1] https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-st...
Changes since the RFC:
- Return EFI_INVALID_PARAM if attributes are not volatile
- Add EFI_WRITE_PROTECTED checks for BS, RT *only* variables
- Add 2 EFI variables and allow userspace to write the file
- Add selftests
Ilias Apalodimas (4): efi_loader: conditionally enable SetvariableRT efi_loader: Add OS notifications for SetVariableRT in RAM efi_loader: add an EFI variable with the variable file contents efi_selftest: add tests for setvariableRT
include/efi_loader.h | 4 + include/efi_variable.h | 15 +- lib/efi_loader/Kconfig | 16 ++ lib/efi_loader/efi_boottime.c | 2 + lib/efi_loader/efi_runtime.c | 1 + lib/efi_loader/efi_var_common.c | 43 ++-- lib/efi_loader/efi_var_file.c | 1 - lib/efi_loader/efi_var_mem.c | 90 +++----- lib/efi_loader/efi_variable.c | 210 +++++++++++++++++- lib/efi_loader/efi_variable_tee.c | 1 - .../efi_selftest_variables_runtime.c | 116 +++++++++- 11 files changed, 401 insertions(+), 98 deletions(-)
I can't get patch 0003 from this series to apply on master or next.
Yes, my bad I forgot to mention you also need (before applying this series) https://lore.kernel.org/u-boot/20240403153335.96971-1-heinrich.schuchardt@ca... https://lore.kernel.org/u-boot/20240402090950.3819705-1-kojima.masahisa@soci... https://lore.kernel.org/u-boot/20240405065058.591452-1-ilias.apalodimas@lina...
Alternatively, you can clone directly https://source.denx.de/u-boot/custodians/u-boot-tpm/-/tree/setvar_rt1
Thanks /Ilias
On 4/7/24 23:53, Mark Kettenis wrote:
From: Ilias Apalodimas ilias.apalodimas@linaro.org
Hi all,
This is an updated version of [0].
When EFI variables are stored on file we don't allow SetVariableRT, since the OS doesn't know how to access or write that file. At the same time keeping the U-Boot drivers alive in runtime sections and performing writes from the firmware is dangerous -- if at all possible.
For GetVariableRT we copy runtime variables in RAM and expose them to the OS. Add a Kconfig option and provide SetVariableRT using the same memory back end. The OS will be responsible for syncing the RAM contents to the file, otherwise any changes made during runtime won't persist reboots.
It's worth noting that the variable store format is defined in EBBR [1] and authenticated variables are explicitly prohibited, since they have to be stored on a medium that's tamper and rollback protected.
The original RFC was just enabling the memory back end. This is a more complete version and we should be able, with small adjustments of userspace tools, fix SetVariableRT. If enabled the firmware will add two new RO EFI variables after ExitBootServices.
RTStorageVolatile -- contains the filename, relative to the ESP VarToFile -- an EFI variable that holds all the BS,RT, NV variables and can be copied to the file defined by RTStorageVolatile.
If any errors occur during the variable init, the firmware will delete them and adjust the RT_PROP table accordingly, disabling SetvarRT.
- pre-patch
$~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (ro,nosuid,nodev,noexec,relatime)
$~ efibootmgr -n 0001 Could not set BootNext: Read-only file system
- post-patch
$~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (rw,nosuid,nodev,noexec,relatime) , disabling SetvarRT. $~ efibootmgr -n 0001 BootNext: 0001 BootCurrent: 0000 BootOrder: 0000,0001 Boot0000* debian HD(1,GPT,bdae5610-3331-4e4d-9466-acb5caf0b4a6,0x800,0x100000)/File(EFI\debian\grubaa64.efi) Boot0001* virtio 0 VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,0000000000000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,850000001f000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,1600850000000000){auto_created_boot_option}
$~ efivar -p -n 8be4df61-93ca-11d2-aa0d-00e098032b8c-BootNext GUID: 8be4df61-93ca-11d2-aa0d-00e098032b8c Name: "BootNext" Attributes: Non-Volatile Boot Service Access Runtime Service Access Value: 00000000 01 00
[0] https://lore.kernel.org/u-boot/20240329071929.2461441-1-ilias.apalodimas@lin... [1] https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-st...
Changes since the RFC:
- Return EFI_INVALID_PARAM if attributes are not volatile
- Add EFI_WRITE_PROTECTED checks for BS, RT *only* variables
- Add 2 EFI variables and allow userspace to write the file
- Add selftests
Ilias Apalodimas (4): efi_loader: conditionally enable SetvariableRT efi_loader: Add OS notifications for SetVariableRT in RAM efi_loader: add an EFI variable with the variable file contents efi_selftest: add tests for setvariableRT
include/efi_loader.h | 4 + include/efi_variable.h | 15 +- lib/efi_loader/Kconfig | 16 ++ lib/efi_loader/efi_boottime.c | 2 + lib/efi_loader/efi_runtime.c | 1 + lib/efi_loader/efi_var_common.c | 43 ++-- lib/efi_loader/efi_var_file.c | 1 - lib/efi_loader/efi_var_mem.c | 90 +++----- lib/efi_loader/efi_variable.c | 210 +++++++++++++++++- lib/efi_loader/efi_variable_tee.c | 1 - .../efi_selftest_variables_runtime.c | 116 +++++++++- 11 files changed, 401 insertions(+), 98 deletions(-)
I can't get patch 0003 from this series to apply on master or next.
Hello Mark,
there are some prerequisite pending patches:
See https://source.denx.de/u-boot/custodians/u-boot-tpm/-/commits/setvar_rt1?ref...
Best regards
Heinrich
From: Ilias Apalodimas ilias.apalodimas@linaro.org Date: Sat, 6 Apr 2024 17:01:51 +0300
Hi all,
Hi Ilias,
This is an updated version of [0].
When EFI variables are stored on file we don't allow SetVariableRT, since the OS doesn't know how to access or write that file. At the same time keeping the U-Boot drivers alive in runtime sections and performing writes from the firmware is dangerous -- if at all possible.
For GetVariableRT we copy runtime variables in RAM and expose them to the OS. Add a Kconfig option and provide SetVariableRT using the same memory back end. The OS will be responsible for syncing the RAM contents to the file, otherwise any changes made during runtime won't persist reboots.
It's worth noting that the variable store format is defined in EBBR [1] and authenticated variables are explicitly prohibited, since they have to be stored on a medium that's tamper and rollback protected.
The original RFC was just enabling the memory back end. This is a more complete version and we should be able, with small adjustments of userspace tools, fix SetVariableRT. If enabled the firmware will add two new RO EFI variables after ExitBootServices.
RTStorageVolatile -- contains the filename, relative to the ESP VarToFile -- an EFI variable that holds all the BS,RT, NV variables and can be copied to the file defined by RTStorageVolatile.
How does the OS know what the right ESP is? The system might have multiple disks that each have an ESP and more than one of those ESPs might contain an ubootefi.var file. Maybe this should be a full EFI device path that includes the partition?
Also, is RTStorageVolutile the right name for this? This is about storing Non-Volatile variables after all...
If any errors occur during the variable init, the firmware will delete them and adjust the RT_PROP table accordingly, disabling SetvarRT.
- pre-patch
$~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (ro,nosuid,nodev,noexec,relatime)
$~ efibootmgr -n 0001 Could not set BootNext: Read-only file system
- post-patch
$~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (rw,nosuid,nodev,noexec,relatime) , disabling SetvarRT. $~ efibootmgr -n 0001 BootNext: 0001 BootCurrent: 0000 BootOrder: 0000,0001 Boot0000* debian HD(1,GPT,bdae5610-3331-4e4d-9466-acb5caf0b4a6,0x800,0x100000)/File(EFI\debian\grubaa64.efi) Boot0001* virtio 0 VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,0000000000000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,850000001f000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,1600850000000000){auto_created_boot_option}
$~ efivar -p -n 8be4df61-93ca-11d2-aa0d-00e098032b8c-BootNext GUID: 8be4df61-93ca-11d2-aa0d-00e098032b8c Name: "BootNext" Attributes: Non-Volatile Boot Service Access Runtime Service Access Value: 00000000 01 00
[0] https://lore.kernel.org/u-boot/20240329071929.2461441-1-ilias.apalodimas@lin... [1] https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-st...
Changes since the RFC:
- Return EFI_INVALID_PARAM if attributes are not volatile
- Add EFI_WRITE_PROTECTED checks for BS, RT *only* variables
- Add 2 EFI variables and allow userspace to write the file
- Add selftests
Ilias Apalodimas (4): efi_loader: conditionally enable SetvariableRT efi_loader: Add OS notifications for SetVariableRT in RAM efi_loader: add an EFI variable with the variable file contents efi_selftest: add tests for setvariableRT
include/efi_loader.h | 4 + include/efi_variable.h | 15 +- lib/efi_loader/Kconfig | 16 ++ lib/efi_loader/efi_boottime.c | 2 + lib/efi_loader/efi_runtime.c | 1 + lib/efi_loader/efi_var_common.c | 43 ++-- lib/efi_loader/efi_var_file.c | 1 - lib/efi_loader/efi_var_mem.c | 90 +++----- lib/efi_loader/efi_variable.c | 210 +++++++++++++++++- lib/efi_loader/efi_variable_tee.c | 1 - .../efi_selftest_variables_runtime.c | 116 +++++++++- 11 files changed, 401 insertions(+), 98 deletions(-)
-- 2.37.2
Hi Mark ,
I am on a conference, not checking emails too much
On Mon, 8 Apr 2024 at 16:16, Mark Kettenis mark.kettenis@xs4all.nl wrote:
From: Ilias Apalodimas ilias.apalodimas@linaro.org Date: Sat, 6 Apr 2024 17:01:51 +0300
Hi all,
Hi Ilias,
This is an updated version of [0].
When EFI variables are stored on file we don't allow SetVariableRT, since the OS doesn't know how to access or write that file. At the same time keeping the U-Boot drivers alive in runtime sections and performing writes from the firmware is dangerous -- if at all possible.
For GetVariableRT we copy runtime variables in RAM and expose them to the OS. Add a Kconfig option and provide SetVariableRT using the same memory back end. The OS will be responsible for syncing the RAM contents to the file, otherwise any changes made during runtime won't persist reboots.
It's worth noting that the variable store format is defined in EBBR [1] and authenticated variables are explicitly prohibited, since they have to be stored on a medium that's tamper and rollback protected.
The original RFC was just enabling the memory back end. This is a more complete version and we should be able, with small adjustments of userspace tools, fix SetVariableRT. If enabled the firmware will add two new RO EFI variables after ExitBootServices.
RTStorageVolatile -- contains the filename, relative to the ESP VarToFile -- an EFI variable that holds all the BS,RT, NV variables and can be copied to the file defined by RTStorageVolatile.
How does the OS know what the right ESP is? The system might have multiple disks that each have an ESP and more than one of those ESPs might contain an ubootefi.var file. Maybe this should be a full EFI device path that includes the partition?
This delegates the problem no? How does the firmware know what ESP the OS will need, so it can inject the proper DP? Usually in Linux, each OS version will know and mount the ESP it needs using a UUID defined during the installation, so the name relative to ESP seemed better.
In any case, this was a concern of mine as well on the RFC and I did say we can alternatively use a full device path, but it will make the userspace app harder. But as I said we will need a mechanism that the firmware understands which OS pairs with each ESP.
Also, is RTStorageVolutile the right name for this? This is about storing Non-Volatile variables after all...
It's supposed to mean "Variable storage is volatile, here's the filename you can use to persist them", but suggestions are always welcome!
Thanks /Ilias
If any errors occur during the variable init, the firmware will delete them and adjust the RT_PROP table accordingly, disabling SetvarRT.
- pre-patch
$~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (ro,nosuid,nodev,noexec,relatime)
$~ efibootmgr -n 0001 Could not set BootNext: Read-only file system
- post-patch
$~ mount | grep efiva efivarfs on /sys/firmware/efi/efivars type efivarfs (rw,nosuid,nodev,noexec,relatime) , disabling SetvarRT. $~ efibootmgr -n 0001 BootNext: 0001 BootCurrent: 0000 BootOrder: 0000,0001 Boot0000* debian HD(1,GPT,bdae5610-3331-4e4d-9466-acb5caf0b4a6,0x800,0x100000)/File(EFI\debian\grubaa64.efi) Boot0001* virtio 0 VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,0000000000000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,850000001f000000)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,1600850000000000){auto_created_boot_option}
$~ efivar -p -n 8be4df61-93ca-11d2-aa0d-00e098032b8c-BootNext GUID: 8be4df61-93ca-11d2-aa0d-00e098032b8c Name: "BootNext" Attributes: Non-Volatile Boot Service Access Runtime Service Access Value: 00000000 01 00
[0] https://lore.kernel.org/u-boot/20240329071929.2461441-1-ilias.apalodimas@lin... [1] https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-st...
Changes since the RFC:
- Return EFI_INVALID_PARAM if attributes are not volatile
- Add EFI_WRITE_PROTECTED checks for BS, RT *only* variables
- Add 2 EFI variables and allow userspace to write the file
- Add selftests
Ilias Apalodimas (4): efi_loader: conditionally enable SetvariableRT efi_loader: Add OS notifications for SetVariableRT in RAM efi_loader: add an EFI variable with the variable file contents efi_selftest: add tests for setvariableRT
include/efi_loader.h | 4 + include/efi_variable.h | 15 +- lib/efi_loader/Kconfig | 16 ++ lib/efi_loader/efi_boottime.c | 2 + lib/efi_loader/efi_runtime.c | 1 + lib/efi_loader/efi_var_common.c | 43 ++-- lib/efi_loader/efi_var_file.c | 1 - lib/efi_loader/efi_var_mem.c | 90 +++----- lib/efi_loader/efi_variable.c | 210 +++++++++++++++++- lib/efi_loader/efi_variable_tee.c | 1 - .../efi_selftest_variables_runtime.c | 116 +++++++++- 11 files changed, 401 insertions(+), 98 deletions(-)
-- 2.37.2
participants (3)
-
Heinrich Schuchardt -
Ilias Apalodimas -
Mark Kettenis