[U-Boot] [PATCH v2 0/2] efi_loader: support APPEND_WRITE

Changes in v2 (Sept 6, 2019) * add a check against read-only variable to delete with APPEND_WRITE * add a check against non-existent variable to append * add/modify APPEND_WRITE tests
AKASHI Takahiro (2): efi_loader: variable: support APPEND_WRITE efi_loader: selftest: enable APPEND_WRITE tests
lib/efi_loader/efi_variable.c | 70 ++++++++++++++--------- lib/efi_selftest/efi_selftest_variables.c | 20 ++++++- 2 files changed, 63 insertions(+), 27 deletions(-)

If EFI_VARIABLE_APPEND_WRITE is specified in attributes at efi_set_variable(), specified data will be appended to the variable's original value. Attributes other than APPEND_WRITE should not be modified.
With this patch, APPEND_WRITE test in 'variables' selftest will pass.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- lib/efi_loader/efi_variable.c | 70 ++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 26 deletions(-)
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 6687b69a400d..48ee255f879b 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -424,17 +424,17 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, efi_uintn_t data_size, const void *data) { char *native_name = NULL, *val = NULL, *s; + const char *old_val; + size_t old_size; efi_status_t ret = EFI_SUCCESS; u32 attr;
EFI_ENTRY(""%ls" %pUl %x %zu %p", variable_name, vendor, attributes, data_size, data);
- /* TODO: implement APPEND_WRITE */ if (!variable_name || !*variable_name || !vendor || ((attributes & EFI_VARIABLE_RUNTIME_ACCESS) && - !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS)) || - (attributes & EFI_VARIABLE_APPEND_WRITE)) { + !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) { ret = EFI_INVALID_PARAMETER; goto out; } @@ -445,35 +445,51 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
#define ACCESS_ATTR (EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS)
- if ((data_size == 0) || !(attributes & ACCESS_ATTR)) { - /* delete the variable: */ - env_set(native_name, NULL); - ret = EFI_SUCCESS; - goto out; - } + old_val = env_get(native_name); + if (old_val) { + old_val = parse_attr(old_val, &attr);
- val = env_get(native_name); - if (val) { - parse_attr(val, &attr); - - /* We should not free val */ - val = NULL; + /* check read-only first */ if (attr & READ_ONLY) { ret = EFI_WRITE_PROTECTED; goto out; }
- /* - * attributes won't be changed - * TODO: take care of APPEND_WRITE once supported - */ - if (attr != attributes) { + if ((data_size == 0) || !(attributes & ACCESS_ATTR)) { + /* delete the variable: */ + env_set(native_name, NULL); + ret = EFI_SUCCESS; + goto out; + } + + /* attributes won't be changed */ + if (attr != (attributes & ~EFI_VARIABLE_APPEND_WRITE)) { ret = EFI_INVALID_PARAMETER; goto out; } + + if (attributes & EFI_VARIABLE_APPEND_WRITE) { + if (!prefix(old_val, "(blob)")) { + return EFI_DEVICE_ERROR; + goto out; + } + old_size = strlen(old_val); + } else { + old_size = 0; + } + } else { + if ((data_size == 0) || !(attributes & ACCESS_ATTR) || + (attributes & EFI_VARIABLE_APPEND_WRITE)) { + /* delete, but nothing to do */ + ret = EFI_NOT_FOUND; + goto out; + } + + old_size = 0; }
- val = malloc(2 * data_size + strlen("{ro,run,boot,nv}(blob)") + 1); + val = malloc(old_size + 2 * data_size + + strlen("{ro,run,boot,nv}(blob)") + 1); if (!val) { ret = EFI_OUT_OF_RESOURCES; goto out; @@ -481,10 +497,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
s = val;
- /* - * store attributes - * TODO: several attributes are not supported - */ + /* store attributes */ attributes &= (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS); @@ -505,8 +518,13 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, } s += sprintf(s, "}");
+ if (old_size) + /* APPEND_WRITE */ + s += sprintf(s, old_val); + else + s += sprintf(s, "(blob)"); + /* store payload: */ - s += sprintf(s, "(blob)"); s = bin2hex(s, data, data_size); *s = '\0';

On 9/6/19 8:09 AM, AKASHI Takahiro wrote:
If EFI_VARIABLE_APPEND_WRITE is specified in attributes at efi_set_variable(), specified data will be appended to the variable's original value. Attributes other than APPEND_WRITE should not be modified.
With this patch, APPEND_WRITE test in 'variables' selftest will pass.
With both patches of the series applied I get the following warning in bootefi selftest:
lib/efi_selftest/efi_selftest_variables.c(151): TODO: SetVariable(APPEND_WRITE) to read-only variable returns wrong code
Best regards
Heinrich
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_variable.c | 70 ++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 26 deletions(-)
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 6687b69a400d..48ee255f879b 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -424,17 +424,17 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, efi_uintn_t data_size, const void *data) { char *native_name = NULL, *val = NULL, *s;
const char *old_val;
size_t old_size; efi_status_t ret = EFI_SUCCESS; u32 attr;
EFI_ENTRY(""%ls" %pUl %x %zu %p", variable_name, vendor, attributes, data_size, data);
- /* TODO: implement APPEND_WRITE */ if (!variable_name || !*variable_name || !vendor || ((attributes & EFI_VARIABLE_RUNTIME_ACCESS) &&
!(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS)) ||
(attributes & EFI_VARIABLE_APPEND_WRITE)) {
ret = EFI_INVALID_PARAMETER; goto out; }!(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) {
@@ -445,35 +445,51 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
#define ACCESS_ATTR (EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS)
- if ((data_size == 0) || !(attributes & ACCESS_ATTR)) {
/* delete the variable: */
env_set(native_name, NULL);
ret = EFI_SUCCESS;
goto out;
- }
- old_val = env_get(native_name);
- if (old_val) {
old_val = parse_attr(old_val, &attr);
- val = env_get(native_name);
- if (val) {
parse_attr(val, &attr);
/* We should not free val */
val = NULL;
if (attr & READ_ONLY) { ret = EFI_WRITE_PROTECTED; goto out; }/* check read-only first */
/*
* attributes won't be changed
* TODO: take care of APPEND_WRITE once supported
*/
if (attr != attributes) {
if ((data_size == 0) || !(attributes & ACCESS_ATTR)) {
/* delete the variable: */
env_set(native_name, NULL);
ret = EFI_SUCCESS;
goto out;
}
/* attributes won't be changed */
}if (attr != (attributes & ~EFI_VARIABLE_APPEND_WRITE)) { ret = EFI_INVALID_PARAMETER; goto out;
if (attributes & EFI_VARIABLE_APPEND_WRITE) {
if (!prefix(old_val, "(blob)")) {
return EFI_DEVICE_ERROR;
goto out;
}
old_size = strlen(old_val);
} else {
old_size = 0;
}
- } else {
if ((data_size == 0) || !(attributes & ACCESS_ATTR) ||
(attributes & EFI_VARIABLE_APPEND_WRITE)) {
/* delete, but nothing to do */
ret = EFI_NOT_FOUND;
goto out;
}
}old_size = 0;
- val = malloc(2 * data_size + strlen("{ro,run,boot,nv}(blob)") + 1);
- val = malloc(old_size + 2 * data_size
if (!val) { ret = EFI_OUT_OF_RESOURCES; goto out;+ strlen("{ro,run,boot,nv}(blob)") + 1);
@@ -481,10 +497,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
s = val;
- /*
* store attributes
* TODO: several attributes are not supported
*/
- /* store attributes */ attributes &= (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS);
@@ -505,8 +518,13 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, } s += sprintf(s, "}");
- if (old_size)
/* APPEND_WRITE */
s += sprintf(s, old_val);
- else
s += sprintf(s, "(blob)");
- /* store payload: */
- s += sprintf(s, "(blob)"); s = bin2hex(s, data, data_size); *s = '\0';

On Mon, Sep 16, 2019 at 07:03:41PM +0200, Heinrich Schuchardt wrote:
On 9/6/19 8:09 AM, AKASHI Takahiro wrote:
If EFI_VARIABLE_APPEND_WRITE is specified in attributes at efi_set_variable(), specified data will be appended to the variable's original value. Attributes other than APPEND_WRITE should not be modified.
With this patch, APPEND_WRITE test in 'variables' selftest will pass.
With both patches of the series applied I get the following warning in bootefi selftest:
lib/efi_selftest/efi_selftest_variables.c(151): TODO: SetVariable(APPEND_WRITE) to read-only variable returns wrong code
Yes, I know. That's why I used TODO here. The reason that this test fails is that we don't support read-only variables, more strictly, we do have read-only attribute, but don't have any interface to specify read-only attribute for any variable.
Read-only is an implicit nature of variable only attributed to some "global" variables UEFI specification defines. So we should fix the issue in a separate patch.
Thanks, -Takahiro Akashi
Best regards
Heinrich
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_variable.c | 70 ++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 26 deletions(-)
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 6687b69a400d..48ee255f879b 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -424,17 +424,17 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, efi_uintn_t data_size, const void *data) { char *native_name = NULL, *val = NULL, *s;
const char *old_val;
size_t old_size; efi_status_t ret = EFI_SUCCESS; u32 attr;
EFI_ENTRY(""%ls" %pUl %x %zu %p", variable_name, vendor, attributes, data_size, data);
- /* TODO: implement APPEND_WRITE */ if (!variable_name || !*variable_name || !vendor || ((attributes & EFI_VARIABLE_RUNTIME_ACCESS) &&
!(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS)) ||
(attributes & EFI_VARIABLE_APPEND_WRITE)) {
ret = EFI_INVALID_PARAMETER; goto out; }!(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) {
@@ -445,35 +445,51 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
#define ACCESS_ATTR (EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS)
- if ((data_size == 0) || !(attributes & ACCESS_ATTR)) {
/* delete the variable: */
env_set(native_name, NULL);
ret = EFI_SUCCESS;
goto out;
- }
- old_val = env_get(native_name);
- if (old_val) {
old_val = parse_attr(old_val, &attr);
- val = env_get(native_name);
- if (val) {
parse_attr(val, &attr);
/* We should not free val */
val = NULL;
if (attr & READ_ONLY) { ret = EFI_WRITE_PROTECTED; goto out; }/* check read-only first */
/*
* attributes won't be changed
* TODO: take care of APPEND_WRITE once supported
*/
if (attr != attributes) {
if ((data_size == 0) || !(attributes & ACCESS_ATTR)) {
/* delete the variable: */
env_set(native_name, NULL);
ret = EFI_SUCCESS;
goto out;
}
/* attributes won't be changed */
}if (attr != (attributes & ~EFI_VARIABLE_APPEND_WRITE)) { ret = EFI_INVALID_PARAMETER; goto out;
if (attributes & EFI_VARIABLE_APPEND_WRITE) {
if (!prefix(old_val, "(blob)")) {
return EFI_DEVICE_ERROR;
goto out;
}
old_size = strlen(old_val);
} else {
old_size = 0;
}
- } else {
if ((data_size == 0) || !(attributes & ACCESS_ATTR) ||
(attributes & EFI_VARIABLE_APPEND_WRITE)) {
/* delete, but nothing to do */
ret = EFI_NOT_FOUND;
goto out;
}
}old_size = 0;
- val = malloc(2 * data_size + strlen("{ro,run,boot,nv}(blob)") + 1);
- val = malloc(old_size + 2 * data_size
if (!val) { ret = EFI_OUT_OF_RESOURCES; goto out;+ strlen("{ro,run,boot,nv}(blob)") + 1);
@@ -481,10 +497,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
s = val;
- /*
* store attributes
* TODO: several attributes are not supported
*/
- /* store attributes */ attributes &= (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS);
@@ -505,8 +518,13 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, } s += sprintf(s, "}");
- if (old_size)
/* APPEND_WRITE */
s += sprintf(s, old_val);
- else
s += sprintf(s, "(blob)");
- /* store payload: */
- s += sprintf(s, "(blob)"); s = bin2hex(s, data, data_size); *s = '\0';

On 9/6/19 8:09 AM, AKASHI Takahiro wrote:
If EFI_VARIABLE_APPEND_WRITE is specified in attributes at efi_set_variable(), specified data will be appended to the variable's original value. Attributes other than APPEND_WRITE should not be modified.
With this patch, APPEND_WRITE test in 'variables' selftest will pass.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_variable.c | 70 ++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 26 deletions(-)
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 6687b69a400d..48ee255f879b 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -424,17 +424,17 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, efi_uintn_t data_size, const void *data) { char *native_name = NULL, *val = NULL, *s;
const char *old_val;
size_t old_size; efi_status_t ret = EFI_SUCCESS; u32 attr;
EFI_ENTRY(""%ls" %pUl %x %zu %p", variable_name, vendor, attributes, data_size, data);
- /* TODO: implement APPEND_WRITE */ if (!variable_name || !*variable_name || !vendor || ((attributes & EFI_VARIABLE_RUNTIME_ACCESS) &&
!(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS)) ||
(attributes & EFI_VARIABLE_APPEND_WRITE)) {
ret = EFI_INVALID_PARAMETER; goto out; }!(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) {
@@ -445,35 +445,51 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
#define ACCESS_ATTR (EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS)
- if ((data_size == 0) || !(attributes & ACCESS_ATTR)) {
/* delete the variable: */
env_set(native_name, NULL);
ret = EFI_SUCCESS;
goto out;
- }
- old_val = env_get(native_name);
- if (old_val) {
old_val = parse_attr(old_val, &attr);
- val = env_get(native_name);
- if (val) {
parse_attr(val, &attr);
/* We should not free val */
val = NULL;
if (attr & READ_ONLY) { ret = EFI_WRITE_PROTECTED; goto out; }/* check read-only first */
/*
* attributes won't be changed
* TODO: take care of APPEND_WRITE once supported
*/
if (attr != attributes) {
if ((data_size == 0) || !(attributes & ACCESS_ATTR)) {
The understanding of EDK2 is that no access attributes means 'attributes == 0' (Function VariableServiceSetVariable() in MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c).
EFI_VARIABLE_APPEND_WRITE and data_size = 0 should not delete a variable according to the UEFI spec:
"Unless the EFI_VARIABLE_APPEND_WRITE, EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, or EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS attribute is set (see below), using SetVariable() with a DataSize of zero will cause the entire variable to be deleted."
Best regards
Heinrich
/* delete the variable: */
env_set(native_name, NULL);
ret = EFI_SUCCESS;
goto out;
}
/* attributes won't be changed */
}if (attr != (attributes & ~EFI_VARIABLE_APPEND_WRITE)) { ret = EFI_INVALID_PARAMETER; goto out;
if (attributes & EFI_VARIABLE_APPEND_WRITE) {
if (!prefix(old_val, "(blob)")) {
return EFI_DEVICE_ERROR;
goto out;
}
old_size = strlen(old_val);
} else {
old_size = 0;
}
- } else {
if ((data_size == 0) || !(attributes & ACCESS_ATTR) ||
(attributes & EFI_VARIABLE_APPEND_WRITE)) {
/* delete, but nothing to do */
ret = EFI_NOT_FOUND;
goto out;
}
}old_size = 0;
- val = malloc(2 * data_size + strlen("{ro,run,boot,nv}(blob)") + 1);
- val = malloc(old_size + 2 * data_size
if (!val) { ret = EFI_OUT_OF_RESOURCES; goto out;+ strlen("{ro,run,boot,nv}(blob)") + 1);
@@ -481,10 +497,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
s = val;
- /*
* store attributes
* TODO: several attributes are not supported
*/
- /* store attributes */ attributes &= (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS);
@@ -505,8 +518,13 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, } s += sprintf(s, "}");
- if (old_size)
/* APPEND_WRITE */
s += sprintf(s, old_val);
- else
s += sprintf(s, "(blob)");
- /* store payload: */
- s += sprintf(s, "(blob)"); s = bin2hex(s, data, data_size); *s = '\0';

On Tue, Sep 17, 2019 at 10:38:47PM +0200, Heinrich Schuchardt wrote:
On 9/6/19 8:09 AM, AKASHI Takahiro wrote:
If EFI_VARIABLE_APPEND_WRITE is specified in attributes at efi_set_variable(), specified data will be appended to the variable's original value. Attributes other than APPEND_WRITE should not be modified.
With this patch, APPEND_WRITE test in 'variables' selftest will pass.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_variable.c | 70 ++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 26 deletions(-)
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 6687b69a400d..48ee255f879b 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -424,17 +424,17 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, efi_uintn_t data_size, const void *data) { char *native_name = NULL, *val = NULL, *s;
const char *old_val;
size_t old_size; efi_status_t ret = EFI_SUCCESS; u32 attr;
EFI_ENTRY(""%ls" %pUl %x %zu %p", variable_name, vendor, attributes, data_size, data);
- /* TODO: implement APPEND_WRITE */ if (!variable_name || !*variable_name || !vendor || ((attributes & EFI_VARIABLE_RUNTIME_ACCESS) &&
!(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS)) ||
(attributes & EFI_VARIABLE_APPEND_WRITE)) {
ret = EFI_INVALID_PARAMETER; goto out; }!(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) {
@@ -445,35 +445,51 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
#define ACCESS_ATTR (EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS)
- if ((data_size == 0) || !(attributes & ACCESS_ATTR)) {
/* delete the variable: */
env_set(native_name, NULL);
ret = EFI_SUCCESS;
goto out;
- }
- old_val = env_get(native_name);
- if (old_val) {
old_val = parse_attr(old_val, &attr);
- val = env_get(native_name);
- if (val) {
parse_attr(val, &attr);
/* We should not free val */
val = NULL;
if (attr & READ_ONLY) { ret = EFI_WRITE_PROTECTED; goto out; }/* check read-only first */
/*
* attributes won't be changed
* TODO: take care of APPEND_WRITE once supported
*/
if (attr != attributes) {
if ((data_size == 0) || !(attributes & ACCESS_ATTR)) {
The understanding of EDK2 is that no access attributes means 'attributes == 0' (Function VariableServiceSetVariable() in MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c).
No, it's not correct. The right place you should refer to is: UpdateVariable ( [snip up to l.2279 or so] // // Setting a data variable with no access, or zero DataSize attributes // causes it to be deleted. // When the EFI_VARIABLE_APPEND_WRITE attribute is set, DataSize of zero will // not delete the variable. // if ((((Attributes & EFI_VARIABLE_APPEND_WRITE) == 0) && (DataSize == 0))|| ((Attributes & (EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS)) == 0)) {
EFI_VARIABLE_APPEND_WRITE and data_size = 0 should not delete a variable according to the UEFI spec:
This is correct.
Thanks, -Takahiro Akashi
"Unless the EFI_VARIABLE_APPEND_WRITE, EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, or EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS attribute is set (see below), using SetVariable() with a DataSize of zero will cause the entire variable to be deleted."
Best regards
Heinrich
/* delete the variable: */
env_set(native_name, NULL);
ret = EFI_SUCCESS;
goto out;
}
/* attributes won't be changed */
}if (attr != (attributes & ~EFI_VARIABLE_APPEND_WRITE)) { ret = EFI_INVALID_PARAMETER; goto out;
if (attributes & EFI_VARIABLE_APPEND_WRITE) {
if (!prefix(old_val, "(blob)")) {
return EFI_DEVICE_ERROR;
goto out;
}
old_size = strlen(old_val);
} else {
old_size = 0;
}
- } else {
if ((data_size == 0) || !(attributes & ACCESS_ATTR) ||
(attributes & EFI_VARIABLE_APPEND_WRITE)) {
/* delete, but nothing to do */
ret = EFI_NOT_FOUND;
goto out;
}
}old_size = 0;
- val = malloc(2 * data_size + strlen("{ro,run,boot,nv}(blob)") + 1);
- val = malloc(old_size + 2 * data_size
if (!val) { ret = EFI_OUT_OF_RESOURCES; goto out;+ strlen("{ro,run,boot,nv}(blob)") + 1);
@@ -481,10 +497,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
s = val;
- /*
* store attributes
* TODO: several attributes are not supported
*/
- /* store attributes */ attributes &= (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS);
@@ -505,8 +518,13 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, } s += sprintf(s, "}");
- if (old_size)
/* APPEND_WRITE */
s += sprintf(s, old_val);
- else
s += sprintf(s, "(blob)");
- /* store payload: */
- s += sprintf(s, "(blob)"); s = bin2hex(s, data, data_size); *s = '\0';

Now that APPEND_WRITE is supported, the result check for the only existing test case should be changed to 'todo' to 'error', while two more test cases are added.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- lib/efi_selftest/efi_selftest_variables.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/lib/efi_selftest/efi_selftest_variables.c b/lib/efi_selftest/efi_selftest_variables.c index 06c1a032dd04..a6b41d1f008d 100644 --- a/lib/efi_selftest/efi_selftest_variables.c +++ b/lib/efi_selftest/efi_selftest_variables.c @@ -21,6 +21,9 @@ static const efi_guid_t guid_vendor0 = static const efi_guid_t guid_vendor1 = EFI_GUID(0xff629290, 0x1fc1, 0xd73f, 0x8f, 0xb1, 0x32, 0xf9, 0x0c, 0xa0, 0x42, 0xea); +static const efi_guid_t guid_global = + EFI_GUID(0x8be4df61, 0x93ca, 0x11d2, + 0xaa, 0x0d, 0x00, 0xe0, 0x98, 0x03, 0x2b, 0x8c);
/* * Setup unit test. @@ -116,7 +119,7 @@ static int execute(void) EFI_VARIABLE_APPEND_WRITE, 7, v + 8); if (ret != EFI_SUCCESS) { - efi_st_todo("SetVariable(APPEND_WRITE) failed\n"); + efi_st_error("SetVariable(APPEND_WRITE) failed\n"); } else { len = EFI_ST_MAX_DATA_SIZE; ret = runtime->get_variable(L"efi_st_var1", &guid_vendor1, @@ -131,6 +134,21 @@ static int execute(void) if (memcmp(data, v, len)) efi_st_todo("GetVariable returned wrong value\n"); } + /* Append variable 2 */ + ret = runtime->set_variable(L"efi_none", &guid_vendor1, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_APPEND_WRITE, + 15, v); + if (ret != EFI_NOT_FOUND) + efi_st_error("SetVariable(APPEND_WRITE) with size 0 to non-existent variable returns wrong code\n"); + /* Append variable 3 */ + ret = runtime->set_variable(L"PlatformLangCodes", &guid_global, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_APPEND_WRITE, + 15, v); + if (ret != EFI_WRITE_PROTECTED) + efi_st_todo("SetVariable(APPEND_WRITE) to read-only variable returns wrong code\n"); /* Enumerate variables */ boottime->set_mem(&guid, 16, 0); *varname = 0;
participants (3)
-
AKASHI Takahiro
-
Heinrich Schuchardt
-
Heinrich Schuchardt