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,