On 9/6/19 8:09 AM, AKASHI Takahiro wrote:
If EFI_VARIABLE_APPEND_WRITE is specified in attributes at
efi_set_variable(), specified data will be appended to the variable's
original value. Attributes other than APPEND_WRITE should not be
modified.

With this patch, APPEND_WRITE test in 'variables' selftest will pass.

Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
---
  lib/efi_loader/efi_variable.c | 70 ++++++++++++++++++++++-------------
  1 file changed, 44 insertions(+), 26 deletions(-)

diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 6687b69a400d..48ee255f879b 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -424,17 +424,17 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
                                     efi_uintn_t data_size, const void *data)
  {
        char *native_name = NULL, *val = NULL, *s;
+       const char *old_val;
+       size_t old_size;
        efi_status_t ret = EFI_SUCCESS;
        u32 attr;

        EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes,
                  data_size, data);

-       /* TODO: implement APPEND_WRITE */
        if (!variable_name || !*variable_name || !vendor ||
            ((attributes & EFI_VARIABLE_RUNTIME_ACCESS) &&
-            !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS)) ||
-           (attributes & EFI_VARIABLE_APPEND_WRITE)) {
+            !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) {
                ret = EFI_INVALID_PARAMETER;
                goto out;
        }
@@ -445,35 +445,51 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,

  #define ACCESS_ATTR (EFI_VARIABLE_RUNTIME_ACCESS | 
EFI_VARIABLE_BOOTSERVICE_ACCESS)

-       if ((data_size == 0) || !(attributes & ACCESS_ATTR)) {
-               /* delete the variable: */
-               env_set(native_name, NULL);
-               ret = EFI_SUCCESS;
-               goto out;
-       }
+       old_val = env_get(native_name);
+       if (old_val) {
+               old_val = parse_attr(old_val, &attr);

-       val = env_get(native_name);
-       if (val) {
-               parse_attr(val, &attr);
-
-               /* We should not free val */
-               val = NULL;
+               /* check read-only first */
                if (attr & READ_ONLY) {
                        ret = EFI_WRITE_PROTECTED;
                        goto out;
                }

-               /*
-                * attributes won't be changed
-                * TODO: take care of APPEND_WRITE once supported
-                */
-               if (attr != attributes) {
+               if ((data_size == 0) || !(attributes & ACCESS_ATTR)) {

The understanding of EDK2 is that no access attributes means 'attributes
== 0' (Function VariableServiceSetVariable() in
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c).

EFI_VARIABLE_APPEND_WRITE and data_size = 0 should not delete a variable
according to the UEFI spec:

"Unless the EFI_VARIABLE_APPEND_WRITE,
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, or
EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS attribute is set (see below),
using SetVariable() with a DataSize of zero will cause the entire
variable to be deleted."

Best regards

Heinrich

+                       /* delete the variable: */
+                       env_set(native_name, NULL);
+                       ret = EFI_SUCCESS;
+                       goto out;
+               }
+
+               /* attributes won't be changed */
+               if (attr != (attributes & ~EFI_VARIABLE_APPEND_WRITE)) {
                        ret = EFI_INVALID_PARAMETER;
                        goto out;
                }
+
+               if (attributes & EFI_VARIABLE_APPEND_WRITE) {
+                       if (!prefix(old_val, "(blob)")) {
+                               return EFI_DEVICE_ERROR;
+                               goto out;
+                       }
+                       old_size = strlen(old_val);
+               } else {
+                       old_size = 0;
+               }
+       } else {
+               if ((data_size == 0) || !(attributes & ACCESS_ATTR) ||
+                   (attributes & EFI_VARIABLE_APPEND_WRITE)) {
+                       /* delete, but nothing to do */
+                       ret = EFI_NOT_FOUND;
+                       goto out;
+               }
+
+               old_size = 0;
        }

-       val = malloc(2 * data_size + strlen("{ro,run,boot,nv}(blob)") + 1);
+       val = malloc(old_size + 2 * data_size
+                    + strlen("{ro,run,boot,nv}(blob)") + 1);
        if (!val) {
                ret = EFI_OUT_OF_RESOURCES;
                goto out;
@@ -481,10 +497,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,

        s = val;

-       /*
-        * store attributes
-        * TODO: several attributes are not supported
-        */
+       /* store attributes */
        attributes &= (EFI_VARIABLE_NON_VOLATILE |
                       EFI_VARIABLE_BOOTSERVICE_ACCESS |
                       EFI_VARIABLE_RUNTIME_ACCESS);
@@ -505,8 +518,13 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
        }
        s += sprintf(s, "}");

+       if (old_size)
+               /* APPEND_WRITE */
+               s += sprintf(s, old_val);
+       else
+               s += sprintf(s, "(blob)");
+
        /* store payload: */
-       s += sprintf(s, "(blob)");
        s = bin2hex(s, data, data_size);
        *s = '\0';



_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to