Hi Heinrich,
[...]
memset(v2, 0x1, sizeof(v2)); ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, &max_storage, &rem_storage, &max_size);@@ -63,10 +69,107 @@ static int execute(void) EFI_VARIABLE_RUNTIME_ACCESS, 3, v + 4); if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
efi_uintn_t prev_len, delta;if (ret != EFI_INVALID_PARAMETER) { efi_st_error("SetVariable failed\n"); return EFI_ST_FAILURE; }len = sizeof(data);ret = runtime->get_variable(u"RTStorageVolatile",&efi_rt_var_guid,&attr, &len, data);if (ret != EFI_SUCCESS) {efi_st_error("GetVariable failed\n");return EFI_ST_FAILURE;}len = sizeof(data2);ret = runtime->get_variable(u"VarToFile", &efi_rt_var_guid,&attr, &len, data2);if (ret != EFI_SUCCESS) {efi_st_error("GetVariable failed\n");return EFI_ST_FAILURE;}/** VarToFile will size must change once a variable is inserted* Store it now, we'll use it later*/prev_len = len;ret = runtime->set_variable(u"efi_st_var0", &guid_vendor0,EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS |EFI_VARIABLE_NON_VOLATILE,sizeof(v2),v2);/** This will try to update VarToFile as well and must fail,* without changing or deleting VarToFile*/if (ret != EFI_OUT_OF_RESOURCES) {efi_st_error("SetVariable failed\n");return EFI_ST_FAILURE;}len = sizeof(data2);ret = runtime->get_variable(u"VarToFile", &efi_rt_var_guid,&attr, &len, data2);if (ret != EFI_SUCCESS || prev_len != len) {efi_st_error("Get/SetVariable failed\n");return EFI_ST_FAILURE;}/* SetVariableRT updates VarToFile with efi_st_var0 */ret = runtime->set_variable(u"efi_st_var0", &guid_vendor0,EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS |EFI_VARIABLE_NON_VOLATILE,sizeof(v), v);It is fine to test with variable name size (24) and variable value size (16) both being multiples of 8. But this does not cover the generic case.
We already test SetVariable with a non-aligned variable at the start. I'll keep this as is and add append 1byte at the last test as you suggested.
if (ret != EFI_SUCCESS) {efi_st_error("SetVariable failed\n");return EFI_ST_FAILURE;}len = sizeof(data2);delta = 2 * (u16_strlen(u"efi_st_var0") + 1) + sizeof(v) +sizeof(struct efi_var_entry);In the file all variable are stored at 8 byte aligned positions. I am missing ALIGN(,8) here.
Ah yes
ret = runtime->get_variable(u"VarToFile", &efi_rt_var_guid,&attr, &len, data2);if (ret != EFI_SUCCESS || prev_len + delta != len) {efi_st_error("Get/SetVariable failed\n");return EFI_ST_FAILURE;}We should check the header fields of VarToFile (reserved, magic, length, crc32), e.g.
struct efi_var_file *hdr = (void *)data2; if (memcmp(hdr->magic, EFI_VAR_FILE_MAGIC, 8) || len != ALIGN(hdr->length + sizeof(hdr), 8) || ... )
Sure, good idea,
/* append on an existing variable will updateVarToFile */ret = runtime->set_variable(u"efi_st_var0", &guid_vendor0,EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS |EFI_VARIABLE_APPEND_WRITE |EFI_VARIABLE_NON_VOLATILE,sizeof(v), v);How about just appending 1 byte when using EFI_VARIABLE_APPEND_WRITE and checking that the delta here too?
Yes, I'll change this
[...]
Thanks /Ilias