[PATCH] efi_loader: Set variable attributes when EFI_BUFFER_TOO_SMALL is returned

Starting UEFI Spec 2.8 we must fill in the variable attributes when GetVariable() returns EFI_BUFFER_TOO_SMALL and Attributes is non-NULL.
This code was written with 2.7 in mind so let's move the code around a bit and fill in the attributes EFI_BUFFER_TOO_SMALL is returned
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- lib/efi_loader/efi_variable_tee.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index 58931c4efd74..33c7dfc53d14 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -408,22 +408,31 @@ efi_status_t efi_get_variable_int(const u16 *variable_name, /* Communicate */ ret = mm_communicate(comm_buf, payload_size); if (ret == EFI_SUCCESS || ret == EFI_BUFFER_TOO_SMALL) { + int tmp; /* Update with reported data size for trimmed case */ *data_size = var_acc->data_size; + /* + * UEFI > 2.7 needs the attributes set even if the buffer is + * smaller + */ + tmp = get_property_int(variable_name, name_size, vendor, + &var_property); + if (tmp != EFI_SUCCESS) { + ret = tmp; + goto out; + } + + if (attributes) { + *attributes = var_acc->attr; + if (var_property.property & + VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY) + *attributes |= EFI_VARIABLE_READ_ONLY; + } } - if (ret != EFI_SUCCESS) - goto out;
- ret = get_property_int(variable_name, name_size, vendor, &var_property); if (ret != EFI_SUCCESS) goto out;
- if (attributes) { - *attributes = var_acc->attr; - if (var_property.property & VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY) - *attributes |= EFI_VARIABLE_READ_ONLY; - } - if (data) memcpy(data, (u8 *)var_acc->name + var_acc->name_size, var_acc->data_size);

On 3/16/22 13:55, Ilias Apalodimas wrote:
Starting UEFI Spec 2.8 we must fill in the variable attributes when GetVariable() returns EFI_BUFFER_TOO_SMALL and Attributes is non-NULL.
This code was written with 2.7 in mind so let's move the code around a bit and fill in the attributes EFI_BUFFER_TOO_SMALL is returned
In the UEFI spec changelog:
1954 set (*Attributes) when GetVariable() returns EFI_BUFFER_TOO_SMALL and Attributes is non-NULL
In efi_get_variable_mem() this already works correctly.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
lib/efi_loader/efi_variable_tee.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index 58931c4efd74..33c7dfc53d14 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -408,22 +408,31 @@ efi_status_t efi_get_variable_int(const u16 *variable_name, /* Communicate */ ret = mm_communicate(comm_buf, payload_size); if (ret == EFI_SUCCESS || ret == EFI_BUFFER_TOO_SMALL) {
You could make the code easier to read with:
if (ret != EFI_SUCCESS && ret != EFI_BUFFER_TOO_SMALL) goto out;
/* Update with reported data size for trimmed case */ *data_size = var_acc->data_size;int tmp;
/*
* UEFI > 2.7 needs the attributes set even if the buffer is
* smaller
*/
tmp = get_property_int(variable_name, name_size, vendor,
&var_property);
Why call get_property_int() if (!attributes)? You don't use the data in this case.
Best regards
Heinrich
if (tmp != EFI_SUCCESS) {
ret = tmp;
goto out;
}
if (attributes) {
*attributes = var_acc->attr;
if (var_property.property &
VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY)
*attributes |= EFI_VARIABLE_READ_ONLY;
}}
if (ret != EFI_SUCCESS)
goto out;
ret = get_property_int(variable_name, name_size, vendor, &var_property); if (ret != EFI_SUCCESS) goto out;
if (attributes) {
*attributes = var_acc->attr;
if (var_property.property & VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY)
*attributes |= EFI_VARIABLE_READ_ONLY;
}
if (data) memcpy(data, (u8 *)var_acc->name + var_acc->name_size, var_acc->data_size);

On Wed, Mar 16, 2022 at 02:40:20PM +0100, Heinrich Schuchardt wrote:
On 3/16/22 13:55, Ilias Apalodimas wrote:
Starting UEFI Spec 2.8 we must fill in the variable attributes when GetVariable() returns EFI_BUFFER_TOO_SMALL and Attributes is non-NULL.
This code was written with 2.7 in mind so let's move the code around a bit and fill in the attributes EFI_BUFFER_TOO_SMALL is returned
In the UEFI spec changelog:
1954 set (*Attributes) when GetVariable() returns EFI_BUFFER_TOO_SMALL and Attributes is non-NULL
In efi_get_variable_mem() this already works correctly.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
lib/efi_loader/efi_variable_tee.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index 58931c4efd74..33c7dfc53d14 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -408,22 +408,31 @@ efi_status_t efi_get_variable_int(const u16 *variable_name, /* Communicate */ ret = mm_communicate(comm_buf, payload_size); if (ret == EFI_SUCCESS || ret == EFI_BUFFER_TOO_SMALL) {
You could make the code easier to read with:
if (ret != EFI_SUCCESS && ret != EFI_BUFFER_TOO_SMALL) goto out;
Sure
/* Update with reported data size for trimmed case */ *data_size = var_acc->data_size;int tmp;
/*
* UEFI > 2.7 needs the attributes set even if the buffer is
* smaller
*/
tmp = get_property_int(variable_name, name_size, vendor,
&var_property);
Why call get_property_int() if (!attributes)? You don't use the data in this case.
Good point. I just moved the code around, but i'll fix that along the way
Thanks /Ilias
Best regards
Heinrich
if (tmp != EFI_SUCCESS) {
ret = tmp;
goto out;
}
if (attributes) {
*attributes = var_acc->attr;
if (var_property.property &
VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY)
*attributes |= EFI_VARIABLE_READ_ONLY;
}}
if (ret != EFI_SUCCESS)
goto out;
ret = get_property_int(variable_name, name_size, vendor, &var_property); if (ret != EFI_SUCCESS) goto out;
if (attributes) {
*attributes = var_acc->attr;
if (var_property.property & VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY)
*attributes |= EFI_VARIABLE_READ_ONLY;
}
if (data) memcpy(data, (u8 *)var_acc->name + var_acc->name_size, var_acc->data_size);
participants (2)
-
Heinrich Schuchardt
-
Ilias Apalodimas