Re: [PATCH v3 4/4] efi_selftest: add tests for setvariableRT

2024-04-20 Thread Ilias Apalodimas
Hi Heinrich,

I was about to fix and send a v4, but I see you fixed them up on the PR.
Thanks!

On Sat, 20 Apr 2024 at 10:23, Heinrich Schuchardt  wrote:
>
> On 4/18/24 14:54, 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
> > - Try to add/delete runtime variables
> > - Verify VarToFile contains a valid file format
> >
> > Signed-off-by: Ilias Apalodimas 
> > ---
> >   .../efi_selftest_variables_runtime.c  | 197 +-
> >   1 file changed, 196 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c 
> > b/lib/efi_selftest/efi_selftest_variables_runtime.c
> > index 986924b881dd..28b4e9e7afa5 100644
> > --- a/lib/efi_selftest/efi_selftest_variables_runtime.c
> > +++ b/lib/efi_selftest/efi_selftest_variables_runtime.c
> > @@ -10,6 +10,8 @@
> >*/
> >
> >   #include 
> > +#include 
> > +#include 
> >
> >   #define EFI_ST_MAX_DATA_SIZE 16
> >   #define EFI_ST_MAX_VARNAME_SIZE 40
> > @@ -17,6 +19,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.
> > @@ -41,15 +45,18 @@ static int setup(const efi_handle_t img_handle,
> >   static int execute(void)
> >   {
> >   efi_status_t ret;
> > - efi_uintn_t len;
> > + efi_uintn_t len, avail, append_len = 17;
> >   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,
> >  _storage, _storage,
> >  _size);
> > @@ -63,11 +70,199 @@ 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;
> > + struct efi_var_entry *var;
> > + struct efi_var_file *hdr;
> > +
> >   /* At runtime only non-volatile variables may be set. */
> >   if (ret != EFI_INVALID_PARAMETER) {
> >   efi_st_error("SetVariable failed\n");
> >   return EFI_ST_FAILURE;
> >   }
> > +
> > + /* runtime atttribute must be set */
> > + ret = runtime->set_variable(u"efi_st_var0", _vendor0,
> > + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > + EFI_VARIABLE_NON_VOLATILE,
> > + 3, v + 4);
> > + if (ret != EFI_INVALID_PARAMETER) {
> > + efi_st_error("SetVariable failed\n");
> > + return EFI_ST_FAILURE;
> > + }
> > +
> > + len = sizeof(data);
> > + ret = runtime->get_variable(u"RTStorageVolatile",
> > + _rt_var_guid,
> > + , , data);
> > + if (ret != EFI_SUCCESS) {
> > + efi_st_error("GetVariable failed\n");
> > + return EFI_ST_FAILURE;
> > + }
>
> We should check data against EFI_VAR_FILE_NAME.
>
> +   if (len != sizeof(EFI_VAR_FILE_NAME) ||
> +   memcmp(data, EFI_VAR_FILE_NAME,
> +  sizeof(EFI_VAR_FILE_NAME))) {
> +   data[len - 1] = 0;
> +   efi_st_error("RTStorageVolatile = %s\n", data);
> +   return EFI_ST_FAILURE;
> +   }
> +
>
> (memcmp() because we want to be able to carve out efi_selftest as a
> standalone binary in future).
>
> > +
> > + len = sizeof(data2);
> > + ret = runtime->get_variable(u"VarToFile", _rt_var_guid,
> > + , , data2);
> > + if (ret != EFI_SUCCESS) {
> > + efi_st_error("GetVariable 

Re: [PATCH v3 4/4] efi_selftest: add tests for setvariableRT

2024-04-20 Thread Heinrich Schuchardt

On 4/18/24 14:54, 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
- Try to add/delete runtime variables
- Verify VarToFile contains a valid file format

Signed-off-by: Ilias Apalodimas 
---
  .../efi_selftest_variables_runtime.c  | 197 +-
  1 file changed, 196 insertions(+), 1 deletion(-)

diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c 
b/lib/efi_selftest/efi_selftest_variables_runtime.c
index 986924b881dd..28b4e9e7afa5 100644
--- a/lib/efi_selftest/efi_selftest_variables_runtime.c
+++ b/lib/efi_selftest/efi_selftest_variables_runtime.c
@@ -10,6 +10,8 @@
   */

  #include 
+#include 
+#include 

  #define EFI_ST_MAX_DATA_SIZE 16
  #define EFI_ST_MAX_VARNAME_SIZE 40
@@ -17,6 +19,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.
@@ -41,15 +45,18 @@ static int setup(const efi_handle_t img_handle,
  static int execute(void)
  {
efi_status_t ret;
-   efi_uintn_t len;
+   efi_uintn_t len, avail, append_len = 17;
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,
   _storage, _storage,
   _size);
@@ -63,11 +70,199 @@ 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;
+   struct efi_var_entry *var;
+   struct efi_var_file *hdr;
+
/* At runtime only non-volatile variables may be set. */
if (ret != EFI_INVALID_PARAMETER) {
efi_st_error("SetVariable failed\n");
return EFI_ST_FAILURE;
}
+
+   /* runtime atttribute must be set */
+   ret = runtime->set_variable(u"efi_st_var0", _vendor0,
+   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+   EFI_VARIABLE_NON_VOLATILE,
+   3, v + 4);
+   if (ret != EFI_INVALID_PARAMETER) {
+   efi_st_error("SetVariable failed\n");
+   return EFI_ST_FAILURE;
+   }
+
+   len = sizeof(data);
+   ret = runtime->get_variable(u"RTStorageVolatile",
+   _rt_var_guid,
+   , , data);
+   if (ret != EFI_SUCCESS) {
+   efi_st_error("GetVariable failed\n");
+   return EFI_ST_FAILURE;
+   }


We should check data against EFI_VAR_FILE_NAME.

+   if (len != sizeof(EFI_VAR_FILE_NAME) ||
+   memcmp(data, EFI_VAR_FILE_NAME,
+  sizeof(EFI_VAR_FILE_NAME))) {
+   data[len - 1] = 0;
+   efi_st_error("RTStorageVolatile = %s\n", data);
+   return EFI_ST_FAILURE;
+   }
+

(memcmp() because we want to be able to carve out efi_selftest as a
standalone binary in future).


+
+   len = sizeof(data2);
+   ret = runtime->get_variable(u"VarToFile", _rt_var_guid,
+   , , data2);
+   if (ret != EFI_SUCCESS) {
+   efi_st_error("GetVariable failed\n");
+   return EFI_ST_FAILURE;
+   }
+   /*
+* VarToFile 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", _vendor0,
+   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+   

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

2024-04-18 Thread Ilias Apalodimas
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
- Try to add/delete runtime variables
- Verify VarToFile contains a valid file format

Signed-off-by: Ilias Apalodimas 
---
 .../efi_selftest_variables_runtime.c  | 197 +-
 1 file changed, 196 insertions(+), 1 deletion(-)

diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c 
b/lib/efi_selftest/efi_selftest_variables_runtime.c
index 986924b881dd..28b4e9e7afa5 100644
--- a/lib/efi_selftest/efi_selftest_variables_runtime.c
+++ b/lib/efi_selftest/efi_selftest_variables_runtime.c
@@ -10,6 +10,8 @@
  */
 
 #include 
+#include 
+#include 
 
 #define EFI_ST_MAX_DATA_SIZE 16
 #define EFI_ST_MAX_VARNAME_SIZE 40
@@ -17,6 +19,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.
@@ -41,15 +45,18 @@ static int setup(const efi_handle_t img_handle,
 static int execute(void)
 {
efi_status_t ret;
-   efi_uintn_t len;
+   efi_uintn_t len, avail, append_len = 17;
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,
   _storage, _storage,
   _size);
@@ -63,11 +70,199 @@ 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;
+   struct efi_var_entry *var;
+   struct efi_var_file *hdr;
+
/* At runtime only non-volatile variables may be set. */
if (ret != EFI_INVALID_PARAMETER) {
efi_st_error("SetVariable failed\n");
return EFI_ST_FAILURE;
}
+
+   /* runtime atttribute must be set */
+   ret = runtime->set_variable(u"efi_st_var0", _vendor0,
+   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+   EFI_VARIABLE_NON_VOLATILE,
+   3, v + 4);
+   if (ret != EFI_INVALID_PARAMETER) {
+   efi_st_error("SetVariable failed\n");
+   return EFI_ST_FAILURE;
+   }
+
+   len = sizeof(data);
+   ret = runtime->get_variable(u"RTStorageVolatile",
+   _rt_var_guid,
+   , , data);
+   if (ret != EFI_SUCCESS) {
+   efi_st_error("GetVariable failed\n");
+   return EFI_ST_FAILURE;
+   }
+
+   len = sizeof(data2);
+   ret = runtime->get_variable(u"VarToFile", _rt_var_guid,
+   , , data2);
+   if (ret != EFI_SUCCESS) {
+   efi_st_error("GetVariable failed\n");
+   return EFI_ST_FAILURE;
+   }
+   /*
+* VarToFile 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", _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;
+