All of lore.kernel.org
 help / color / mirror / Atom feed
From: AKASHI Takahiro <takahiro.akashi@linaro.org>
To: u-boot@lists.denx.de
Subject: [PATCH v2 1/1] efi_loader: prepare for read only OP-TEE variables
Date: Tue, 23 Jun 2020 08:44:09 +0900	[thread overview]
Message-ID: <20200622234409.GA26273@laputa> (raw)
In-Reply-To: <20200622161027.124993-1-xypron.glpk@gmx.de>

On Mon, Jun 22, 2020 at 06:10:27PM +0200, Heinrich Schuchardt wrote:
> We currently have two implementations of UEFI variables:
> 
> * variables provided via an OP-TEE module
> * variables stored in the U-Boot environment
> 
> Read only variables are up to now only implemented in the U-Boot
> environment implementation.
> 
> Provide a common interface for both implementations that allows handling
> read-only variables.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2:
> 	add missing efi_variable.h
> 	consider attributes==NULL in efi_variable_get()
> ---
>  include/efi_variable.h               |  40 +++++++
>  lib/efi_loader/Makefile              |   1 +
>  lib/efi_loader/efi_variable.c        | 171 ++++++++-------------------
>  lib/efi_loader/efi_variable_common.c |  79 +++++++++++++
>  lib/efi_loader/efi_variable_tee.c    |  75 ++++--------
>  5 files changed, 188 insertions(+), 178 deletions(-)
>  create mode 100644 include/efi_variable.h
>  create mode 100644 lib/efi_loader/efi_variable_common.c
> 
> diff --git a/include/efi_variable.h b/include/efi_variable.h
> new file mode 100644
> index 0000000000..784dbd9cf4
> --- /dev/null
> +++ b/include/efi_variable.h

I think that all the stuff here should be put in efi_loader.h.
I don't see any benefit of having a separate header.


> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2020, Heinrich Schuchardt <xypron.glpk@gmx.de>
> + */
> +
> +#ifndef _EFI_VARIABLE_H
> +#define _EFI_VARIABLE_H
> +
> +#define EFI_VARIABLE_READ_ONLY BIT(31)

This is not part of UEFI specification.
Having the same prefix, EFI_VARIABLE_, as other attributes
can be confusing.

-Takahiro Akashi


