| Message ID | 20180525000925.GA33845@intel.com |
|---|---|
| State | Superseded |
| Delegated to: | Alexander Graf |
| Headers | show |
| Series | [U-Boot] efi_selftest: check for buffer overflow in efi_get_variable | expand |
On 05/25/2018 02:09 AM, Ivan Gorinov wrote: > Check if GetVariable writes past the end of the output data buffer. > > Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com> The patch produces build warnings: lib/efi_selftest/efi_selftest_variables.c: In function ‘execute’: lib/efi_selftest/efi_selftest_variables.c:75:10: warning: assignment makes pointer from integer without a cast [-Wint-conversion] data[3] = 0xff; ^ lib/efi_selftest/efi_selftest_variables.c:87:14: warning: comparison between pointer and integer if (data[3] != 0xff) { This is due to an inappropriate definition in my prior patch. Could you, please, add this change: -u8 *data[EFI_ST_MAX_DATA_SIZE]; +u8 data[EFI_ST_MAX_DATA_SIZE]; Otherwise your patch is fine. Best regards Heinrich > --- > lib/efi_selftest/efi_selftest_variables.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/lib/efi_selftest/efi_selftest_variables.c b/lib/efi_selftest/efi_selftest_variables.c > index f5e8eb6..8a02dbc 100644 > --- a/lib/efi_selftest/efi_selftest_variables.c > +++ b/lib/efi_selftest/efi_selftest_variables.c > @@ -72,6 +72,22 @@ static int execute(void) > efi_st_error("SetVariable failed\n"); > return EFI_ST_FAILURE; > } > + data[3] = 0xff; > + len = 3; > + ret = runtime->get_variable(L"efi_st_var0", &guid_vendor0, > + &attr, &len, data); > + if (ret != EFI_SUCCESS) { > + efi_st_error("GetVariable failed\n"); > + return EFI_ST_FAILURE; > + } > + if (efi_st_memcmp(data, v + 4, 3)) { > + efi_st_error("GetVariable returned wrong value\n"); > + return EFI_ST_FAILURE; > + } > + if (data[3] != 0xff) { > + efi_st_error("GetVariable wrote past the end of the buffer\n"); > + return EFI_ST_FAILURE; > + } > /* Set variable 1 */ > ret = runtime->set_variable(L"efi_st_var1", &guid_vendor1, > EFI_VARIABLE_BOOTSERVICE_ACCESS, >
diff --git a/lib/efi_selftest/efi_selftest_variables.c b/lib/efi_selftest/efi_selftest_variables.c index f5e8eb6..8a02dbc 100644 --- a/lib/efi_selftest/efi_selftest_variables.c +++ b/lib/efi_selftest/efi_selftest_variables.c @@ -72,6 +72,22 @@ static int execute(void) efi_st_error("SetVariable failed\n"); return EFI_ST_FAILURE; } + data[3] = 0xff; + len = 3; + ret = runtime->get_variable(L"efi_st_var0", &guid_vendor0, + &attr, &len, data); + if (ret != EFI_SUCCESS) { + efi_st_error("GetVariable failed\n"); + return EFI_ST_FAILURE; + } + if (efi_st_memcmp(data, v + 4, 3)) { + efi_st_error("GetVariable returned wrong value\n"); + return EFI_ST_FAILURE; + } + if (data[3] != 0xff) { + efi_st_error("GetVariable wrote past the end of the buffer\n"); + return EFI_ST_FAILURE; + } /* Set variable 1 */ ret = runtime->set_variable(L"efi_st_var1", &guid_vendor1, EFI_VARIABLE_BOOTSERVICE_ACCESS,
Check if GetVariable writes past the end of the output data buffer. Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com> --- lib/efi_selftest/efi_selftest_variables.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)