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

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;

• 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);
  

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);