> From: Ilias Apalodimas <[email protected]> > Date: Fri, 29 Mar 2024 09:19:27 +0200 > > 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-storage > > Open questions: > Looking at the EFI spec, I can't find a documented way of notifying the OS > that the storage is volatile. I would like to send a hint to the OS about > that and I was thinking of adding a configuration table with the filename, > which U-Boot expects to be on the ESP. Alternatively we could add a device > path which would be a bit harder to parse from the userspace application, > but safer in case multiple ESPs are present.
Should there be a way for the OS to opt-in to this new behaviour? > Since I am not too familiar with how BSDs treat and expose config tables, > we could instead add a RO efi variable with identical semantics, which > would be easier to read from userspace. There is an EFIIOC_GET_TABLE ioctl that allows retreiving a table by GUID. > Signed-off-by: Ilias Apalodimas <[email protected]> > --- > lib/efi_loader/Kconfig | 14 +++ > lib/efi_loader/efi_runtime.c | 4 + > lib/efi_loader/efi_variable.c | 108 ++++++++++++++++-- > .../efi_selftest_variables_runtime.c | 13 ++- > 4 files changed, 126 insertions(+), 13 deletions(-) > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > index a7c3e05c13a0..c7f7383189e5 100644 > --- a/lib/efi_loader/Kconfig > +++ b/lib/efi_loader/Kconfig > @@ -63,6 +63,20 @@ 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 storate. 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. > + > 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..b38ce239e2d2 100644 > --- a/lib/efi_loader/efi_runtime.c > +++ b/lib/efi_loader/efi_runtime.c > @@ -126,6 +126,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 40f7a0fb10d5..3b770ff16971 100644 > --- a/lib/efi_loader/efi_variable.c > +++ b/lib/efi_loader/efi_variable.c > @@ -218,17 +218,21 @@ 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) > +/** > + * efi_check_setvariable() - 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 EFIAPI > +efi_check_setvariable(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; > > @@ -256,6 +260,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 = efi_check_setvariable(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); > @@ -434,7 +457,72 @@ 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 */ > + if (attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) > + return EFI_INVALID_PARAMETER; > + > + ret = efi_check_setvariable(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 &= ~(u32)EFI_VARIABLE_APPEND_WRITE; > + delete = !append && (!data_size || !attributes); > + > + if (var) { > + if (var->attr & EFI_VARIABLE_READ_ONLY) > + return EFI_WRITE_PROTECTED; > + > + /* attributes won't be changed */ > + if (!delete && (((var->attr & ~(u32)EFI_VARIABLE_READ_ONLY) != > + (attributes & ~(u32)EFI_VARIABLE_READ_ONLY)))) > + return EFI_INVALID_PARAMETER; > + time = var->time; > + } else { > + if (delete || append) > + /* > + * Trying to delete or to update a non-existent > + * variable. > + */ > + return EFI_NOT_FOUND; > + } > + > + if (delete) { > + /* EFI_NOT_FOUND has been handled before */ > + attributes = var->attr; > + ret = EFI_SUCCESS; > + } else if (append) { > + 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; > +#endif > } > > /** > diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c > b/lib/efi_selftest/efi_selftest_variables_runtime.c > index 4700d9424105..6001b44989c8 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_SUCCESS) { > + 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, > -- > 2.37.2 > >
