diff mbox series

[v2] efi_loader: update attribute check for QueryVariableInfo()

Message ID 20230202135335.15939-1-masahisa.kojima@linaro.org
State Accepted
Commit 454a9442fbce761db7655381fdfaded8f09c8cf3
Headers show
Series [v2] efi_loader: update attribute check for QueryVariableInfo() | expand

Commit Message

Masahisa Kojima Feb. 2, 2023, 1:53 p.m. UTC
Current U-Boot supports two EFI variable service, U-Boot own
implementation and op-tee based StMM variable service.

With ACS Security Interface Extension(SIE) v22.10_SIE_REL1.1.0,
there are several failure items of QueryVariableInfo().

Current attribute check for QueryVariableInfo() was implemented
based on the Self Certification Test (SCT) II Case Specification,
June 2017, chapter 4.1.4 QueryVariableInfo().
This test case specification is outdated and don't align at all
with the SCT test case code, and UEFI specification v2.10 does
not clearly define the priority of the attribute check.

For U-Boot standard case that EFI variables are stored in a file
in the ESP, this commit modifies the attribute check to get align
to the EDK2 implementation.

For latter case(op-tee based StMM variable service), parameter check
should be delegated to StMM.

Now all ACS SIE QueryVariableInfo() test cases passed both EFI variable
storage implementations.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
Changes in v2:
- fix both U-Boot file based variable storage and StMM

 lib/efi_loader/efi_var_common.c | 10 +---------
 lib/efi_loader/efi_variable.c   | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+), 9 deletions(-)
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index eb83702781..ad50bffd2b 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -165,17 +165,9 @@  efi_status_t EFIAPI efi_query_variable_info(
 
 	if (!maximum_variable_storage_size ||
 	    !remaining_variable_storage_size ||
-	    !maximum_variable_size ||
-	    !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))
+	    !maximum_variable_size)
 		return EFI_EXIT(EFI_INVALID_PARAMETER);
 
-	if ((attributes & ~(u32)EFI_VARIABLE_MASK) ||
-	    (attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) ||
-	    (attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) ||
-	    (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT) &&
-	     (attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)))
-		return EFI_EXIT(EFI_UNSUPPORTED);
-
 	ret = efi_query_variable_info_int(attributes,
 					  maximum_variable_storage_size,
 					  remaining_variable_storage_size,
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 7c32adf6e5..ba135f827d 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -349,6 +349,28 @@  efi_status_t efi_query_variable_info_int(u32 attributes,
 					 u64 *remaining_variable_storage_size,
 					 u64 *maximum_variable_size)
 {
+	if (attributes == 0)
+		return EFI_INVALID_PARAMETER;
+
+	/* EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated */
+	if ((attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) ||
+	    ((attributes & EFI_VARIABLE_MASK) == 0))
+		return EFI_UNSUPPORTED;
+
+	if ((attributes & EFI_VARIABLE_MASK) == EFI_VARIABLE_NON_VOLATILE)
+		return EFI_INVALID_PARAMETER;
+
+	/* 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;
+
+	if (attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD)
+		return EFI_UNSUPPORTED;
+
+	if (attributes & ~(u32)EFI_VARIABLE_MASK)
+		return EFI_INVALID_PARAMETER;
+
 	*maximum_variable_storage_size = EFI_VAR_BUF_SIZE -
 					 sizeof(struct efi_var_file);
 	*remaining_variable_storage_size = efi_var_mem_free();