On 4/29/21 7:46 PM, Vincent Stehlé wrote:

QueryVariableInfo() must return EFI_INVALID_PARAMETER when an invalid combination of attribute bits is supplied.

This fixes three SCT QueryVariableInfo_Conf failures.

Signed-off-by: Vincent Stehlé vincent.stehle@arm.com Reviewed-by: Grant Likely grant.likely@arm.com Cc: Heinrich Schuchardt xypron.glpk@gmx.de Cc: Alexander Graf agraf@csgraf.de

Changes since v1:

• Remove if/else and return directly

---

lib/efi_loader/efi_var_common.c | 4 ++++ 1 file changed, 4 insertions(+)

diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c index b11ed91a74a..cbf8685fad5 100644 --- a/lib/efi_loader/efi_var_common.c +++ b/lib/efi_loader/efi_var_common.c @@ -160,6 +160,10 @@ efi_status_t EFIAPI efi_query_variable_info( EFI_ENTRY("%x %p %p %p", attributes, maximum_variable_storage_size, remaining_variable_storage_size, maximum_variable_size);

• if (!attributes || ((attributes & EFI_VARIABLE_RUNTIME_ACCESS) &&

• 	    !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS)))
  

• return EFI_EXIT(EFI_INVALID_PARAMETER);
  

Thank you for looking into closing this UEFI compliance gap.

The checks must be executed at runtime too. efi_query_variable_info() only covers boot time.

StandaloneMM has its own check in RuntimeServiceQueryVariableInfo(). That function only checks that attributes is non-zero. If you consider that check wrong, you would have to fix it there.

Your U-Boot side checks should be put into efi_query_variable_info_int().

Why should we consider the following values valid?

attributes == EFI_VARIABLE_APPEND_WRITE attributes == EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS attributes == EFI_VARIABLE_HARDWARE_ERROR_RECORD attributes == 0xffffff00

EFI_VARIABLE_APPEND_WRITE shall be ignored according to chapter 8.2 "Variable Services". Shouldn't we remove the bit before checking the remaining value?

EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS for a variable that is neither BS nor RT makes little sense.

According to chapter 8.2.4.2, "Hardware Error Record Variables" a hardware error variable must be NV, BS, RT, HR.

0xffffff00 has bits set that can never appear in attributes.

efi_set_variable_int() would also accept some invalid combinations of flags. You could try to extract a common function for checking.

Best regards

Heinrich

• ret = efi_query_variable_info_int(attributes, maximum_variable_storage_size, remaining_variable_storage_size,