diff mbox series

[v2,1/4] efi_loader: conditionally enable SetvariableRT

Message ID 20240417101928.119115-2-ilias.apalodimas@linaro.org
State Superseded
Headers show
Series Enable SetVariable at runtime | expand

Commit Message

Ilias Apalodimas April 17, 2024, 10:19 a.m. UTC
When we store EFI variables on file we don't allow 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 GetVariable at runtime we copy runtime variables in RAM and expose them
to the OS. Add a Kconfig option and provide SetVariable at runtime 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

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 lib/efi_loader/Kconfig                        |  16 +++
 lib/efi_loader/efi_runtime.c                  |   4 +
 lib/efi_loader/efi_variable.c                 | 115 ++++++++++++++++--
 .../efi_selftest_variables_runtime.c          |  13 +-
 4 files changed, 134 insertions(+), 14 deletions(-)

Comments

Heinrich Schuchardt April 17, 2024, 12:27 p.m. UTC | #1
On 17.04.24 12:19, Ilias Apalodimas wrote:
> When we store EFI variables on file we don't allow 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 GetVariable at runtime we copy runtime variables in RAM and expose them
> to the OS. Add a Kconfig option and provide SetVariable at runtime 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
> 
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>   lib/efi_loader/Kconfig                        |  16 +++
>   lib/efi_loader/efi_runtime.c                  |   4 +
>   lib/efi_loader/efi_variable.c                 | 115 ++++++++++++++++--
>   .../efi_selftest_variables_runtime.c          |  13 +-
>   4 files changed, 134 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index e13a6f9f4c3a..cc8371a3bb4c 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -62,6 +62,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 a61c9a77b13f..dde083b09665 100644
> --- a/lib/efi_loader/efi_runtime.c
> +++ b/lib/efi_loader/efi_runtime.c
> @@ -127,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 e6c1219a11c8..abc2a3402f42 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -219,17 +219,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;
>   
> @@ -261,6 +264,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);
> @@ -454,7 +476,78 @@ efi_set_variable_runtime(u16 *variable_name, const efi_guid_t *vendor,
>   			 u32 attributes, efi_uintn_t data_size,
>   			 const void *data)
>   {
> -	return EFI_UNSUPPORTED;
> +	struct efi_var_entry *var;
> +	efi_uintn_t ret;
> +	bool append, delete;
> +	u64 time = 0;
> +
> +	if (!IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE))
> +		return EFI_UNSUPPORTED;
> +
> +	/*
> +	 * Authenticated variables are not supported. The EFI spec
> +	 * in §32.3.6 requires keys to be stored in non-volatile storage which
> +	 * is tamper and delete resistant.
> +	 * 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 = (void *)((uintptr_t)var->name +
> +			sizeof(u16) * (u16_strlen(var->name) + 1));
> +
> +		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;
>   }
>   
>   /**
> 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) {

A comment might be helpful here:

     /* At runtime only non-volatile variables may be set. */

Otherwise looks good to me.

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

> +			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,
Ilias Apalodimas April 17, 2024, 12:33 p.m. UTC | #2
On Wed, 17 Apr 2024 at 15:28, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On 17.04.24 12:19, Ilias Apalodimas wrote:
> > When we store EFI variables on file we don't allow 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 GetVariable at runtime we copy runtime variables in RAM and expose them
> > to the OS. Add a Kconfig option and provide SetVariable at runtime 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
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >   lib/efi_loader/Kconfig                        |  16 +++
> >   lib/efi_loader/efi_runtime.c                  |   4 +
> >   lib/efi_loader/efi_variable.c                 | 115 ++++++++++++++++--
> >   .../efi_selftest_variables_runtime.c          |  13 +-
> >   4 files changed, 134 insertions(+), 14 deletions(-)
> >
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index e13a6f9f4c3a..cc8371a3bb4c 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -62,6 +62,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 a61c9a77b13f..dde083b09665 100644
> > --- a/lib/efi_loader/efi_runtime.c
> > +++ b/lib/efi_loader/efi_runtime.c
> > @@ -127,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 e6c1219a11c8..abc2a3402f42 100644
> > --- a/lib/efi_loader/efi_variable.c
> > +++ b/lib/efi_loader/efi_variable.c
> > @@ -219,17 +219,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;
> >
> > @@ -261,6 +264,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);
> > @@ -454,7 +476,78 @@ efi_set_variable_runtime(u16 *variable_name, const efi_guid_t *vendor,
> >                        u32 attributes, efi_uintn_t data_size,
> >                        const void *data)
> >   {
> > -     return EFI_UNSUPPORTED;
> > +     struct efi_var_entry *var;
> > +     efi_uintn_t ret;
> > +     bool append, delete;
> > +     u64 time = 0;
> > +
> > +     if (!IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE))
> > +             return EFI_UNSUPPORTED;
> > +
> > +     /*
> > +      * Authenticated variables are not supported. The EFI spec
> > +      * in §32.3.6 requires keys to be stored in non-volatile storage which
> > +      * is tamper and delete resistant.
> > +      * 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 = (void *)((uintptr_t)var->name +
> > +                     sizeof(u16) * (u16_strlen(var->name) + 1));
> > +
> > +             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;
> >   }
> >
> >   /**
> > 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) {
>
> A comment might be helpful here:
>
>      /* At runtime only non-volatile variables may be set. */
>
> Otherwise looks good to me.
>
> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

Thanks, Heinrich!
I'll add the comment in v3

>
> > +                     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,
>
diff mbox series

Patch

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index e13a6f9f4c3a..cc8371a3bb4c 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -62,6 +62,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 a61c9a77b13f..dde083b09665 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -127,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 e6c1219a11c8..abc2a3402f42 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -219,17 +219,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;
 
@@ -261,6 +264,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);
@@ -454,7 +476,78 @@  efi_set_variable_runtime(u16 *variable_name, const efi_guid_t *vendor,
 			 u32 attributes, efi_uintn_t data_size,
 			 const void *data)
 {
-	return EFI_UNSUPPORTED;
+	struct efi_var_entry *var;
+	efi_uintn_t ret;
+	bool append, delete;
+	u64 time = 0;
+
+	if (!IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE))
+		return EFI_UNSUPPORTED;
+
+	/*
+	 * Authenticated variables are not supported. The EFI spec
+	 * in §32.3.6 requires keys to be stored in non-volatile storage which
+	 * is tamper and delete resistant.
+	 * 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 = (void *)((uintptr_t)var->name +
+			sizeof(u16) * (u16_strlen(var->name) + 1));
+
+		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;
 }
 
 /**
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,