[PATCH] efi_loader: leave attribute check to StMM variable service
Current U-Boot supports two EFI variable service, U-Boot own implementation and op-tee based StMM variable service. For latter case, parameter check should leave to StMM. This commit removes the attribute check from the common function(efi_query_variable_info) and moves it to lib/efi_loader/efi_variable.c.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org --- lib/efi_loader/efi_var_common.c | 10 +--------- lib/efi_loader/efi_variable.c | 10 ++++++++++ 2 files changed, 11 insertions(+), 9 deletions(-)
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..86f39181e0 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -349,6 +349,16 @@ efi_status_t efi_query_variable_info_int(u32 attributes, u64 *remaining_variable_storage_size, u64 *maximum_variable_size) { + if (!(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS)) + 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); + *maximum_variable_storage_size = EFI_VAR_BUF_SIZE - sizeof(struct efi_var_file); *remaining_variable_storage_size = efi_var_mem_free();
Hi Kojima-san
This looks correct when U-Boot is using StMM for the variable storage. Since Arm claims that SCT document is outdated should we also fix the default behavior? IOW U-Boot should return identical values when variables are stored in a file in the ESP.
On Thu, Jan 26, 2023 at 12:15:12PM +0900, Masahisa Kojima wrote:
Current U-Boot supports two EFI variable service, U-Boot own implementation and op-tee based StMM variable service. For latter case, parameter check should leave to StMM. This commit removes the attribute check from the common function(efi_query_variable_info) and moves it to lib/efi_loader/efi_variable.c.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org
lib/efi_loader/efi_var_common.c | 10 +--------- lib/efi_loader/efi_variable.c | 10 ++++++++++ 2 files changed, 11 insertions(+), 9 deletions(-)
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)
- 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..86f39181e0 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -349,6 +349,16 @@ efi_status_t efi_query_variable_info_int(u32 attributes, u64 *remaining_variable_storage_size, u64 *maximum_variable_size) {
- if (!(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))
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);- *maximum_variable_storage_size = EFI_VAR_BUF_SIZE - sizeof(struct efi_var_file); *remaining_variable_storage_size = efi_var_mem_free();
-- 2.17.1
Cheers /Ilias
On Wed, 1 Feb 2023 at 18:32, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Kojima-san
This looks correct when U-Boot is using StMM for the variable storage. Since Arm claims that SCT document is outdated should we also fix the default behavior? IOW U-Boot should return identical values when variables are stored in a file in the ESP.
Hi Ilias,
Yes, I will also fix the case that EFI variables are stored in a file in the ESP.
Thanks, Masahisa Kojima
On Thu, Jan 26, 2023 at 12:15:12PM +0900, Masahisa Kojima wrote:
Current U-Boot supports two EFI variable service, U-Boot own implementation and op-tee based StMM variable service. For latter case, parameter check should leave to StMM. This commit removes the attribute check from the common function(efi_query_variable_info) and moves it to lib/efi_loader/efi_variable.c.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org
lib/efi_loader/efi_var_common.c | 10 +--------- lib/efi_loader/efi_variable.c | 10 ++++++++++ 2 files changed, 11 insertions(+), 9 deletions(-)
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..86f39181e0 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -349,6 +349,16 @@ efi_status_t efi_query_variable_info_int(u32 attributes, u64 *remaining_variable_storage_size, u64 *maximum_variable_size) {
if (!(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))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);*maximum_variable_storage_size = EFI_VAR_BUF_SIZE - sizeof(struct efi_var_file); *remaining_variable_storage_size = efi_var_mem_free();-- 2.17.1
Cheers /Ilias
participants (2)
-
Ilias Apalodimas -
Masahisa Kojima