From 584a2ada6e5e7752ad1af2fd54fa8d424a4da8d6 Mon Sep 17 00:00:00 2001 From: Masahisa Kojima Date: Tue, 2 Apr 2024 18:09:50 +0900 Subject: efi_loader: fix append write behavior to non-existent variable Current "variables" efi_selftest result is inconsistent between the U-Boot file storage and the tee-based StandaloneMM RPMB secure storage. U-Boot file storage implementation does not accept SetVariale call to non-existent variable with EFI_VARIABLE_APPEND_WRITE, it return EFI_NOT_FOUND. However it is accepted and new variable is created in EDK II StandaloneMM implementation if valid data and size are specified. If data size is 0, EFI_SUCCESS is returned. Since UEFI specification does not clearly describe the behavior of the append write to non-existent variable, let's update the U-Boot file storage implementation to get aligned with the EDK II reference implementation. Signed-off-by: Masahisa Kojima Reviewed-by: Ilias Apalodimas Tested-by: Ilias Apalodimas --- lib/efi_loader/efi_variable.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) (limited to 'lib/efi_loader/efi_variable.c') diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 40f7a0fb10d..d1db7ade0a0 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -282,11 +282,21 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, } time = var->time; } else { - if (delete || append) - /* - * Trying to delete or to update a non-existent - * variable. - */ + /* + * UEFI specification does not clearly describe the expected + * behavior of append write with data size 0, we follow + * the EDK II reference implementation. + */ + if (append && !data_size) + return EFI_SUCCESS; + + /* + * EFI_VARIABLE_APPEND_WRITE to non-existent variable is accepted + * and new variable is created in EDK II reference implementation. + * We follow it and only check the deletion here. + */ + if (delete) + /* Trying to delete a non-existent variable. */ return EFI_NOT_FOUND; } @@ -329,7 +339,11 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, /* EFI_NOT_FOUND has been handled before */ attributes = var->attr; ret = EFI_SUCCESS; - } else if (append) { + } else if (append && var) { + /* + * data is appended if EFI_VARIABLE_APPEND_WRITE is set and + * variable exists. + */ u16 *old_data = var->name; for (; *old_data; ++old_data) -- cgit v1.2.3 From 19327c1f90e528cd89eb170250ebea6338b424f5 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 3 Apr 2024 17:33:33 +0200 Subject: efi_loader: all variable attributes are 32bit GetVariable() and SetVariable() use an uint32_t value for attributes. The UEFI specification defines the related constants as 32bit. Add the missing EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS constant. Signed-off-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas --- lib/efi_loader/efi_variable.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/efi_loader/efi_variable.c') diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index d1db7ade0a0..8f553828021 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -259,7 +259,7 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, /* check if a variable exists */ var = efi_var_mem_find(vendor, variable_name, NULL); append = !!(attributes & EFI_VARIABLE_APPEND_WRITE); - attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE; + attributes &= ~EFI_VARIABLE_APPEND_WRITE; delete = !append && (!data_size || !attributes); /* check attributes */ @@ -398,7 +398,7 @@ efi_status_t efi_query_variable_info_int(u32 attributes, EFI_VARIABLE_RUNTIME_ACCESS) return EFI_INVALID_PARAMETER; - if (attributes & ~(u32)EFI_VARIABLE_MASK) + if (attributes & ~EFI_VARIABLE_MASK) return EFI_INVALID_PARAMETER; *maximum_variable_storage_size = EFI_VAR_BUF_SIZE - -- cgit v1.2.3 From 3b51c3a0b03411b07f0acd8bf2361ba54043fdcf Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 3 Apr 2024 17:33:34 +0200 Subject: efi_loader: EFI_VARIABLE_READ_ONLY should be 32bit GetVariable() and SetVariable() only accept a 32bit value for attributes. It makes not sense to define EFI_VARIABLE_READ_ONLY as unsigned long. Signed-off-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas --- lib/efi_loader/efi_variable.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/efi_loader/efi_variable.c') diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 8f553828021..b2f8ebdd78e 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -276,8 +276,8 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, /* attributes won't be changed */ if (!delete && ((ro_check && var->attr != attributes) || - (!ro_check && ((var->attr & ~(u32)EFI_VARIABLE_READ_ONLY) - != (attributes & ~(u32)EFI_VARIABLE_READ_ONLY))))) { + (!ro_check && ((var->attr & ~EFI_VARIABLE_READ_ONLY) + != (attributes & ~EFI_VARIABLE_READ_ONLY))))) { return EFI_INVALID_PARAMETER; } time = var->time; -- cgit v1.2.3 From e0fa2cf39cedd9297c16bc4ea4ff5c512bb4e0ec Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 3 Apr 2024 17:33:35 +0200 Subject: efi_loader: handle EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS We don't yet support EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS for file based variables, but we should pass it to TEE based variable stores. Signed-off-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas --- lib/efi_loader/efi_variable.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'lib/efi_loader/efi_variable.c') diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index b2f8ebdd78e..6fe3792a12a 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -235,8 +235,12 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, if (data_size && !data) return EFI_INVALID_PARAMETER; - /* EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated */ - if (attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) + /* + * EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated. + * We don't support EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS. + */ + if (attributes & (EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | \ + EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS)) return EFI_UNSUPPORTED; /* Make sure if runtime bit is set, boot service bit is set also */ -- cgit v1.2.3 From 6b0f194648f0f11281c01fb570d30931e6c3a13e Mon Sep 17 00:00:00 2001 From: Ilias Apalodimas Date: Wed, 3 Apr 2024 18:43:53 +0300 Subject: efi_loader: Don't delete variable from memory if adding a new one failed Our efi_var_mem_xxx() functions don't have a replace variant. Instead we add a new variable and delete the old instance when trying to replace a variable. Currently we delete the old version without checking the new one got added Signed-off-by: Ilias Apalodimas Reviewed-by: Heinrich Schuchardt --- lib/efi_loader/efi_variable.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib/efi_loader/efi_variable.c') diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 6fe3792a12a..2951dc78c7d 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -360,11 +360,12 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, ret = efi_var_mem_ins(variable_name, vendor, attributes, data_size, data, 0, NULL, time); } - efi_var_mem_del(var); if (ret != EFI_SUCCESS) return ret; + efi_var_mem_del(var); + if (var_type == EFI_AUTH_VAR_PK) ret = efi_init_secure_state(); else -- cgit v1.2.3