Hi Heinrich,

On Wed, 15 Feb 2023 at 17:49, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

On 2/15/23 03:29, Masahisa Kojima wrote:

...

UEFI specification v2.10 says that EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and EFI_UNSUPPORTED should be returned in SetVariable variable service. Current implementation returns EFI_INVALID_PARAMETER, let's fix the return value.

Together with above change, this commit also updates the SetVariable attribute check to be aligned with the EDK2 reference implementation.

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

Changes in v2:

• fix coding style

• HR must be set with NV

  lib/efi_loader/efi_variable.c | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 4c85cfa607..b12e79d658 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -230,9 +230,31 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, u64 time = 0; enum efi_auth_var_type var_type;

• if (!variable_name || !*variable_name || !vendor ||
  

•     ((attributes & EFI_VARIABLE_RUNTIME_ACCESS) &&
  

•      !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS)))
  

• if (!variable_name || !*variable_name || !vendor)
  

•         return EFI_INVALID_PARAMETER;
  

• if (data_size && !data)
  

•         return EFI_INVALID_PARAMETER;
  

• /* EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated */
  

• if (attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS ||
  

•     (!(attributes & EFI_VARIABLE_MASK)))
  

UEFI Spec 2.10, p. 215: "Setting a data variable with no access attributes causes it to be deleted."

We don't want to make deletion unsupported.

Sorry, you are correct. I will remove the second condition.

Thanks, Masahisa Kojima

Best regards

Heinrich

...

•         return EFI_UNSUPPORTED;
  

• /* Make sure if runtime bit is set, boot service bit is set also */
  

• if ((attributes &
  

•      (EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS)) ==
  

•     EFI_VARIABLE_RUNTIME_ACCESS)
  

•         return EFI_INVALID_PARAMETER;
  

• /* only EFI_VARIABLE_NON_VOLATILE attribute is invalid */
  

• if ((attributes & EFI_VARIABLE_MASK) == EFI_VARIABLE_NON_VOLATILE)
  

•         return EFI_INVALID_PARAMETER;
  

• /* Make sure HR is set with NV */
  

• if ((attributes &
  

•      (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_HARDWARE_ERROR_RECORD)) ==
  

•     EFI_VARIABLE_HARDWARE_ERROR_RECORD)
          return EFI_INVALID_PARAMETER;
  
  /* check if a variable exists */
  

@@ -281,8 +303,6 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,

  /* authenticate a variable */
  if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) {

•         if (attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS)
  

•                 return EFI_INVALID_PARAMETER;
          if (attributes &
              EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) {
                  u32 env_attr;
  

@@ -300,8 +320,7 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, } } else { if (attributes &

•             (EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS |
  

•              EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) {
  

•             EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) {
                  EFI_PRINT("Secure boot is not configured\n");
                  return EFI_INVALID_PARAMETER;
          }