[PATCH v2 4/4] efi_selftest: add tests for setvariableRT

Heinrich Schuchardt xypron.glpk at gmx.de
Wed Apr 17 14:22:00 CEST 2024


On 17.04.24 12:19, Ilias Apalodimas wrote:
> 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 at 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..e492b50a43c2 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),
> +					    v2);
> +		/*
> +		 * 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);

It is fine to test with variable name size (24) and variable value size
(16) both being multiples of 8. But this does not cover the generic case.

> +		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);

In the file all variable are stored at 8 byte aligned positions.
I am missing ALIGN(,8) here.

> +		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;
> +		}

We should check the header fields of VarToFile (reserved, magic, length,
crc32), e.g.

struct efi_var_file *hdr = (void *)data2;
if (memcmp(hdr->magic, EFI_VAR_FILE_MAGIC, 8) ||
     len != ALIGN(hdr->length + sizeof(hdr), 8) ||
     ... )

> +
> +		/* 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);

How about just appending 1 byte when using EFI_VARIABLE_APPEND_WRITE and
checking that the delta here too?

Best regards

Heinrich

> +		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");



More information about the U-Boot mailing list