| Message ID | 20200322095731.76982-2-xypron.glpk@gmx.de |
|---|---|
| State | Accepted |
| Commit | 4d7f5af841c4622fb6c5d155e31c1072f3b052df |
| Headers | show |
| Series | efi_loader: correct reported length in GetNextVariable() | expand |
Heinrich Schuchardt <xypron.glpk at 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 at 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
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);
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 at 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(-) -- 2.25.1