* [PATCH 0/2] efi_loader: correct reported length in GetNextVariable() @ 2020-03-20 18:28 Heinrich Schuchardt 2020-03-20 18:28 ` [PATCH 1/2] " Heinrich Schuchardt 2020-03-20 18:28 ` [PATCH 2/2] efi_selftest: check length report by GetNextVariableName() Heinrich Schuchardt 0 siblings, 2 replies; 6+ messages in thread From: Heinrich Schuchardt @ 2020-03-20 18:28 UTC (permalink / raw) To: u-boot 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(). Provide a 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 | 2 +- lib/efi_selftest/efi_selftest_variables.c | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] efi_loader: correct reported length in GetNextVariable() 2020-03-20 18:28 [PATCH 0/2] efi_loader: correct reported length in GetNextVariable() Heinrich Schuchardt @ 2020-03-20 18:28 ` Heinrich Schuchardt 2020-03-24 8:41 ` Punit Agrawal 2020-03-30 6:40 ` AKASHI Takahiro 2020-03-20 18:28 ` [PATCH 2/2] efi_selftest: check length report by GetNextVariableName() Heinrich Schuchardt 1 sibling, 2 replies; 6+ messages in thread From: Heinrich Schuchardt @ 2020-03-20 18:28 UTC (permalink / raw) To: u-boot 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> --- lib/efi_loader/efi_variable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index c316bdfec0..04ead34c6f 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -299,7 +299,7 @@ static efi_status_t parse_uboot_variable(char *variable, p = variable_name; utf8_utf16_strncpy(&p, name, name_len); variable_name[name_len] = 0; - *variable_name_size = name_len + 1; + *variable_name_size = sizeof(u16) * (name_len + 1); /* guid */ c = *(name - 1); -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/2] efi_loader: correct reported length in GetNextVariable() 2020-03-20 18:28 ` [PATCH 1/2] " Heinrich Schuchardt @ 2020-03-24 8:41 ` Punit Agrawal 2020-03-30 6:40 ` AKASHI Takahiro 1 sibling, 0 replies; 6+ messages in thread From: Punit Agrawal @ 2020-03-24 8:41 UTC (permalink / raw) To: u-boot Hi Heinrich, 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> > --- > lib/efi_loader/efi_variable.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > index c316bdfec0..04ead34c6f 100644 > --- a/lib/efi_loader/efi_variable.c > +++ b/lib/efi_loader/efi_variable.c > @@ -299,7 +299,7 @@ static efi_status_t parse_uboot_variable(char *variable, > p = variable_name; > utf8_utf16_strncpy(&p, name, name_len); > variable_name[name_len] = 0; > - *variable_name_size = name_len + 1; > + *variable_name_size = sizeof(u16) * (name_len + 1); Maybe I am missing something, but isn't a similar fix needed in the function where the buffer is checked for sufficient size? For context, I am referring to if (*variable_name_size < (name_len + 1)) { *variable_name_size = name_len + 1; return EFI_BUFFER_TOO_SMALL; } Thanks, Punit > > /* guid */ > c = *(name - 1); > -- > 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] efi_loader: correct reported length in GetNextVariable() 2020-03-20 18:28 ` [PATCH 1/2] " Heinrich Schuchardt 2020-03-24 8:41 ` Punit Agrawal @ 2020-03-30 6:40 ` AKASHI Takahiro 1 sibling, 0 replies; 6+ messages in thread From: AKASHI Takahiro @ 2020-03-30 6:40 UTC (permalink / raw) To: u-boot On Fri, Mar 20, 2020 at 07:28:19PM +0100, Heinrich Schuchardt wrote: > 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(). Please also correct a function description of GetNextVariable(). -Takahiro Akashi > Fixes: d99a87f84b75 ("efi_loader: implement GetNextVariableName()") > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > lib/efi_loader/efi_variable.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > index c316bdfec0..04ead34c6f 100644 > --- a/lib/efi_loader/efi_variable.c > +++ b/lib/efi_loader/efi_variable.c > @@ -299,7 +299,7 @@ static efi_status_t parse_uboot_variable(char *variable, > p = variable_name; > utf8_utf16_strncpy(&p, name, name_len); > variable_name[name_len] = 0; > - *variable_name_size = name_len + 1; > + *variable_name_size = sizeof(u16) * (name_len + 1); > > /* guid */ > c = *(name - 1); > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] efi_selftest: check length report by GetNextVariableName() 2020-03-20 18:28 [PATCH 0/2] efi_loader: correct reported length in GetNextVariable() Heinrich Schuchardt 2020-03-20 18:28 ` [PATCH 1/2] " Heinrich Schuchardt @ 2020-03-20 18:28 ` Heinrich Schuchardt 2020-03-30 6:43 ` AKASHI Takahiro 1 sibling, 1 reply; 6+ messages in thread From: Heinrich Schuchardt @ 2020-03-20 18:28 UTC (permalink / raw) To: u-boot GetNextVariableName should report the length of the variable including the final 0x0000 in bytes. Check this in the unit test. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- lib/efi_selftest/efi_selftest_variables.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/efi_selftest/efi_selftest_variables.c b/lib/efi_selftest/efi_selftest_variables.c index 5d98c029b8..b7ead340f5 100644 --- a/lib/efi_selftest/efi_selftest_variables.c +++ b/lib/efi_selftest/efi_selftest_variables.c @@ -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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] efi_selftest: check length report by GetNextVariableName() 2020-03-20 18:28 ` [PATCH 2/2] efi_selftest: check length report by GetNextVariableName() Heinrich Schuchardt @ 2020-03-30 6:43 ` AKASHI Takahiro 0 siblings, 0 replies; 6+ messages in thread From: AKASHI Takahiro @ 2020-03-30 6:43 UTC (permalink / raw) To: u-boot On Fri, Mar 20, 2020 at 07:28:20PM +0100, Heinrich Schuchardt wrote: > GetNextVariableName should report the length of the variable including the > final 0x0000 in bytes. > > Check this in the unit test. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > lib/efi_selftest/efi_selftest_variables.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/lib/efi_selftest/efi_selftest_variables.c b/lib/efi_selftest/efi_selftest_variables.c > index 5d98c029b8..b7ead340f5 100644 > --- a/lib/efi_selftest/efi_selftest_variables.c > +++ b/lib/efi_selftest/efi_selftest_variables.c > @@ -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")) Logically, length check should be added here, too. -Takahiro Akashi > flag |= 2; > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-30 6:43 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-03-20 18:28 [PATCH 0/2] efi_loader: correct reported length in GetNextVariable() Heinrich Schuchardt 2020-03-20 18:28 ` [PATCH 1/2] " Heinrich Schuchardt 2020-03-24 8:41 ` Punit Agrawal 2020-03-30 6:40 ` AKASHI Takahiro 2020-03-20 18:28 ` [PATCH 2/2] efi_selftest: check length report by GetNextVariableName() Heinrich Schuchardt 2020-03-30 6:43 ` AKASHI Takahiro
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox