The VariableNameSize parameter is in bytes but u16_strnlen() counts u16.

Fix the parameter check for null termination.

Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- include/efi_variable.h | 3 ++- lib/efi_loader/efi_var_mem.c | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/efi_variable.h b/include/efi_variable.h index 03a3ecb235..805e6c5f1e 100644 --- a/include/efi_variable.h +++ b/include/efi_variable.h @@ -268,7 +268,8 @@ const efi_guid_t *efi_auth_var_get_guid(const u16 *name); * efi_get_next_variable_name_mem() - Runtime common code across efi variable * implementations for GetNextVariable() * from the cached memory copy - * @variable_name_size: size of variable_name buffer in byte + * + * @variable_name_size: size of variable_name buffer in bytes * @variable_name: name of uefi variable's name in u16 * @vendor: vendor's guid * diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c index 13909b1d26..0bac594e00 100644 --- a/lib/efi_loader/efi_var_mem.c +++ b/lib/efi_loader/efi_var_mem.c @@ -315,14 +315,14 @@ efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, u16 *variable_name, efi_guid_t *vendor) { struct efi_var_entry *var; - efi_uintn_t old_size; + efi_uintn_t len, old_size; u16 *pdata;

if (!variable_name_size || !variable_name || !vendor) return EFI_INVALID_PARAMETER;

- if (u16_strnlen(variable_name, *variable_name_size) == - *variable_name_size) + len = *variable_name_size >> 1; + if (u16_strnlen(variable_name, len) == len) return EFI_INVALID_PARAMETER;

if (!efi_var_mem_find(vendor, variable_name, &var) && *variable_name)

Test that GetNextVariableName() checks the parameters.

Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- lib/efi_selftest/efi_selftest_variables.c | 35 +++++++++++++++++++++++ 1 file changed, 35 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_variables.c b/lib/efi_selftest/efi_selftest_variables.c index dc1d5c8f43..c7a3fdbaa6 100644 --- a/lib/efi_selftest/efi_selftest_variables.c +++ b/lib/efi_selftest/efi_selftest_variables.c @@ -141,6 +141,41 @@ static int execute(void) return EFI_ST_FAILURE; } /* Enumerate variables */ + + ret = runtime->get_next_variable_name(NULL, u"efi_st_var1", &guid); + if (ret != EFI_INVALID_PARAMETER) { + efi_st_error("GetNextVariableName missing parameter check\n"); + return EFI_ST_FAILURE; + } + + len = 24; + ret = runtime->get_next_variable_name(&len, NULL, &guid); + if (ret != EFI_INVALID_PARAMETER) { + efi_st_error("GetNextVariableName missing parameter check\n"); + return EFI_ST_FAILURE; + } + + len = 24; + ret = runtime->get_next_variable_name(&len, u"efi_st_var1", NULL); + if (ret != EFI_INVALID_PARAMETER) { + efi_st_error("GetNextVariableName missing parameter check\n"); + return EFI_ST_FAILURE; + } + + len = 1; + ret = runtime->get_next_variable_name(&len, u"", &guid); + if (ret != EFI_INVALID_PARAMETER) { + efi_st_error("GetNextVariableName missing parameter check\n"); + return EFI_ST_FAILURE; + } + + len = 16; + ret = runtime->get_next_variable_name(&len, u"efi_st_var1", &guid); + if (ret != EFI_INVALID_PARAMETER) { + efi_st_error("GetNextVariableName missing parameter check\n"); + return EFI_ST_FAILURE; + } + boottime->set_mem(&guid, 16, 0); *varname = 0; flag = 0;

On Sun, 18 Dec 2022 at 08:09, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:

The VariableNameSize parameter is in bytes but u16_strnlen() counts u16.

Fix the parameter check for null termination. Provide unit test for parameter checks in GetNextVariableName

@Ilias: Could you, please, run the unit test against the OP-TEE variable store (CONFIG_EFI_MM_COMM_TEE=y)..

Sure, keep in mind there's one test there that fails for StMM (and we should probably look into fixing it in StMM) With this hack applied -- which irrelevant to your patch, everything seems ok

diff --git a/lib/efi_selftest/efi_selftest_variables.c b/lib/efi_selftest/efi_selftest_variables.c index c7a3fdbaa67d..8254bacd08e1 100644 --- a/lib/efi_selftest/efi_selftest_variables.c +++ b/lib/efi_selftest/efi_selftest_variables.c @@ -138,7 +138,7 @@ static int execute(void) 15, v); if (ret != EFI_NOT_FOUND) { efi_st_error("SetVariable(APPEND_WRITE) with size 0 to non-existent variable returns wrong code\n"); - return EFI_ST_FAILURE; + //return EFI_ST_FAILURE; } /* Enumerate variables */

Testing EFI API implementation

Selected test: 'variables'

Setting up 'variables' Setting up 'variables' succeeded

Executing 'variables' lib/efi_selftest/efi_selftest_variables.c(140): ERROR: SetVariable(APPEND_WRITE) with size 0 to non-existent variable returns wrong code Executing 'variables' succeeded

Summary: 0 failures

Cheers /Ilias

Heinrich Schuchardt (2): efi_loader: fix efi_get_next_variable_name_mem() efi_selftest: conformance test for GetNextVariableName

include/efi_variable.h | 3 +- lib/efi_loader/efi_var_mem.c | 6 ++-- lib/efi_selftest/efi_selftest_variables.c | 35 +++++++++++++++++++++++ 3 files changed, 40 insertions(+), 4 deletions(-)

-- 2.37.2