[PATCH v2 0/2] efi_loader: correct reported length in GetNextVariable()
The runtime service GetNextVariable() returns the length of the next variable including the closing 0x0000. This length should be in bytes.
Comparing the output of EDK2 and U-Boot shows that this is currently not correctly implemented:
EDK2: OsIndicationsSupported: 46 PlatformLang: 26 PlatformLangCodes: 36
U-Boot: OsIndicationsSupported: 23 PlatformLang: 13 PlatformLangCodes: 18
Provide correct length in GetNextVariable().
v2: correct return value of the initial call too increase buffer size for variable names in unit test
Heinrich Schuchardt (2): efi_loader: correct reported length in GetNextVariable() efi_selftest: check length report by GetNextVariableName()
lib/efi_loader/efi_variable.c | 11 ++++++----- lib/efi_selftest/efi_selftest_variables.c | 10 ++++++++-- 2 files changed, 14 insertions(+), 7 deletions(-)
-- 2.25.1
The runtime service GetNextVariable() returns the length of the next variable including the closing 0x0000. This length should be in bytes.
Comparing the output of EDK2 and U-Boot shows that this is currently not correctly implemented:
EDK2: OsIndicationsSupported: 46 PlatformLang: 26 PlatformLangCodes: 36
U-Boot: OsIndicationsSupported: 23 PlatformLang: 13 PlatformLangCodes: 18
Provide correct length in GetNextVariable().
Fixes: d99a87f84b75 ("efi_loader: implement GetNextVariableName()") Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2: correct return value of the initial call too --- lib/efi_loader/efi_variable.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 99d2f01f57..3bec2d0d17 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -273,7 +273,8 @@ static efi_status_t parse_uboot_variable(char *variable, u32 *attributes) { char *guid, *name, *end, c; - unsigned long name_len; + size_t name_len; + efi_uintn_t old_variable_name_size; u16 *p;
guid = strchr(variable, '_'); @@ -289,17 +290,17 @@ static efi_status_t parse_uboot_variable(char *variable, return EFI_INVALID_PARAMETER;
name_len = end - name; - if (*variable_name_size < (name_len + 1)) { - *variable_name_size = name_len + 1; + old_variable_name_size = *variable_name_size; + *variable_name_size = sizeof(u16) * (name_len + 1); + if (old_variable_name_size < *variable_name_size) return EFI_BUFFER_TOO_SMALL; - } + end++; /* point to value */
/* variable name */ p = variable_name; utf8_utf16_strncpy(&p, name, name_len); variable_name[name_len] = 0; - *variable_name_size = name_len + 1;
/* guid */ c = *(name - 1); -- 2.25.1
Heinrich Schuchardt xypron.glpk@gmx.de writes:
The runtime service GetNextVariable() returns the length of the next variable including the closing 0x0000. This length should be in bytes.
Comparing the output of EDK2 and U-Boot shows that this is currently not correctly implemented:
EDK2: OsIndicationsSupported: 46 PlatformLang: 26 PlatformLangCodes: 36
U-Boot: OsIndicationsSupported: 23 PlatformLang: 13 PlatformLangCodes: 18
Provide correct length in GetNextVariable().
Fixes: d99a87f84b75 ("efi_loader: implement GetNextVariableName()") Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v2: correct return value of the initial call too
lib/efi_loader/efi_variable.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 99d2f01f57..3bec2d0d17 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -273,7 +273,8 @@ static efi_status_t parse_uboot_variable(char *variable, u32 *attributes) { char *guid, *name, *end, c;
- unsigned long name_len;
size_t name_len;
efi_uintn_t old_variable_name_size; u16 *p;
guid = strchr(variable, '_');
@@ -289,17 +290,17 @@ static efi_status_t parse_uboot_variable(char *variable, return EFI_INVALID_PARAMETER;
name_len = end - name;
- if (*variable_name_size < (name_len + 1)) {
*variable_name_size = name_len + 1;
- old_variable_name_size = *variable_name_size;
- *variable_name_size = sizeof(u16) * (name_len + 1);
- if (old_variable_name_size < *variable_name_size) return EFI_BUFFER_TOO_SMALL;
- }
Ah I see it's fixed in this version. Please ignore my comment on the previous version.
Thanks, Punit
end++; /* point to value */
/* variable name */ p = variable_name; utf8_utf16_strncpy(&p, name, name_len); variable_name[name_len] = 0;
*variable_name_size = name_len + 1;
/* guid */ c = *(name - 1);
-- 2.25.1
GetNextVariableName should report the length of the variable including the final 0x0000 in bytes.
Check this in the unit test.
Increase the buffer size for variable names. 40 bytes is too short.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2: Increase the buffer size for variable names. --- lib/efi_selftest/efi_selftest_variables.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/lib/efi_selftest/efi_selftest_variables.c b/lib/efi_selftest/efi_selftest_variables.c index 5d98c029b8..2c16f3db6c 100644 --- a/lib/efi_selftest/efi_selftest_variables.c +++ b/lib/efi_selftest/efi_selftest_variables.c @@ -11,7 +11,7 @@ #include <efi_selftest.h>
#define EFI_ST_MAX_DATA_SIZE 16 -#define EFI_ST_MAX_VARNAME_SIZE 40 +#define EFI_ST_MAX_VARNAME_SIZE 80
static struct efi_boot_services *boottime; static struct efi_runtime_services *runtime; @@ -155,8 +155,14 @@ static int execute(void) return EFI_ST_FAILURE; } if (!memcmp(&guid, &guid_vendor0, sizeof(efi_guid_t)) && - !efi_st_strcmp_16_8(varname, "efi_st_var0")) + !efi_st_strcmp_16_8(varname, "efi_st_var0")) { flag |= 1; + if (len != 24) { + efi_st_error("GetNextVariableName report wrong length %u, expected 24\n", + (unsigned int)len); + return EFI_ST_FAILURE; + } + } if (!memcmp(&guid, &guid_vendor1, sizeof(efi_guid_t)) && !efi_st_strcmp_16_8(varname, "efi_st_var1")) flag |= 2; -- 2.25.1
participants (2)
-
Heinrich Schuchardt -
Punit Agrawal