[RFC PATCH] efi_loader: conditionally enable SetvariableRT
Mark Kettenis
mark.kettenis at xs4all.nl
Fri Mar 29 13:25:21 CET 2024
> From: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> 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 <ilias.apalodimas at linaro.org>
> ---
> 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
>
>
More information about the U-Boot
mailing list