[U-Boot] [PATCH] efi_loader: variable: attributes may not be changed if a variable exists

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Tue, May 14, 2019 at 08:08:49PM +0200, Heinrich Schuchardt wrote:
> On 5/14/19 8:35 AM, Heinrich Schuchardt wrote:
> >On 5/14/19 6:57 AM, AKASHI Takahiro wrote:
> >>If a variable already exists, efi_set_variable() should not change
> >>the variable's attributes. This patch enforces it.
> >
> >This behavior is mandated by UEFI spec 2.7.
> >
> >Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> 
> This patch let's `bootefi selftest`fail:
> 
> Executing 'variables'
> lib/efi_selftest/efi_selftest_variables.c(60):
> TODO: QueryVariableInfo failed
> lib/efi_selftest/efi_selftest_variables.c(119):
> ERROR: SetVariable failed
> lib/efi_selftest/efi_selftest.c(110):
> ERROR: Executing 'variables' failed
> 
> The preferred solution would be to implement APPEND_WRITE.
> 
> Otherwise at least adjust the unit test concerning APPEND_WRITE to use
> efi_st_todo() and not to abort the test.

Since the current code doesn't supoort APPEND_WRITE, my commit
doesn't break anything. You should fix selftest first.

I don't have an immediate plan to implement APPEND_WRITE for now.

-Takahiro Akashi

> I suggest that you always run `bootefi selftest` before submitting
> changes to the UEFI sub-system.
> 
> Best regards
> 
> Heinrich
> 
> >
> >>
> >>Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >>---
> >>  lib/efi_loader/efi_variable.c | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >>diff --git a/lib/efi_loader/efi_variable.c
> >>b/lib/efi_loader/efi_variable.c
> >>index 37728c3c165d..c4f3a5d2743d 100644
> >>--- a/lib/efi_loader/efi_variable.c
> >>+++ b/lib/efi_loader/efi_variable.c
> >>@@ -450,6 +450,15 @@ efi_status_t EFIAPI efi_set_variable(u16
> >>*variable_name,
> >>              ret = EFI_WRITE_PROTECTED;
> >>              goto out;
> >>          }
> >>+
> >>+        /*
> >>+         * attributes won't be changed
> >>+         * TODO: take care of APPEND_WRITE once supported
> >>+         */
> >>+        if (attr != attributes) {
> >>+            ret = EFI_INVALID_PARAMETER;
> >>+            goto out;
> >>+        }
> >>      }
> >>
> >>      val = malloc(2 * data_size + strlen("{ro,run,boot}(blob)") + 1);
> >>
> >
> >
>