[U-Boot] [PATCH v2 0/3] efi_loader: variable: attributes may not be changed if a variable exists
Changes in v2 * add patch#1 and patch#2 to avoid sefltest failure * doesn't free a buffer returned by env_get() (patch#3)
AKASHI Takahiro (3): efi: selftest: APPEND_WRITE is not supported efi_loader: variable: return error for APPEND_WRITE efi_loader: variable: attributes may not be changed if a variable exists
lib/efi_loader/efi_variable.c | 16 ++++++++++--- lib/efi_selftest/efi_selftest_variables.c | 28 +++++++++++------------ 2 files changed, 27 insertions(+), 17 deletions(-)
The error here should be marked *todo*.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- lib/efi_selftest/efi_selftest_variables.c | 28 +++++++++++------------ 1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/lib/efi_selftest/efi_selftest_variables.c b/lib/efi_selftest/efi_selftest_variables.c index b028c64bbc85..06c1a032dd04 100644 --- a/lib/efi_selftest/efi_selftest_variables.c +++ b/lib/efi_selftest/efi_selftest_variables.c @@ -116,21 +116,21 @@ static int execute(void) EFI_VARIABLE_APPEND_WRITE, 7, v + 8); if (ret != EFI_SUCCESS) { - efi_st_error("SetVariable failed\n"); - return EFI_ST_FAILURE; - } - len = EFI_ST_MAX_DATA_SIZE; - ret = runtime->get_variable(L"efi_st_var1", &guid_vendor1, - &attr, &len, data); - if (ret != EFI_SUCCESS) { - efi_st_error("GetVariable failed\n"); - return EFI_ST_FAILURE; + efi_st_todo("SetVariable(APPEND_WRITE) failed\n"); + } else { + len = EFI_ST_MAX_DATA_SIZE; + ret = runtime->get_variable(L"efi_st_var1", &guid_vendor1, + &attr, &len, data); + if (ret != EFI_SUCCESS) { + efi_st_error("GetVariable failed\n"); + return EFI_ST_FAILURE; + } + if (len != 15) + efi_st_todo("GetVariable returned wrong length %u\n", + (unsigned int)len); + if (memcmp(data, v, len)) + efi_st_todo("GetVariable returned wrong value\n"); } - if (len != 15) - efi_st_todo("GetVariable returned wrong length %u\n", - (unsigned int)len); - if (memcmp(data, v, len)) - efi_st_todo("GetVariable returned wrong value\n"); /* Enumerate variables */ boottime->set_mem(&guid, 16, 0); *varname = 0;
On 5/24/19 8:59 AM, AKASHI Takahiro wrote:
The error here should be marked *todo*.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
The current efi_et_variable() doesn't support EFI_VARIABLE_APPEND_WRITE attiribut for now, and so should return an error.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- lib/efi_loader/efi_variable.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 28b1aa7505ae..e3ec502ffb45 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -427,7 +427,8 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, EFI_ENTRY(""%ls" %pUl %x %zu %p", variable_name, vendor, attributes, data_size, data);
- if (!variable_name || !vendor) { + if (!variable_name || !vendor || + (attributes & EFI_VARIABLE_APPEND_WRITE)) { ret = EFI_INVALID_PARAMETER; goto out; }
On 5/24/19 8:59 AM, AKASHI Takahiro wrote:
The current efi_et_variable() doesn't support EFI_VARIABLE_APPEND_WRITE attiribut for now, and so should return an error.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
I will correct the typos in the commit message and add a TODO comment in the code.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/efi_loader/efi_variable.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 28b1aa7505ae..e3ec502ffb45 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -427,7 +427,8 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, EFI_ENTRY(""%ls" %pUl %x %zu %p", variable_name, vendor, attributes, data_size, data);
- if (!variable_name || !vendor) {
- if (!variable_name || !vendor ||
(attributes & EFI_VARIABLE_APPEND_WRITE)) {
If a variable already exists, efi_set_variable() should not change the variable's attributes. This patch enforces it.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- lib/efi_loader/efi_variable.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index e3ec502ffb45..1bb3bbf3393e 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -450,12 +450,21 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, if (val) { parse_attr(val, &attr);
+ /* We should not free val */ + val = NULL; if (attr & READ_ONLY) { - /* We should not free val */ - val = NULL; ret = EFI_WRITE_PROTECTED; goto out; } + + /* + * attributes won't be changed + * TODO: take care of APPEND_WRITE once supported + */ + if (attr != attributes) { + ret = EFI_INVALID_PARAMETER; + goto out; + } }
val = malloc(2 * data_size + strlen("{ro,run,boot}(blob)") + 1);
On 5/24/19 8:59 AM, AKASHI Takahiro wrote:
If a variable already exists, efi_set_variable() should not change the variable's attributes. This patch enforces it.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/efi_loader/efi_variable.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
participants (2)
-
AKASHI Takahiro -
Heinrich Schuchardt