> +/**
> + * efi_get_variable() - retrieve value of a UEFI variable
> + *
> + * @variable_name:	name of the variable
> + * @vendor:		vendor GUID
> + * @attributes:		attributes of the variable
> + * @data_size:		size of the buffer to which the variable value is copied
> + * @data:		buffer to which the variable value is copied
> + * Return:		status code
> + */
> +efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor,
> +				  u32 *attributes, efi_uintn_t *data_size,
> +				  void *data);
> +
> +/**
> + * efi_set_variable() - set value of a UEFI variable
> + *
> + * @variable_name:	name of the variable
> + * @vendor:		vendor GUID
> + * @attributes:		attributes of the variable
> + * @data_size:		size of the buffer with the variable value
> + * @data:		buffer with the variable value
> + * @ro_check:		check the read only read only bit in attributes
> + * Return:		status code
> + */
> +efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
> +				  u32 attributes, efi_uintn_t data_size,
> +				  const void *data, bool ro_check);
> +
> +#endif
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 57c7e66ea0..16b93ef7f0 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -35,6 +35,7 @@ obj-y += efi_root_node.o
>  obj-y += efi_runtime.o
>  obj-y += efi_setup.o
>  obj-$(CONFIG_EFI_UNICODE_COLLATION_PROTOCOL2) += efi_unicode_collation.o
> +obj-y += efi_variable_common.o
>  ifeq ($(CONFIG_EFI_MM_COMM_TEE),y)
>  obj-y += efi_variable_tee.o
>  else
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index e097670e28..85df1427bc 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -7,6 +7,7 @@
> 
>  #include <common.h>
>  #include <efi_loader.h>
> +#include <efi_variable.h>
>  #include <env.h>
>  #include <env_internal.h>
>  #include <hexdump.h>
> @@ -15,7 +16,6 @@
>  #include <search.h>
>  #include <uuid.h>
>  #include <crypto/pkcs7_parser.h>
> -#include <linux/bitops.h>
>  #include <linux/compat.h>
>  #include <u-boot/crc.h>
> 
> @@ -30,20 +30,6 @@ static bool efi_secure_boot;
>  static int efi_secure_mode;
>  static u8 efi_vendor_keys;
> 
> -#define READ_ONLY BIT(31)
> -
> -static efi_status_t efi_get_variable_common(u16 *variable_name,
> -					    const efi_guid_t *vendor,
> -					    u32 *attributes,
> -					    efi_uintn_t *data_size, void *data);
> -
> -static efi_status_t efi_set_variable_common(u16 *variable_name,
> -					    const efi_guid_t *vendor,
> -					    u32 attributes,
> -					    efi_uintn_t data_size,
> -					    const void *data,
> -					    bool ro_check);
> -
>  /*
>   * Mapping between EFI variables and u-boot variables:
>   *
> @@ -154,7 +140,7 @@ static const char *parse_attr(const char *str, u32 *attrp, u64 *timep)
>  		str++;
> 
>  		if ((s = prefix(str, "ro"))) {
> -			attr |= READ_ONLY;
> +			attr |= EFI_VARIABLE_READ_ONLY;
>  		} else if ((s = prefix(str, "nv"))) {
>  			attr |= EFI_VARIABLE_NON_VOLATILE;
>  		} else if ((s = prefix(str, "boot"))) {
> @@ -202,29 +188,29 @@ static efi_status_t efi_set_secure_state(int sec_boot, int setup_mode,
> 
>  	attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
>  		     EFI_VARIABLE_RUNTIME_ACCESS |
> -		     READ_ONLY;
> -	ret = efi_set_variable_common(L"SecureBoot", &efi_global_variable_guid,
> -				      attributes, sizeof(sec_boot), &sec_boot,
> -				      false);
> +		     EFI_VARIABLE_READ_ONLY;
> +	ret = efi_set_variable_int(L"SecureBoot", &efi_global_variable_guid,
> +				   attributes, sizeof(sec_boot), &sec_boot,
> +				   false);
>  	if (ret != EFI_SUCCESS)
>  		goto err;
> 
> -	ret = efi_set_variable_common(L"SetupMode", &efi_global_variable_guid,
> -				      attributes, sizeof(setup_mode),
> -				      &setup_mode, false);
> +	ret = efi_set_variable_int(L"SetupMode", &efi_global_variable_guid,
> +				   attributes, sizeof(setup_mode),
> +				   &setup_mode, false);
>  	if (ret != EFI_SUCCESS)
>  		goto err;
> 
> -	ret = efi_set_variable_common(L"AuditMode", &efi_global_variable_guid,
> -				      attributes, sizeof(audit_mode),
> -				      &audit_mode, false);
> +	ret = efi_set_variable_int(L"AuditMode", &efi_global_variable_guid,
> +				   attributes, sizeof(audit_mode),
> +				   &audit_mode, false);
>  	if (ret != EFI_SUCCESS)
>  		goto err;
> 
> -	ret = efi_set_variable_common(L"DeployedMode",
> -				      &efi_global_variable_guid, attributes,
> -				      sizeof(deployed_mode), &deployed_mode,
> -				      false);
> +	ret = efi_set_variable_int(L"DeployedMode",
> +				   &efi_global_variable_guid, attributes,
> +				   sizeof(deployed_mode), &deployed_mode,
> +				   false);
>  err:
>  	return ret;
>  }
> @@ -234,7 +220,7 @@ err:
>   * @mode:	new state
>   *
>   * Depending on @mode, secure boot related variables are updated.
> - * Those variables are *read-only* for users, efi_set_variable_common()
> + * Those variables are *read-only* for users, efi_set_variable_int()
>   * is called here.
>   *
>   * Return:	status code
> @@ -252,10 +238,10 @@ static efi_status_t efi_transfer_secure_state(enum efi_secure_mode mode)
> 
>  		efi_secure_boot = true;
>  	} else if (mode == EFI_MODE_AUDIT) {
> -		ret = efi_set_variable_common(L"PK", &efi_global_variable_guid,
> -					      EFI_VARIABLE_BOOTSERVICE_ACCESS |
> -					      EFI_VARIABLE_RUNTIME_ACCESS,
> -					      0, NULL, false);
> +		ret = efi_set_variable_int(L"PK", &efi_global_variable_guid,
> +					   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +					   EFI_VARIABLE_RUNTIME_ACCESS,
> +					   0, NULL, false);
>  		if (ret != EFI_SUCCESS)
>  			goto err;
> 
> @@ -307,8 +293,8 @@ static efi_status_t efi_init_secure_state(void)
>  	 */
> 
>  	size = 0;
> -	ret = efi_get_variable_common(L"PK", &efi_global_variable_guid,
> -				      NULL, &size, NULL);
> +	ret = efi_get_variable_int(L"PK", &efi_global_variable_guid,
> +				   NULL, &size, NULL);
>  	if (ret == EFI_BUFFER_TOO_SMALL) {
>  		if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT))
>  			mode = EFI_MODE_USER;
> @@ -325,13 +311,13 @@ static efi_status_t efi_init_secure_state(void)
> 
>  	ret = efi_transfer_secure_state(mode);
>  	if (ret == EFI_SUCCESS)
> -		ret = efi_set_variable_common(L"VendorKeys",
> -					      &efi_global_variable_guid,
> -					      EFI_VARIABLE_BOOTSERVICE_ACCESS |
> -					      EFI_VARIABLE_RUNTIME_ACCESS |
> -					      READ_ONLY,
> -					      sizeof(efi_vendor_keys),
> -					      &efi_vendor_keys, false);
> +		ret = efi_set_variable_int(L"VendorKeys",
> +					   &efi_global_variable_guid,
> +					   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +					   EFI_VARIABLE_RUNTIME_ACCESS |
> +					   EFI_VARIABLE_READ_ONLY,
> +					   sizeof(efi_vendor_keys),
> +					   &efi_vendor_keys, false);
> 
>  err:
>  	return ret;
> @@ -593,10 +579,9 @@ static efi_status_t efi_variable_authenticate(u16 *variable,
>  }
>  #endif /* CONFIG_EFI_SECURE_BOOT */
> 
> -static efi_status_t efi_get_variable_common(u16 *variable_name,
> -					    const efi_guid_t *vendor,
> -					    u32 *attributes,
> -					    efi_uintn_t *data_size, void *data)
> +efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor,
> +				  u32 *attributes, efi_uintn_t *data_size,
> +				  void *data)
>  {
>  	char *native_name;
>  	efi_status_t ret;
> @@ -679,35 +664,6 @@ out:
>  	return ret;
>  }
> 
> -/**
> - * efi_efi_get_variable() - retrieve value of a UEFI variable
> - *
> - * This function implements the GetVariable runtime service.
> - *
> - * See the Unified Extensible Firmware Interface (UEFI) specification for
> - * details.
> - *
> - * @variable_name:	name of the variable
> - * @vendor:		vendor GUID
> - * @attributes:		attributes of the variable
> - * @data_size:		size of the buffer to which the variable value is copied
> - * @data:		buffer to which the variable value is copied
> - * Return:		status code
> - */
> -efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> -				     const efi_guid_t *vendor, u32 *attributes,
> -				     efi_uintn_t *data_size, void *data)
> -{
> -	efi_status_t ret;
> -
> -	EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes,
> -		  data_size, data);
> -
> -	ret = efi_get_variable_common(variable_name, vendor, attributes,
> -				      data_size, data);
> -	return EFI_EXIT(ret);
> -}
> -
>  static char *efi_variables_list;
>  static char *efi_cur_variable;
> 
> @@ -871,12 +827,9 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
>  	return EFI_EXIT(ret);
>  }
> 
> -static efi_status_t efi_set_variable_common(u16 *variable_name,
> -					    const efi_guid_t *vendor,
> -					    u32 attributes,
> -					    efi_uintn_t data_size,
> -					    const void *data,
> -					    bool ro_check)
> +efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
> +				  u32 attributes, efi_uintn_t data_size,
> +				  const void *data, bool ro_check)
>  {
>  	char *native_name = NULL, *old_data = NULL, *val = NULL, *s;
>  	efi_uintn_t old_size;
> @@ -899,15 +852,15 @@ static efi_status_t efi_set_variable_common(u16 *variable_name,
>  	/* check if a variable exists */
>  	old_size = 0;
>  	attr = 0;
> -	ret = efi_get_variable_common(variable_name, vendor, &attr,
> -				      &old_size, NULL);
> +	ret = efi_get_variable_int(variable_name, vendor, &attr,
> +				   &old_size, NULL);
>  	append = !!(attributes & EFI_VARIABLE_APPEND_WRITE);
>  	attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE;
>  	delete = !append && (!data_size || !attributes);
> 
>  	/* check attributes */
>  	if (old_size) {
> -		if (ro_check && (attr & READ_ONLY)) {
> +		if (ro_check && (attr & EFI_VARIABLE_READ_ONLY)) {
>  			ret = EFI_WRITE_PROTECTED;
>  			goto err;
>  		}
> @@ -915,8 +868,8 @@ static efi_status_t efi_set_variable_common(u16 *variable_name,
>  		/* attributes won't be changed */
>  		if (!delete &&
>  		    ((ro_check && attr != attributes) ||
> -		     (!ro_check && ((attr & ~(u32)READ_ONLY)
> -				    != (attributes & ~(u32)READ_ONLY))))) {
> +		     (!ro_check && ((attr & ~(u32)EFI_VARIABLE_READ_ONLY)
> +				    != (attributes & ~(u32)EFI_VARIABLE_READ_ONLY))))) {
>  			ret = EFI_INVALID_PARAMETER;
>  			goto err;
>  		}
> @@ -990,8 +943,8 @@ static efi_status_t efi_set_variable_common(u16 *variable_name,
>  			ret = EFI_OUT_OF_RESOURCES;
>  			goto err;
>  		}
> -		ret = efi_get_variable_common(variable_name, vendor,
> -					      &attr, &old_size, old_data);
> +		ret = efi_get_variable_int(variable_name, vendor,
> +					   &attr, &old_size, old_data);
>  		if (ret != EFI_SUCCESS)
>  			goto err;
>  	} else {
> @@ -1011,7 +964,7 @@ static efi_status_t efi_set_variable_common(u16 *variable_name,
>  	/*
>  	 * store attributes
>  	 */
> -	attributes &= (READ_ONLY |
> +	attributes &= (EFI_VARIABLE_READ_ONLY |
>  		       EFI_VARIABLE_NON_VOLATILE |
>  		       EFI_VARIABLE_BOOTSERVICE_ACCESS |
>  		       EFI_VARIABLE_RUNTIME_ACCESS |
> @@ -1020,7 +973,7 @@ static efi_status_t efi_set_variable_common(u16 *variable_name,
>  	while (attributes) {
>  		attr = 1 << (ffs(attributes) - 1);
> 
> -		if (attr == READ_ONLY) {
> +		if (attr == EFI_VARIABLE_READ_ONLY) {
>  			s += sprintf(s, "ro");
>  		} else if (attr == EFI_VARIABLE_NON_VOLATILE) {
>  			s += sprintf(s, "nv");
> @@ -1074,12 +1027,12 @@ out:
>  		/* update VendorKeys */
>  		if (vendor_keys_modified & efi_vendor_keys) {
>  			efi_vendor_keys = 0;
> -			ret = efi_set_variable_common(
> +			ret = efi_set_variable_int(
>  						L"VendorKeys",
>  						&efi_global_variable_guid,
>  						EFI_VARIABLE_BOOTSERVICE_ACCESS
>  						 | EFI_VARIABLE_RUNTIME_ACCESS
> -						 | READ_ONLY,
> +						 | EFI_VARIABLE_READ_ONLY,
>  						sizeof(efi_vendor_keys),
>  						&efi_vendor_keys,
>  						false);
> @@ -1096,36 +1049,6 @@ err:
>  	return ret;
>  }
> 
> -/**
> - * efi_set_variable() - set value of a UEFI variable
> - *
> - * This function implements the SetVariable runtime service.
> - *
> - * See the Unified Extensible Firmware Interface (UEFI) specification for
> - * details.
> - *
> - * @variable_name:	name of the variable
> - * @vendor:		vendor GUID
> - * @attributes:		attributes of the variable
> - * @data_size:		size of the buffer with the variable value
> - * @data:		buffer with the variable value
> - * Return:		status code
> - */
> -efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
> -				     const efi_guid_t *vendor, u32 attributes,
> -				     efi_uintn_t data_size, const void *data)
> -{
> -	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes,
> -		  data_size, data);
> -
> -	/* READ_ONLY bit is not part of API */
> -	attributes &= ~(u32)READ_ONLY;
> -
> -	return EFI_EXIT(efi_set_variable_common(variable_name, vendor,
> -						attributes, data_size, data,
> -						true));
> -}
> -
>  /**
>   * efi_query_variable_info() - get information about EFI variables
>   *
> diff --git a/lib/efi_loader/efi_variable_common.c b/lib/efi_loader/efi_variable_common.c
> new file mode 100644
> index 0000000000..e6a39fceac
> --- /dev/null
> +++ b/lib/efi_loader/efi_variable_common.c
> @@ -0,0 +1,79 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * UEFI runtime variable services
> + *
> + * Copyright (c) 2020, Heinrich Schuchardt <xypron.glpk@gmx.de>
> + */
> +
> +#include <common.h>
> +#include <efi_loader.h>
> +#include <efi_variable.h>
> +#include <linux/bitops.h>
> +
> +/**
> + * efi_efi_get_variable() - retrieve value of a UEFI variable
> + *
> + * This function implements the GetVariable runtime service.
> + *
> + * See the Unified Extensible Firmware Interface (UEFI) specification for
> + * details.
> + *
> + * @variable_name:	name of the variable
> + * @vendor:		vendor GUID
> + * @attributes:		attributes of the variable
> + * @data_size:		size of the buffer to which the variable value is copied
> + * @data:		buffer to which the variable value is copied
> + * Return:		status code
> + */
> +efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> +				     const efi_guid_t *vendor, u32 *attributes,
> +				     efi_uintn_t *data_size, void *data)
> +{
> +	efi_status_t ret;
> +
> +	EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes,
> +		  data_size, data);
> +
> +	ret = efi_get_variable_int(variable_name, vendor, attributes,
> +				   data_size, data);
> +
> +	/* Remove EFI_VARIABLE_READ_ONLY flag */
> +	if (attributes)
> +		*attributes &= EFI_VARIABLE_MASK;
> +
> +	return EFI_EXIT(ret);
> +}
> +
> +/**
> + * efi_set_variable() - set value of a UEFI variable
> + *
> + * This function implements the SetVariable runtime service.
> + *
> + * See the Unified Extensible Firmware Interface (UEFI) specification for
> + * details.
> + *
> + * @variable_name:	name of the variable
> + * @vendor:		vendor GUID
> + * @attributes:		attributes of the variable
> + * @data_size:		size of the buffer with the variable value
> + * @data:		buffer with the variable value
> + * Return:		status code
> + */
> +efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
> +				     const efi_guid_t *vendor, u32 attributes,
> +				     efi_uintn_t data_size, const void *data)
> +{
> +	efi_status_t ret;
> +
> +	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes,
> +		  data_size, data);
> +
> +	/* Make sure that the EFI_VARIABLE_READ_ONLY flag is not set */
> +	if (attributes & ~(u32)EFI_VARIABLE_MASK)
> +		ret = EFI_INVALID_PARAMETER;
> +	else
> +		ret = efi_set_variable_int(variable_name, vendor, attributes,
> +					   data_size, data, true);
> +
> +	return EFI_EXIT(ret);
> +}
> diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> index cacc76e23d..2513878c82 100644
> --- a/lib/efi_loader/efi_variable_tee.c
> +++ b/lib/efi_loader/efi_variable_tee.c
> @@ -10,6 +10,7 @@
>  #include <efi.h>
>  #include <efi_api.h>
>  #include <efi_loader.h>
> +#include <efi_variable.h>
>  #include <tee.h>
>  #include <malloc.h>
>  #include <mm_communication.h>
> @@ -243,24 +244,9 @@ out:
>  	return ret;
>  }
> 
> -/**
> - * efi_get_variable() - retrieve value of a UEFI variable
> - *
> - * This function implements the GetVariable runtime service.
> - *
> - * See the Unified Extensible Firmware Interface (UEFI) specification for
> - * details.
> - *
> - * @name:		name of the variable
> - * @guid:		vendor GUID
> - * @attr:		attributes of the variable
> - * @data_size:		size of the buffer to which the variable value is copied
> - * @data:		buffer to which the variable value is copied
> - * Return:		status code
> - */
> -efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
> -				     u32 *attr, efi_uintn_t *data_size,
> -				     void *data)
> +efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor,
> +				  u32 *attributes, efi_uintn_t *data_size,
> +				  void *data)
>  {
>  	struct smm_variable_access *var_acc;
>  	efi_uintn_t payload_size;
> @@ -269,15 +255,13 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
>  	u8 *comm_buf = NULL;
>  	efi_status_t ret;
> 
> -	EFI_ENTRY("\"%ls\" %pUl %p %p %p", name, guid, attr, data_size, data);
> -
> -	if (!name || !guid || !data_size) {
> +	if (!variable_name || !vendor || !data_size) {
>  		ret = EFI_INVALID_PARAMETER;
>  		goto out;
>  	}
> 
>  	/* Check payload size */
> -	name_size = u16_strsize(name);
> +	name_size = u16_strsize(variable_name);
>  	if (name_size > max_payload_size - MM_VARIABLE_ACCESS_HEADER_SIZE) {
>  		ret = EFI_INVALID_PARAMETER;
>  		goto out;
> @@ -300,11 +284,11 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
>  		goto out;
> 
>  	/* Fill in contents */
> -	guidcpy(&var_acc->guid, guid);
> +	guidcpy(&var_acc->guid, vendor);
>  	var_acc->data_size = tmp_dsize;
>  	var_acc->name_size = name_size;
> -	var_acc->attr = attr ? *attr : 0;
> -	memcpy(var_acc->name, name, name_size);
> +	var_acc->attr = attributes ? *attributes : 0;
> +	memcpy(var_acc->name, variable_name, name_size);
> 
>  	/* Communicate */
>  	ret = mm_communicate(comm_buf, payload_size);
> @@ -315,8 +299,8 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
>  	if (ret != EFI_SUCCESS)
>  		goto out;
> 
> -	if (attr)
> -		*attr = var_acc->attr;
> +	if (attributes)
> +		*attributes = var_acc->attr;
>  	if (data)
>  		memcpy(data, (u8 *)var_acc->name + var_acc->name_size,
>  		       var_acc->data_size);
> @@ -325,7 +309,7 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
> 
>  out:
>  	free(comm_buf);
> -	return EFI_EXIT(ret);
> +	return ret;
>  }
> 
>  /**
> @@ -417,24 +401,9 @@ out:
>  	return EFI_EXIT(ret);
>  }
> 
> -/**
> - * efi_set_variable() - set value of a UEFI variable
> - *
> - * This function implements the SetVariable runtime service.
> - *
> - * See the Unified Extensible Firmware Interface (UEFI) specification for
> - * details.
> - *
> - * @name:		name of the variable
> - * @guid:		vendor GUID
> - * @attr:		attributes of the variable
> - * @data_size:		size of the buffer with the variable value
> - * @data:		buffer with the variable value
> - * Return:		status code
> - */
> -efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
> -				     u32 attr, efi_uintn_t data_size,
> -				     const void *data)
> +efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
> +				  u32 attributes, efi_uintn_t data_size,
> +				  const void *data, bool ro_check)
>  {
>  	struct smm_variable_access *var_acc;
>  	efi_uintn_t payload_size;
> @@ -442,9 +411,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
>  	u8 *comm_buf = NULL;
>  	efi_status_t ret;
> 
> -	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", name, guid, attr, data_size, data);
> -
> -	if (!name || name[0] == 0 || !guid) {
> +	if (!variable_name || variable_name[0] == 0 || !vendor) {
>  		ret = EFI_INVALID_PARAMETER;
>  		goto out;
>  	}
> @@ -454,7 +421,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
>  	}
> 
>  	/* Check payload size */
> -	name_size = u16_strsize(name);
> +	name_size = u16_strsize(variable_name);
>  	payload_size = MM_VARIABLE_ACCESS_HEADER_SIZE + name_size + data_size;
>  	if (payload_size > max_payload_size) {
>  		ret = EFI_INVALID_PARAMETER;
> @@ -468,11 +435,11 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
>  		goto out;
> 
>  	/* Fill in contents */
> -	guidcpy(&var_acc->guid, guid);
> +	guidcpy(&var_acc->guid, vendor);
>  	var_acc->data_size = data_size;
>  	var_acc->name_size = name_size;
> -	var_acc->attr = attr;
> -	memcpy(var_acc->name, name, name_size);
> +	var_acc->attr = attributes;
> +	memcpy(var_acc->name, variable_name, name_size);
>  	memcpy((u8 *)var_acc->name + name_size, data, data_size);
> 
>  	/* Communicate */
> @@ -480,7 +447,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
> 
>  out:
>  	free(comm_buf);
> -	return EFI_EXIT(ret);
> +	return ret;
>  }
> 
>  /**
> --
> 2.27.0
> 

  reply	other threads:[~2020-06-22 23:44 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-22 16:10 [PATCH v2 1/1] efi_loader: prepare for read only OP-TEE variables Heinrich Schuchardt
2020-06-22 23:44 ` AKASHI Takahiro [this message]
2020-06-24  5:51   ` Heinrich Schuchardt
2020-06-24  6:29     ` AKASHI Takahiro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200622234409.GA26273@laputa \
    --to=takahiro.akashi@linaro.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.