* [PATCH 1/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway
2021-12-20 14:14 [PATCH 0/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway xueliang.zhong
@ 2021-12-20 14:14 ` xueliang.zhong
2021-12-20 14:14 ` [PATCH 2/4] arm-bsp/u-boot: corstone1000: Fix SCT failures xueliang.zhong
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: xueliang.zhong @ 2021-12-20 14:14 UTC (permalink / raw)
To: meta-arm, Ross.Burton; +Cc: nd, Gowtham Suresh Kumar
From: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
This patch will add a macro to configure the volatile and
non volatile storage in SMM gateway. Few useful logs are
also added to the secure world.
Change-Id: Ifdb405a09a9a72718df8b335b9f42509dd8c850c
Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
---
...-logs-to-functions-in-SMM-gateway-SP.patch | 251 ++++++++++++++++++
.../0027-Configure-storage-size.patch | 43 +++
.../trusted-services/ts-corstone1000.inc | 2 +
3 files changed, 296 insertions(+)
create mode 100644 meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0026-Add-logs-to-functions-in-SMM-gateway-SP.patch
create mode 100644 meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0027-Configure-storage-size.patch
diff --git a/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0026-Add-logs-to-functions-in-SMM-gateway-SP.patch b/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0026-Add-logs-to-functions-in-SMM-gateway-SP.patch
new file mode 100644
index 0000000..1f46586
--- /dev/null
+++ b/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0026-Add-logs-to-functions-in-SMM-gateway-SP.patch
@@ -0,0 +1,251 @@
+Upstream-Status: Pending [Not submitted to upstream yet]
+Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+
+From 9dc09450180d2d35b61359399c1313a97016ad07 Mon Sep 17 00:00:00 2001
+From: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+Date: Thu, 16 Dec 2021 13:29:58 +0000
+Subject: [PATCH] Add logs to functions in SMM gateway SP
+
+Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+
+%% original patch: 0026-Logging.patch
+---
+ .../backend/uefi_variable_store.c | 29 +++++++++++++++++--
+ .../provider/smm_variable_provider.c | 7 +++--
+ 2 files changed, 32 insertions(+), 4 deletions(-)
+
+diff --git a/components/service/smm_variable/backend/uefi_variable_store.c b/components/service/smm_variable/backend/uefi_variable_store.c
+index ed50eaf..0c371e9 100644
+--- a/components/service/smm_variable/backend/uefi_variable_store.c
++++ b/components/service/smm_variable/backend/uefi_variable_store.c
+@@ -11,6 +11,7 @@
+ #include "uefi_variable_store.h"
+ #include "variable_index_iterator.h"
+ #include "variable_checker.h"
++#include <trace.h>
+
+ /* Private functions */
+ static void load_variable_index(
+@@ -151,12 +152,15 @@ void uefi_variable_store_set_storage_limits(
+ size_t total_capacity,
+ size_t max_variable_size)
+ {
++ EMSG("In func %s\n", __func__);
+ struct delegate_variable_store *delegate_store = select_delegate_store(
+ context,
+ attributes);
+
+ delegate_store->total_capacity = total_capacity;
+ delegate_store->max_variable_size = max_variable_size;
++ EMSG("In func %s total_capacity is %d\n", __func__, total_capacity);
++ EMSG("In func %s max_variable_size is %d\n", __func__, max_variable_size);
+ }
+
+ efi_status_t uefi_variable_store_set_variable(
+@@ -265,6 +269,7 @@ efi_status_t uefi_variable_store_get_variable(
+ size_t max_data_len,
+ size_t *total_length)
+ {
++ EMSG("In func %s\n", __func__);
+ efi_status_t status = check_name_terminator(var->Name, var->NameSize);
+ if (status != EFI_SUCCESS) return status;
+
+@@ -299,6 +304,7 @@ efi_status_t uefi_variable_store_get_next_variable_name(
+ size_t max_name_len,
+ size_t *total_length)
+ {
++ EMSG("In func %s\n", __func__);
+ efi_status_t status = check_name_terminator(cur->Name, cur->NameSize);
+ if (status != EFI_SUCCESS) return status;
+
+@@ -329,6 +335,8 @@ efi_status_t uefi_variable_store_query_variable_info(
+ struct uefi_variable_store *context,
+ SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO *var_info)
+ {
++
++ EMSG("In func %s\n", __func__);
+ struct delegate_variable_store *delegate_store = select_delegate_store(
+ context,
+ var_info->Attributes);
+@@ -337,13 +345,15 @@ efi_status_t uefi_variable_store_query_variable_info(
+ context,
+ var_info->Attributes,
+ delegate_store->storage_backend);
+-
++ EMSG("In func %s total_used is %d\n", __func__, total_used);
+ var_info->MaximumVariableSize = delegate_store->max_variable_size;
+ var_info->MaximumVariableStorageSize = delegate_store->total_capacity;
+ var_info->RemainingVariableStorageSize = (total_used < delegate_store->total_capacity) ?
+ delegate_store->total_capacity - total_used :
+ 0;
+-
++ EMSG("In func %s var_info->MaximumVariableSize is %d\n", __func__, var_info->MaximumVariableSize);
++ EMSG("In func %s var_info->MaximumVariableStorageSize is %d\n", __func__, var_info->MaximumVariableStorageSize);
++ EMSG("In func %s var_info->RemainingVariableStorageSize is %d\n", __func__, var_info->RemainingVariableStorageSize);
+ return EFI_SUCCESS;
+ }
+
+@@ -358,6 +368,7 @@ efi_status_t uefi_variable_store_set_var_check_property(
+ struct uefi_variable_store *context,
+ const SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY *property)
+ {
++ EMSG("In func %s\n", __func__);
+ efi_status_t status = check_name_terminator(property->Name, property->NameSize);
+ if (status != EFI_SUCCESS) return status;
+
+@@ -404,6 +415,7 @@ efi_status_t uefi_variable_store_get_var_check_property(
+ struct uefi_variable_store *context,
+ SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY *property)
+ {
++ EMSG("In func %s\n", __func__);
+ efi_status_t status = check_name_terminator(property->Name, property->NameSize);
+ if (status != EFI_SUCCESS) return status;
+
+@@ -430,6 +442,7 @@ efi_status_t uefi_variable_store_get_var_check_property(
+ static void load_variable_index(
+ struct uefi_variable_store *context)
+ {
++ EMSG("In func %s\n", __func__);
+ struct storage_backend *persistent_store = context->persistent_store.storage_backend;
+
+ if (persistent_store) {
+@@ -444,6 +457,7 @@ static void load_variable_index(
+ context->index_sync_buffer_size,
+ context->index_sync_buffer,
+ &data_len);
++ EMSG("In func %s get status is %d\n", __func__, psa_status);
+
+ if (psa_status == PSA_SUCCESS) {
+
+@@ -455,6 +469,7 @@ static void load_variable_index(
+ static efi_status_t sync_variable_index(
+ struct uefi_variable_store *context)
+ {
++ EMSG("In func %s\n", __func__);
+ efi_status_t status = EFI_SUCCESS;
+
+ /* Sync the varibale index to storage if anything is dirty */
+@@ -479,6 +494,7 @@ static efi_status_t sync_variable_index(
+ data_len,
+ context->index_sync_buffer,
+ PSA_STORAGE_FLAG_NONE);
++ EMSG("In func %s set status is %d\n", __func__, psa_status);
+
+ status = psa_to_efi_storage_status(psa_status);
+ }
+@@ -490,6 +506,7 @@ static efi_status_t sync_variable_index(
+ static efi_status_t check_capabilities(
+ const SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var)
+ {
++ EMSG("In func %s\n", __func__);
+ efi_status_t status = EFI_SUCCESS;
+
+ /* Check if any unsupported variable attributes have been requested */
+@@ -551,6 +568,7 @@ static efi_status_t store_variable_data(
+ const struct variable_info *info,
+ const SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var)
+ {
++ EMSG("In func %s\n", __func__);
+ psa_status_t psa_status = PSA_SUCCESS;
+ size_t data_len = var->DataSize;
+ const uint8_t *data = (const uint8_t*)var +
+@@ -599,6 +617,7 @@ static efi_status_t remove_variable_data(
+ struct uefi_variable_store *context,
+ const struct variable_info *info)
+ {
++ EMSG("In func %s\n", __func__);
+ psa_status_t psa_status = PSA_SUCCESS;
+
+ if (info->is_variable_set) {
+@@ -613,6 +632,7 @@ static efi_status_t remove_variable_data(
+ delegate_store->storage_backend->context,
+ context->owner_id,
+ info->metadata.uid);
++ EMSG("In func %s status is %d\n", __func__, psa_status);
+ }
+ }
+
+@@ -625,6 +645,7 @@ static efi_status_t load_variable_data(
+ SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var,
+ size_t max_data_len)
+ {
++ EMSG("In func %s\n", __func__);
+ psa_status_t psa_status = PSA_SUCCESS;
+ size_t data_len = 0;
+ uint8_t *data = (uint8_t*)var +
+@@ -644,6 +665,7 @@ static efi_status_t load_variable_data(
+ max_data_len,
+ data,
+ &data_len);
++ EMSG("In func %s get status is %d\n", __func__, psa_status);
+
+ var->DataSize = data_len;
+ }
+@@ -771,6 +793,7 @@ static void purge_orphan_index_entries(
+ context->owner_id,
+ info->metadata.uid,
+ &storage_info);
++ EMSG("In func %s get status is %d\n", __func__, psa_status);
+
+ if (psa_status != PSA_SUCCESS) {
+
+@@ -802,6 +825,7 @@ static size_t space_used(
+ uint32_t attributes,
+ struct storage_backend *storage_backend)
+ {
++ EMSG("In func %s\n", __func__);
+ if (!storage_backend) return 0;
+
+ size_t total_used = 0;
+@@ -823,6 +847,7 @@ static size_t space_used(
+ context->owner_id,
+ info->metadata.uid,
+ &storage_info);
++ EMSG("In func %s get status is %d\n", __func__, psa_status);
+
+ if (psa_status == PSA_SUCCESS) total_used += storage_info.size;
+ }
+diff --git a/components/service/smm_variable/provider/smm_variable_provider.c b/components/service/smm_variable/provider/smm_variable_provider.c
+index 52e68d0..1f362c1 100644
+--- a/components/service/smm_variable/provider/smm_variable_provider.c
++++ b/components/service/smm_variable/provider/smm_variable_provider.c
+@@ -9,6 +9,7 @@
+ #include <protocols/service/smm_variable/smm_variable_proto.h>
+ #include <protocols/rpc/common/packed-c/status.h>
+ #include "smm_variable_provider.h"
++#include <trace.h>
+
+ /* Service request handlers */
+ static rpc_status_t get_variable_handler(void *context, struct call_req *req);
+@@ -252,17 +253,18 @@ static rpc_status_t set_variable_handler(void *context, struct call_req* req)
+
+ static rpc_status_t query_variable_info_handler(void *context, struct call_req* req)
+ {
++ EMSG("In func %s \n", __func__);
+ efi_status_t efi_status = EFI_INVALID_PARAMETER;
+ struct smm_variable_provider *this_instance = (struct smm_variable_provider*)context;
+
+ const struct call_param_buf *req_buf = call_req_get_req_buf(req);
+-
++ EMSG("In func %s sizeof(SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO) is %d\n", __func__, sizeof(SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO));
+ if (req_buf->data_len >= sizeof(SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO)) {
+
+ struct call_param_buf *resp_buf = call_req_get_resp_buf(req);
+
+ if (resp_buf->size >= req_buf->data_len) {
+-
++
+ memmove(resp_buf->data, req_buf->data, req_buf->data_len);
+
+ efi_status = uefi_variable_store_query_variable_info(
+@@ -272,6 +274,7 @@ static rpc_status_t query_variable_info_handler(void *context, struct call_req*
+ if (efi_status == EFI_SUCCESS) {
+
+ resp_buf->data_len = sizeof(SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO);
++ EMSG("In func %s resp_buf->data_len is %d\n", __func__, resp_buf->data_len);
+ }
+ }
+ else {
+--
+2.17.1
+
diff --git a/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0027-Configure-storage-size.patch b/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0027-Configure-storage-size.patch
new file mode 100644
index 0000000..764cfe6
--- /dev/null
+++ b/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0027-Configure-storage-size.patch
@@ -0,0 +1,43 @@
+Upstream-Status: Pending [Not submitted to upstream yet]
+Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+
+From 02746a26472f6aa7d57cfd5388823b0ec3c8a945 Mon Sep 17 00:00:00 2001
+From: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+Date: Thu, 16 Dec 2021 21:31:40 +0000
+Subject: [PATCH] Configure storage size
+
+---
+ .../service/smm_variable/backend/uefi_variable_store.c | 5 +++--
+ 1 file changed, 3 insertions(+), 2 deletions(-)
+
+diff --git a/components/service/smm_variable/backend/uefi_variable_store.c b/components/service/smm_variable/backend/uefi_variable_store.c
+index 0c371e9..b7cfff4 100644
+--- a/components/service/smm_variable/backend/uefi_variable_store.c
++++ b/components/service/smm_variable/backend/uefi_variable_store.c
+@@ -87,6 +87,7 @@ static efi_status_t check_name_terminator(
+ * may be overridden using uefi_variable_store_set_storage_limits()
+ */
+ #define DEFAULT_MAX_VARIABLE_SIZE (2048)
++#define CONFIGURE_STORAGE_SIZE (50)
+
+ efi_status_t uefi_variable_store_init(
+ struct uefi_variable_store *context,
+@@ -100,13 +101,13 @@ efi_status_t uefi_variable_store_init(
+ /* Initialise persistent store defaults */
+ context->persistent_store.is_nv = true;
+ context->persistent_store.max_variable_size = DEFAULT_MAX_VARIABLE_SIZE;
+- context->persistent_store.total_capacity = DEFAULT_MAX_VARIABLE_SIZE * max_variables;
++ context->persistent_store.total_capacity = CONFIGURE_STORAGE_SIZE * max_variables;
+ context->persistent_store.storage_backend = persistent_store;
+
+ /* Initialise volatile store defaults */
+ context->volatile_store.is_nv = false;
+ context->volatile_store.max_variable_size = DEFAULT_MAX_VARIABLE_SIZE;
+- context->volatile_store.total_capacity = DEFAULT_MAX_VARIABLE_SIZE * max_variables;
++ context->volatile_store.total_capacity = CONFIGURE_STORAGE_SIZE * max_variables;
+ context->volatile_store.storage_backend = volatile_store;
+
+ context->owner_id = owner_id;
+--
+2.17.1
+
diff --git a/meta-arm-bsp/recipes-security/trusted-services/ts-corstone1000.inc b/meta-arm-bsp/recipes-security/trusted-services/ts-corstone1000.inc
index bb34f54..4a18586 100644
--- a/meta-arm-bsp/recipes-security/trusted-services/ts-corstone1000.inc
+++ b/meta-arm-bsp/recipes-security/trusted-services/ts-corstone1000.inc
@@ -36,6 +36,8 @@ SRC_URI:append = " \
file://0023-add-psa-ipc-crypto-backend.patch \
file://0024-Increase-SMM-gateway-UEFI-variable-macro-value.patch \
file://0025-Add-stub-capsule-update-service-components.patch \
+ file://0026-Add-logs-to-functions-in-SMM-gateway-SP.patch \
+ file://0027-Configure-storage-size.patch \
"
SRC_URI_MBED = "git://github.com/ARMmbed/mbed-crypto.git;protocol=https;branch=development;name=mbed;destsuffix=git/mbedcrypto"
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] arm-bsp/security: Revert set append write TS patch
2021-12-20 14:14 [PATCH 0/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway xueliang.zhong
2021-12-20 14:14 ` [PATCH 1/4] " xueliang.zhong
2021-12-20 14:14 ` [PATCH 2/4] arm-bsp/u-boot: corstone1000: Fix SCT failures xueliang.zhong
@ 2021-12-20 14:14 ` xueliang.zhong
2021-12-20 14:14 ` [PATCH 4/4] arm-bsp/u-boot: populate ESRT table with image_info xueliang.zhong
2021-12-20 22:58 ` [PATCH 0/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway Jon Mason
4 siblings, 0 replies; 6+ messages in thread
From: xueliang.zhong @ 2021-12-20 14:14 UTC (permalink / raw)
To: meta-arm, Ross.Burton; +Cc: nd, Gowtham Suresh Kumar
From: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
Change-Id: I6e8aea102ea24271097efe92f4eb820c68ff70d5
Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
---
...d-uefi-variable-append-write-support.patch | 1220 +++++++++++++++++
.../trusted-services/ts-corstone1000.inc | 1 +
2 files changed, 1221 insertions(+)
create mode 100644 meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0028-Revert-Add-uefi-variable-append-write-support.patch
diff --git a/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0028-Revert-Add-uefi-variable-append-write-support.patch b/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0028-Revert-Add-uefi-variable-append-write-support.patch
new file mode 100644
index 0000000..b7efe19
--- /dev/null
+++ b/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0028-Revert-Add-uefi-variable-append-write-support.patch
@@ -0,0 +1,1220 @@
+Upstream-Status: Pending [Not submitted to upstream yet]
+Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+
+From 85df04f724f95218b57f78425966f0230d75c57e Mon Sep 17 00:00:00 2001
+From: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+Date: Wed, 15 Dec 2021 17:23:25 +0000
+Subject: [PATCH] Revert "Add uefi variable append write support"
+
+This reverts commit e8758d9aff0eddae81a74b0191cd027bcdc92c04.
+---
+ .../backend/test/variable_index_tests.cpp | 90 +++---
+ .../backend/test/variable_store_tests.cpp | 72 +----
+ .../backend/uefi_variable_store.c | 293 ++++++------------
+ .../smm_variable/backend/variable_index.c | 95 ++++--
+ .../smm_variable/backend/variable_index.h | 58 ++--
+ .../backend/variable_index_iterator.c | 4 +-
+ .../backend/variable_index_iterator.h | 2 +-
+ .../service/smm_variable_service_tests.cpp | 48 ---
+ protocols/service/smm_variable/parameters.h | 3 -
+ 9 files changed, 239 insertions(+), 426 deletions(-)
+
+diff --git a/components/service/smm_variable/backend/test/variable_index_tests.cpp b/components/service/smm_variable/backend/test/variable_index_tests.cpp
+index 8edc0e7..c8bacf9 100644
+--- a/components/service/smm_variable/backend/test/variable_index_tests.cpp
++++ b/components/service/smm_variable/backend/test/variable_index_tests.cpp
+@@ -69,37 +69,34 @@ TEST_GROUP(UefiVariableIndexTests)
+
+ void create_variables()
+ {
+- struct variable_info *info = NULL;
++ const struct variable_info *info = NULL;
+
+- info = variable_index_add_entry(
++ info = variable_index_add_variable(
+ &m_variable_index,
+ &guid_1,
+ name_1.size() * sizeof(int16_t),
+- name_1.data());
+- CHECK_TRUE(info);
+- variable_index_set_variable(
+- info,
++ name_1.data(),
+ EFI_VARIABLE_BOOTSERVICE_ACCESS);
+
+- info = variable_index_add_entry(
++ CHECK_TRUE(info);
++
++ info = variable_index_add_variable(
+ &m_variable_index,
+ &guid_2,
+ name_2.size() * sizeof(int16_t),
+- name_2.data());
+- CHECK_TRUE(info);
+- variable_index_set_variable(
+- info,
++ name_2.data(),
+ EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS);
+
+- info = variable_index_add_entry(
++ CHECK_TRUE(info);
++
++ info = variable_index_add_variable(
+ &m_variable_index,
+ &guid_1,
+ name_3.size() * sizeof(int16_t),
+- name_3.data());
+- CHECK_TRUE(info);
+- variable_index_set_variable(
+- info,
++ name_3.data(),
+ EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS);
++
++ CHECK_TRUE(info);
+ }
+
+ static const size_t MAX_VARIABLES = 10;
+@@ -114,7 +111,7 @@ TEST_GROUP(UefiVariableIndexTests)
+
+ TEST(UefiVariableIndexTests, emptyIndexOperations)
+ {
+- struct variable_info *info = NULL;
++ const struct variable_info *info = NULL;
+
+ /* Expect not to find a variable */
+ info = variable_index_find(
+@@ -133,34 +130,36 @@ TEST(UefiVariableIndexTests, emptyIndexOperations)
+ POINTERS_EQUAL(NULL, info);
+
+ /* Remove should silently return */
+- variable_index_clear_variable(
++ variable_index_remove_variable(
+ &m_variable_index,
+ info);
+ }
+
+ TEST(UefiVariableIndexTests, addWithOversizedName)
+ {
+- struct variable_info *info = NULL;
++ const struct variable_info *info = NULL;
+ std::vector<int16_t> name;
+
+ name = to_variable_name(L"a long variable name that exceeds the length limit");
+
+- info = variable_index_add_entry(
++ info = variable_index_add_variable(
+ &m_variable_index,
+ &guid_1,
+ name.size() * sizeof(int16_t),
+- name.data());
++ name.data(),
++ EFI_VARIABLE_BOOTSERVICE_ACCESS);
+
+ /* Expect the add to fail because of an oversized name */
+ POINTERS_EQUAL(NULL, info);
+
+ name = to_variable_name(L"a long variable name that fits!");
+
+- info = variable_index_add_entry(
++ info = variable_index_add_variable(
+ &m_variable_index,
+ &guid_1,
+ name.size() * sizeof(int16_t),
+- name.data());
++ name.data(),
++ EFI_VARIABLE_BOOTSERVICE_ACCESS);
+
+ /* Expect the add succeed */
+ CHECK_TRUE(info);
+@@ -168,17 +167,18 @@ TEST(UefiVariableIndexTests, addWithOversizedName)
+
+ TEST(UefiVariableIndexTests, variableIndexFull)
+ {
+- struct variable_info *info = NULL;
++ const struct variable_info *info = NULL;
+ EFI_GUID guid = guid_1;
+
+ /* Expect to be able to fill the index */
+ for (size_t i = 0; i < MAX_VARIABLES; ++i) {
+
+- info = variable_index_add_entry(
++ info = variable_index_add_variable(
+ &m_variable_index,
+ &guid,
+ name_1.size() * sizeof(int16_t),
+- name_1.data());
++ name_1.data(),
++ EFI_VARIABLE_BOOTSERVICE_ACCESS);
+
+ CHECK_TRUE(info);
+
+@@ -187,11 +187,12 @@ TEST(UefiVariableIndexTests, variableIndexFull)
+ }
+
+ /* Variable index should now be full */
+- info = variable_index_add_entry(
++ info = variable_index_add_variable(
+ &m_variable_index,
+ &guid,
+ name_1.size() * sizeof(int16_t),
+- name_1.data());
++ name_1.data(),
++ EFI_VARIABLE_BOOTSERVICE_ACCESS);
+
+ POINTERS_EQUAL(NULL, info);
+ }
+@@ -322,7 +323,7 @@ TEST(UefiVariableIndexTests, dumpBufferTooSmall)
+ TEST(UefiVariableIndexTests, removeVariable)
+ {
+ uint8_t buffer[MAX_VARIABLES * sizeof(struct variable_metadata)];
+- struct variable_info *info = NULL;
++ const struct variable_info *info = NULL;
+
+ create_variables();
+
+@@ -333,7 +334,7 @@ TEST(UefiVariableIndexTests, removeVariable)
+ name_2.size() * sizeof(int16_t),
+ name_2.data());
+
+- variable_index_clear_variable(
++ variable_index_remove_variable(
+ &m_variable_index,
+ info);
+
+@@ -351,7 +352,7 @@ TEST(UefiVariableIndexTests, removeVariable)
+ name_1.size() * sizeof(int16_t),
+ name_1.data());
+
+- variable_index_clear_variable(
++ variable_index_remove_variable(
+ &m_variable_index,
+ info);
+
+@@ -369,7 +370,7 @@ TEST(UefiVariableIndexTests, removeVariable)
+ name_3.size() * sizeof(int16_t),
+ name_3.data());
+
+- variable_index_clear_variable(
++ variable_index_remove_variable(
+ &m_variable_index,
+ info);
+
+@@ -394,7 +395,7 @@ TEST(UefiVariableIndexTests, removeVariable)
+
+ TEST(UefiVariableIndexTests, checkIterator)
+ {
+- struct variable_info *info = NULL;
++ const struct variable_info *info = NULL;
+
+ create_variables();
+
+@@ -418,7 +419,7 @@ TEST(UefiVariableIndexTests, checkIterator)
+ UNSIGNED_LONGS_EQUAL(name_2.size() * sizeof(int16_t), info->metadata.name_size);
+ MEMCMP_EQUAL(name_2.data(), info->metadata.name, info->metadata.name_size);
+
+- struct variable_info *info_to_remove = info;
++ const struct variable_info *info_to_remove = info;
+
+ variable_index_iterator_next(&iter);
+ CHECK_FALSE(variable_index_iterator_is_done(&iter));
+@@ -434,8 +435,7 @@ TEST(UefiVariableIndexTests, checkIterator)
+ CHECK_TRUE(variable_index_iterator_is_done(&iter));
+
+ /* Now remove the middle entry */
+- variable_index_clear_variable(&m_variable_index, info_to_remove);
+- variable_index_remove_unused_entry(&m_variable_index, info_to_remove);
++ variable_index_remove_variable(&m_variable_index, info_to_remove);
+
+ /* Iterate again but this time there should only be two entries */
+ variable_index_iterator_first(&iter, &m_variable_index);
+@@ -478,7 +478,7 @@ TEST(UefiVariableIndexTests, setCheckConstraintsExistingVar)
+ constraints.max_size = 100;
+
+ /* Set check constraints on one of the variables */
+- struct variable_info *info = variable_index_find(
++ const struct variable_info *info = variable_index_find(
+ &m_variable_index,
+ &guid_2,
+ name_2.size() * sizeof(int16_t),
+@@ -488,7 +488,7 @@ TEST(UefiVariableIndexTests, setCheckConstraintsExistingVar)
+ CHECK_TRUE(info->is_variable_set);
+ CHECK_FALSE(info->is_constraints_set);
+
+- variable_index_set_constraints(info, &constraints);
++ variable_index_update_constraints(info, &constraints);
+
+ CHECK_TRUE(info->is_constraints_set);
+ CHECK_TRUE(info->is_variable_set);
+@@ -496,7 +496,7 @@ TEST(UefiVariableIndexTests, setCheckConstraintsExistingVar)
+ /* Remove the variable but still expect the variable to be indexed
+ * because of the set constraints.
+ */
+- variable_index_clear_variable(
++ variable_index_remove_variable(
+ &m_variable_index,
+ info);
+
+@@ -588,7 +588,7 @@ TEST(UefiVariableIndexTests, setCheckConstraintsNonExistingVar)
+ constraints.max_size = 100;
+
+ /* Initially expect no variable_info */
+- struct variable_info *info = variable_index_find(
++ const struct variable_info *info = variable_index_find(
+ &m_variable_index,
+ &guid_2,
+ name_2.size() * sizeof(int16_t),
+@@ -597,19 +597,19 @@ TEST(UefiVariableIndexTests, setCheckConstraintsNonExistingVar)
+ CHECK_FALSE(info);
+
+ /* Adding the check constraints should result in an entry being added */
+- info = variable_index_add_entry(
++ info = variable_index_add_constraints(
+ &m_variable_index,
+ &guid_2,
+ name_2.size() * sizeof(int16_t),
+- name_2.data());
+- CHECK_TRUE(info);
++ name_2.data(),
++ &constraints);
+
+- variable_index_set_constraints(info, &constraints);
++ CHECK_TRUE(info);
+ CHECK_FALSE(info->is_variable_set);
+ CHECK_TRUE(info->is_constraints_set);
+
+ /* Updating the variable should cause the variable to be marked as set */
+- variable_index_set_variable(info, EFI_VARIABLE_RUNTIME_ACCESS);
++ variable_index_update_variable(info, EFI_VARIABLE_RUNTIME_ACCESS);
+
+ CHECK_TRUE(info->is_variable_set);
+ CHECK_TRUE(info->is_constraints_set);
+diff --git a/components/service/smm_variable/backend/test/variable_store_tests.cpp b/components/service/smm_variable/backend/test/variable_store_tests.cpp
+index e90c106..235642e 100644
+--- a/components/service/smm_variable/backend/test/variable_store_tests.cpp
++++ b/components/service/smm_variable/backend/test/variable_store_tests.cpp
+@@ -305,37 +305,6 @@ TEST(UefiVariableStoreTests, setGetRoundtrip)
+ /* Expect got variable data to be the same as the set value */
+ UNSIGNED_LONGLONGS_EQUAL(input_data.size(), output_data.size());
+ LONGS_EQUAL(0, input_data.compare(output_data));
+-
+- /* Extend the variable using an append write */
+- std::string input_data2 = " jumps over the lazy dog";
+-
+- status = set_variable(var_name, input_data2, EFI_VARIABLE_APPEND_WRITE);
+- UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+-
+- status = get_variable(var_name, output_data);
+- UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+-
+- std::string expected_output = input_data + input_data2;
+-
+- /* Expect the append write operation to have extended the variable */
+- UNSIGNED_LONGLONGS_EQUAL(expected_output.size(), output_data.size());
+- LONGS_EQUAL(0, expected_output.compare(output_data));
+-
+- /* Expect query_variable_info to return consistent values */
+- size_t max_variable_storage_size = 0;
+- size_t remaining_variable_storage_size = 0;
+- size_t max_variable_size = 0;
+-
+- status = query_variable_info(
+- 0,
+- &max_variable_storage_size,
+- &remaining_variable_storage_size,
+- &max_variable_size);
+- UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+-
+- UNSIGNED_LONGLONGS_EQUAL(STORE_CAPACITY, max_variable_storage_size);
+- UNSIGNED_LONGLONGS_EQUAL(MAX_VARIABLE_SIZE, max_variable_size);
+- UNSIGNED_LONGLONGS_EQUAL(STORE_CAPACITY - expected_output.size(), remaining_variable_storage_size);
+ }
+
+ TEST(UefiVariableStoreTests, persistentSetGet)
+@@ -345,8 +314,7 @@ TEST(UefiVariableStoreTests, persistentSetGet)
+ std::string input_data = "quick brown fox";
+ std::string output_data;
+
+- status = set_variable(var_name, input_data,
+- EFI_VARIABLE_NON_VOLATILE);
++ status = set_variable(var_name, input_data, EFI_VARIABLE_NON_VOLATILE);
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+
+ status = get_variable(var_name, output_data);
+@@ -356,22 +324,6 @@ TEST(UefiVariableStoreTests, persistentSetGet)
+ UNSIGNED_LONGLONGS_EQUAL(input_data.size(), output_data.size());
+ LONGS_EQUAL(0, input_data.compare(output_data));
+
+- /* Extend the variable using an append write */
+- std::string input_data2 = " jumps over the lazy dog";
+-
+- status = set_variable(var_name, input_data2,
+- EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_APPEND_WRITE);
+- UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+-
+- status = get_variable(var_name, output_data);
+- UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+-
+- std::string expected_output = input_data + input_data2;
+-
+- /* Expect the append write operation to have extended the variable */
+- UNSIGNED_LONGLONGS_EQUAL(expected_output.size(), output_data.size());
+- LONGS_EQUAL(0, expected_output.compare(output_data));
+-
+ /* Expect the variable to survive a power cycle */
+ power_cycle();
+
+@@ -380,24 +332,8 @@ TEST(UefiVariableStoreTests, persistentSetGet)
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+
+ /* Still expect got variable data to be the same as the set value */
+- UNSIGNED_LONGLONGS_EQUAL(expected_output.size(), output_data.size());
+- LONGS_EQUAL(0, expected_output.compare(output_data));
+-
+- /* Expect query_variable_info to return consistent values */
+- size_t max_variable_storage_size = 0;
+- size_t remaining_variable_storage_size = 0;
+- size_t max_variable_size = 0;
+-
+- status = query_variable_info(
+- EFI_VARIABLE_NON_VOLATILE,
+- &max_variable_storage_size,
+- &remaining_variable_storage_size,
+- &max_variable_size);
+- UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+-
+- UNSIGNED_LONGLONGS_EQUAL(STORE_CAPACITY, max_variable_storage_size);
+- UNSIGNED_LONGLONGS_EQUAL(MAX_VARIABLE_SIZE, max_variable_size);
+- UNSIGNED_LONGLONGS_EQUAL(STORE_CAPACITY - expected_output.size(), remaining_variable_storage_size);
++ UNSIGNED_LONGLONGS_EQUAL(input_data.size(), output_data.size());
++ LONGS_EQUAL(0, input_data.compare(output_data));
+ }
+
+ TEST(UefiVariableStoreTests, removeVolatile)
+@@ -436,7 +372,7 @@ TEST(UefiVariableStoreTests, removePersistent)
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+
+ /* Remove by setting with zero data length */
+- status = set_variable(var_name, std::string(), EFI_VARIABLE_NON_VOLATILE);
++ status = set_variable(var_name, std::string(), 0);
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+
+ /* Expect variable to no loger exist */
+diff --git a/components/service/smm_variable/backend/uefi_variable_store.c b/components/service/smm_variable/backend/uefi_variable_store.c
+index ed50eaf..d084e8d 100644
+--- a/components/service/smm_variable/backend/uefi_variable_store.c
++++ b/components/service/smm_variable/backend/uefi_variable_store.c
+@@ -46,20 +46,6 @@ static efi_status_t load_variable_data(
+ SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var,
+ size_t max_data_len);
+
+-static psa_status_t store_overwrite(
+- struct delegate_variable_store *delegate_store,
+- uint32_t client_id,
+- uint64_t uid,
+- size_t data_length,
+- const void *data);
+-
+-static psa_status_t store_append_write(
+- struct delegate_variable_store *delegate_store,
+- uint32_t client_id,
+- uint64_t uid,
+- size_t data_length,
+- const void *data);
+-
+ static void purge_orphan_index_entries(
+ struct uefi_variable_store *context);
+
+@@ -163,45 +149,40 @@ efi_status_t uefi_variable_store_set_variable(
+ struct uefi_variable_store *context,
+ const SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var)
+ {
+- bool should_sync_index = false;
+-
+- /* Validate incoming request */
+ efi_status_t status = check_name_terminator(var->Name, var->NameSize);
+ if (status != EFI_SUCCESS) return status;
+
+ status = check_capabilities(var);
++ bool should_sync_index = false;
++
+ if (status != EFI_SUCCESS) return status;
+
+- /* Find an existing entry in the variable index or add a new one */
+- struct variable_info *info = variable_index_find(
++ /* Find in index */
++ const struct variable_info *info = variable_index_find(
+ &context->variable_index,
+ &var->Guid,
+ var->NameSize,
+ var->Name);
+
+- if (!info) {
++ if (info) {
+
+- info = variable_index_add_entry(
+- &context->variable_index,
+- &var->Guid,
+- var->NameSize,
+- var->Name);
++ /* Variable info already exists */
++ status = check_access_permitted_on_set(context, info, var);
+
+- if (!info) return EFI_OUT_OF_RESOURCES;
+- }
++ if (status == EFI_SUCCESS) {
+
+- /* Control access */
+- status = check_access_permitted_on_set(context, info, var);
++ should_sync_index =
++ (var->Attributes & EFI_VARIABLE_NON_VOLATILE) ||
++ (info->is_variable_set && (info->metadata.attributes & EFI_VARIABLE_NON_VOLATILE));
+
+- if (status == EFI_SUCCESS) {
++ if (var->DataSize) {
+
+- /* Access permitted */
+- if (info->is_variable_set) {
+-
+- /* It's a request to update to an existing variable */
+- if (!(var->Attributes &
+- (EFI_VARIABLE_APPEND_WRITE | EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS_MASK)) &&
+- !var->DataSize) {
++ /* It's a set rather than a remove operation */
++ variable_index_update_variable(
++ info,
++ var->Attributes);
++ }
++ else {
+
+ /* It's a remove operation - for a remove, the variable
+ * data must be removed from the storage backend before
+@@ -210,30 +191,31 @@ efi_status_t uefi_variable_store_set_variable(
+ * the storage backend without a corresponding index entry.
+ */
+ remove_variable_data(context, info);
+- variable_index_clear_variable(&context->variable_index, info);
++ variable_index_remove_variable(&context->variable_index, info);
+
+- should_sync_index = (var->Attributes & EFI_VARIABLE_NON_VOLATILE);
+- }
+- else {
+-
+- /* It's a set operation where variable data is potentially
+- * being overwritten or extended.
+- */
+- if ((var->Attributes & ~EFI_VARIABLE_APPEND_WRITE) != info->metadata.attributes) {
+-
+- /* Modifying attributes is forbidden */
+- return EFI_INVALID_PARAMETER;
+- }
++ /* Variable info no longer valid */
++ info = NULL;
+ }
+ }
+ else {
+
+- /* It's a request to create a new variable */
+- variable_index_set_variable(info, var->Attributes);
+-
+- should_sync_index = (var->Attributes & EFI_VARIABLE_NON_VOLATILE);
++ /* Access forbidden */
++ info = NULL;
+ }
+ }
++ else if (var->DataSize) {
++
++ /* It's a new variable */
++ info = variable_index_add_variable(
++ &context->variable_index,
++ &var->Guid,
++ var->NameSize,
++ var->Name,
++ var->Attributes);
++
++ if (!info) status = EFI_OUT_OF_RESOURCES;
++ should_sync_index = info && (info->metadata.attributes & EFI_VARIABLE_NON_VOLATILE);
++ }
+
+ /* The order of these operations is important. For an update
+ * or create operation, The variable index is always synchronized
+@@ -249,13 +231,11 @@ efi_status_t uefi_variable_store_set_variable(
+ }
+
+ /* Store any variable data to the storage backend */
+- if (info->is_variable_set && (status == EFI_SUCCESS)) {
++ if (info && (status == EFI_SUCCESS)) {
+
+ status = store_variable_data(context, info, var);
+ }
+
+- variable_index_remove_unused_entry(&context->variable_index, info);
+-
+ return status;
+ }
+
+@@ -361,41 +341,53 @@ efi_status_t uefi_variable_store_set_var_check_property(
+ efi_status_t status = check_name_terminator(property->Name, property->NameSize);
+ if (status != EFI_SUCCESS) return status;
+
+- /* Find in index or create a new entry */
+- struct variable_info *info = variable_index_find(
++ /* Find in index */
++ const struct variable_info *info = variable_index_find(
+ &context->variable_index,
+ &property->Guid,
+ property->NameSize,
+ property->Name);
+
+- if (!info) {
++ if (info) {
+
+- info = variable_index_add_entry(
+- &context->variable_index,
+- &property->Guid,
+- property->NameSize,
+- property->Name);
++ /* Applying check constraints to an existing variable that may have
++ * constraints already set. These could constrain the setting of
++ * the constraints.
++ */
++ struct variable_constraints constraints = info->check_constraints;
++
++ status = variable_checker_set_constraints(
++ &constraints,
++ info->is_constraints_set,
++ &property->VariableProperty);
++
++ if (status == EFI_SUCCESS) {
+
+- if (!info) return EFI_OUT_OF_RESOURCES;
++ variable_index_update_constraints(info, &constraints);
++ }
+ }
++ else {
+
+- /* Applying check constraints to an existing variable that may have
+- * constraints already set. These could constrain the setting of
+- * the constraints.
+- */
+- struct variable_constraints constraints = info->check_constraints;
++ /* Applying check constraints for a new variable */
++ struct variable_constraints constraints;
+
+- status = variable_checker_set_constraints(
+- &constraints,
+- info->is_constraints_set,
+- &property->VariableProperty);
++ status = variable_checker_set_constraints(
++ &constraints,
++ false,
++ &property->VariableProperty);
+
+- if (status == EFI_SUCCESS) {
++ if (status == EFI_SUCCESS) {
+
+- variable_index_set_constraints(info, &constraints);
+- }
++ info = variable_index_add_constraints(
++ &context->variable_index,
++ &property->Guid,
++ property->NameSize,
++ property->Name,
++ &constraints);
+
+- variable_index_remove_unused_entry(&context->variable_index, info);
++ if (!info) status = EFI_OUT_OF_RESOURCES;
++ }
++ }
+
+ return status;
+ }
+@@ -496,8 +488,7 @@ static efi_status_t check_capabilities(
+ if (var->Attributes & ~(
+ EFI_VARIABLE_NON_VOLATILE |
+ EFI_VARIABLE_BOOTSERVICE_ACCESS |
+- EFI_VARIABLE_RUNTIME_ACCESS |
+- EFI_VARIABLE_APPEND_WRITE)) {
++ EFI_VARIABLE_RUNTIME_ACCESS)) {
+
+ /* An unsupported attribute has been requested */
+ status = EFI_UNSUPPORTED;
+@@ -543,6 +534,17 @@ static efi_status_t check_access_permitted_on_set(
+ var->DataSize);
+ }
+
++ if ((status == EFI_SUCCESS) && var->DataSize) {
++
++ /* Restrict which attributes can be modified for an existing variable */
++ if ((var->Attributes & EFI_VARIABLE_NON_VOLATILE) !=
++ (info->metadata.attributes & EFI_VARIABLE_NON_VOLATILE)) {
++
++ /* Don't permit change of storage class */
++ status = EFI_INVALID_PARAMETER;
++ }
++ }
++
+ return status;
+ }
+
+@@ -562,33 +564,20 @@ static efi_status_t store_variable_data(
+
+ if (delegate_store->storage_backend) {
+
+- if (!(var->Attributes & EFI_VARIABLE_APPEND_WRITE)) {
+-
+- /* Create or overwrite variable data */
+- psa_status = store_overwrite(
+- delegate_store,
+- context->owner_id,
+- info->metadata.uid,
+- data_len,
+- data);
+- }
+- else {
+-
+- /* Append new data to existing variable data */
+- psa_status = store_append_write(
+- delegate_store,
+- context->owner_id,
+- info->metadata.uid,
+- data_len,
+- data);
+- }
++ psa_status = delegate_store->storage_backend->interface->set(
++ delegate_store->storage_backend->context,
++ context->owner_id,
++ info->metadata.uid,
++ data_len,
++ data,
++ PSA_STORAGE_FLAG_NONE);
+ }
+
+ if ((psa_status != PSA_SUCCESS) && delegate_store->is_nv) {
+
+ /* A storage failure has occurred so attempt to fix any
+- * mismatch between the variable index and stored NV variables.
+- */
++ * mismatch between the variable index and stored NV variables.
++ */
+ purge_orphan_index_entries(context);
+ }
+
+@@ -651,100 +640,6 @@ static efi_status_t load_variable_data(
+ return psa_to_efi_storage_status(psa_status);
+ }
+
+-static psa_status_t store_overwrite(
+- struct delegate_variable_store *delegate_store,
+- uint32_t client_id,
+- uint64_t uid,
+- size_t data_length,
+- const void *data)
+-{
+- /* Police maximum variable size limit */
+- if (data_length > delegate_store->max_variable_size) return PSA_ERROR_INVALID_ARGUMENT;
+-
+- psa_status_t psa_status = delegate_store->storage_backend->interface->set(
+- delegate_store->storage_backend->context,
+- client_id,
+- uid,
+- data_length,
+- data,
+- PSA_STORAGE_FLAG_NONE);
+-
+- return psa_status;
+-}
+-
+-static psa_status_t store_append_write(
+- struct delegate_variable_store *delegate_store,
+- uint32_t client_id,
+- uint64_t uid,
+- size_t data_length,
+- const void *data)
+-{
+- struct psa_storage_info_t storage_info;
+-
+- if (data_length == 0) return PSA_SUCCESS;
+-
+- psa_status_t psa_status = delegate_store->storage_backend->interface->get_info(
+- delegate_store->storage_backend->context,
+- client_id,
+- uid,
+- &storage_info);
+-
+- if (psa_status != PSA_SUCCESS) return psa_status;
+-
+- /* Determine size of appended variable */
+- size_t new_size = storage_info.size + data_length;
+-
+- /* Defend against integer overflow */
+- if (new_size < storage_info.size) return PSA_ERROR_INVALID_ARGUMENT;
+-
+- /* Police maximum variable size limit */
+- if (new_size > delegate_store->max_variable_size) return PSA_ERROR_INVALID_ARGUMENT;
+-
+- /* Storage backend doesn't support an append operation so we need
+- * need to read the current variable data, extend it and write it back.
+- */
+- uint8_t *rw_buf = malloc(new_size);
+- if (!rw_buf) return PSA_ERROR_INSUFFICIENT_MEMORY;
+-
+- size_t old_size = 0;
+- psa_status = delegate_store->storage_backend->interface->get(
+- delegate_store->storage_backend->context,
+- client_id,
+- uid,
+- 0,
+- new_size,
+- rw_buf,
+- &old_size);
+-
+- if (psa_status == PSA_SUCCESS) {
+-
+- if ((old_size + data_length) <= new_size) {
+-
+- /* Extend the variable data */
+- memcpy(&rw_buf[old_size], data, data_length);
+-
+- psa_status = delegate_store->storage_backend->interface->set(
+- delegate_store->storage_backend->context,
+- client_id,
+- uid,
+- old_size + data_length,
+- rw_buf,
+- storage_info.flags);
+- }
+- else {
+-
+- /* There's a mismatch between the length obtained from
+- * get_info() and the subsequent length returned by get().
+- */
+- psa_status = PSA_ERROR_STORAGE_FAILURE;
+- }
+- }
+-
+- free(rw_buf);
+-
+- return psa_status;
+-}
+-
+ static void purge_orphan_index_entries(
+ struct uefi_variable_store *context)
+ {
+@@ -759,7 +654,7 @@ static void purge_orphan_index_entries(
+ */
+ while (!variable_index_iterator_is_done(&iter)) {
+
+- struct variable_info *info = variable_index_iterator_current(&iter);
++ const struct variable_info *info = variable_index_iterator_current(&iter);
+
+ if (info->is_variable_set && (info->metadata.attributes & EFI_VARIABLE_NON_VOLATILE)) {
+
+@@ -775,7 +670,7 @@ static void purge_orphan_index_entries(
+ if (psa_status != PSA_SUCCESS) {
+
+ /* Detected a mismatch between the index and storage */
+- variable_index_clear_variable(&context->variable_index, info);
++ variable_index_remove_variable(&context->variable_index, info);
+ any_orphans = true;
+ }
+ }
+diff --git a/components/service/smm_variable/backend/variable_index.c b/components/service/smm_variable/backend/variable_index.c
+index a8a5575..99d7c97 100644
+--- a/components/service/smm_variable/backend/variable_index.c
++++ b/components/service/smm_variable/backend/variable_index.c
+@@ -132,13 +132,13 @@ size_t variable_index_max_dump_size(
+ return sizeof(struct variable_metadata) * context->max_variables;
+ }
+
+-struct variable_info *variable_index_find(
+- struct variable_index *context,
++const struct variable_info *variable_index_find(
++ const struct variable_index *context,
+ const EFI_GUID *guid,
+ size_t name_size,
+ const int16_t *name)
+ {
+- struct variable_info *result = NULL;
++ const struct variable_info *result = NULL;
+ int pos = find_variable(context, guid, name_size, name);
+
+ if (pos >= 0) {
+@@ -149,13 +149,13 @@ struct variable_info *variable_index_find(
+ return result;
+ }
+
+-struct variable_info *variable_index_find_next(
++const struct variable_info *variable_index_find_next(
+ const struct variable_index *context,
+ const EFI_GUID *guid,
+ size_t name_size,
+ const int16_t *name)
+ {
+- struct variable_info *result = NULL;
++ const struct variable_info *result = NULL;
+
+ if (name_size >= sizeof(int16_t)) {
+
+@@ -263,11 +263,12 @@ static struct variable_entry *add_entry(
+ return entry;
+ }
+
+-struct variable_info *variable_index_add_entry(
++const struct variable_info *variable_index_add_variable(
+ struct variable_index *context,
+ const EFI_GUID *guid,
+ size_t name_size,
+- const int16_t *name)
++ const int16_t *name,
++ uint32_t attributes)
+ {
+ struct variable_info *info = NULL;
+ struct variable_entry *entry = add_entry(context, guid, name_size, name);
+@@ -275,41 +276,40 @@ struct variable_info *variable_index_add_entry(
+ if (entry) {
+
+ info = &entry->info;
++
++ info->metadata.attributes = attributes;
++ info->is_variable_set = true;
++
++ mark_dirty(entry);
+ }
+
+ return info;
+ }
+
+-void variable_index_remove_unused_entry(
++const struct variable_info *variable_index_add_constraints(
+ struct variable_index *context,
+- struct variable_info *info)
++ const EFI_GUID *guid,
++ size_t name_size,
++ const int16_t *name,
++ const struct variable_constraints *constraints)
+ {
+- if (info &&
+- !info->is_constraints_set &&
+- !info->is_variable_set) {
+-
+- struct variable_entry *entry = containing_entry(info);
+- entry->in_use = false;
++ struct variable_info *info = NULL;
++ struct variable_entry *entry = add_entry(context, guid, name_size, name);
+
+- memset(info, 0, sizeof(struct variable_info));
+- }
+-}
++ if (entry) {
+
+-void variable_index_set_variable(
+- struct variable_info *info,
+- uint32_t attributes)
+-{
+- struct variable_entry *entry = containing_entry(info);
++ info = &entry->info;
+
+- info->metadata.attributes = attributes;
+- info->is_variable_set = true;
++ info->check_constraints = *constraints;
++ info->is_constraints_set = true;
++ }
+
+- mark_dirty(entry);
++ return info;
+ }
+
+-void variable_index_clear_variable(
++void variable_index_remove_variable(
+ struct variable_index *context,
+- struct variable_info *info)
++ const struct variable_info *info)
+ {
+ if (info) {
+
+@@ -318,17 +318,48 @@ void variable_index_clear_variable(
+
+ /* Mark variable as no longer set */
+ entry->info.is_variable_set = false;
++
++ /* Entry may still be needed if check constraints were set */
++ entry->in_use = info->is_constraints_set;
++
++ if (!entry->in_use) {
++
++ /* Entry not needed so wipe */
++ memset(&entry->info, 0, sizeof(struct variable_info));
++ }
+ }
+ }
+
+-void variable_index_set_constraints(
+- struct variable_info *info,
++void variable_index_update_variable(
++ const struct variable_info *info,
++ uint32_t attributes)
++{
++ if (info) {
++
++ struct variable_info *modified_info = (struct variable_info*)info;
++ struct variable_entry *entry = containing_entry(modified_info);
++
++ if (!modified_info->is_variable_set ||
++ (attributes != modified_info->metadata.attributes)) {
++
++ /* The update changes the variable_info state */
++ modified_info->is_variable_set = true;
++ modified_info->metadata.attributes = attributes;
++ mark_dirty(entry);
++ }
++ }
++}
++
++void variable_index_update_constraints(
++ const struct variable_info *info,
+ const struct variable_constraints *constraints)
+ {
+ if (info) {
+
+- info->check_constraints = *constraints;
+- info->is_constraints_set = true;
++ struct variable_info *modified_info = (struct variable_info*)info;
++
++ modified_info->check_constraints = *constraints;
++ modified_info->is_constraints_set = true;
+ }
+ }
+
+diff --git a/components/service/smm_variable/backend/variable_index.h b/components/service/smm_variable/backend/variable_index.h
+index 63f42ab..e109d0d 100644
+--- a/components/service/smm_variable/backend/variable_index.h
++++ b/components/service/smm_variable/backend/variable_index.h
+@@ -119,8 +119,8 @@ size_t variable_index_max_dump_size(
+ *
+ * @return Pointer to variable_info or NULL
+ */
+-struct variable_info *variable_index_find(
+- struct variable_index *context,
++const struct variable_info *variable_index_find(
++ const struct variable_index *context,
+ const EFI_GUID *guid,
+ size_t name_size,
+ const int16_t *name);
+@@ -135,76 +135,78 @@ struct variable_info *variable_index_find(
+ *
+ * @return Pointer to variable_info or NULL
+ */
+-struct variable_info *variable_index_find_next(
++const struct variable_info *variable_index_find_next(
+ const struct variable_index *context,
+ const EFI_GUID *guid,
+ size_t name_size,
+ const int16_t *name);
+
+ /**
+- * @brief Add a new entry to the index
+- *
+- * An entry is needed either when a new variable is created or
+- * when variable constraints are set for a variable that doesn't
+- * yet exist.
++ * @brief Add a new variable to the index
+ *
+ * @param[in] context variable_index
+ * @param[in] guid The variable's guid
+ * @param[in] name_size The name parameter's size
+ * @param[in] name The variable's name
++ * @param[in] attributes The variable's attributes
+ *
+ * @return Pointer to variable_info or NULL
+ */
+-struct variable_info *variable_index_add_entry(
++const struct variable_info *variable_index_add_variable(
+ struct variable_index *context,
+ const EFI_GUID *guid,
+ size_t name_size,
+- const int16_t *name);
++ const int16_t *name,
++ uint32_t attributes);
+
+ /**
+- * @brief Remove an unused entry from the index
++ * @brief Remove a variable from the index
+ *
+- * Removes an entry if it is not in use.
++ * Removes a variable from the index if it exists.
+ *
+ * @param[in] context variable_index
+ * @param[in] info The variable info corresponding to the entry to remove
+ */
+-void variable_index_remove_unused_entry(
++void variable_index_remove_variable(
+ struct variable_index *context,
+- struct variable_info *info);
++ const struct variable_info *info);
+
+ /**
+- * @brief Set a variable to the index
+- *
+- * An entry for the variable must already exist.
++ * @brief Update a variable that's already in the index
+ *
+ * @param[in] info variable info
+ * @param[in] attributes The variable's attributes
+ */
+-void variable_index_set_variable(
+- struct variable_info *info,
++void variable_index_update_variable(
++ const struct variable_info *info,
+ uint32_t attributes);
+
+ /**
+- * @brief Clear a variable from the index
+- *
+- * Clears a variable from the index
++ * @brief Add a new check constraints object to the index
+ *
+ * @param[in] context variable_index
+- * @param[in] info The variable info corresponding to the variable to clear
++ * @param[in] guid The variable's guid
++ * @param[in] name_size The name parameter's size
++ * @param[in] name The variable's name
++ * @param[in] constraints The check constraints
++ *
++ * @return Pointer to variable_info or NULL
+ */
+-void variable_index_clear_variable(
++const struct variable_info *variable_index_add_constraints(
+ struct variable_index *context,
+- struct variable_info *info);
++ const EFI_GUID *guid,
++ size_t name_size,
++ const int16_t *name,
++ const struct variable_constraints *constraints);
+
+ /**
+- * @brief Set a check constraints object associated with a variavle
++ * @brief Update variable constraints that are already in the index
+ *
+ * @param[in] info variable info
+ * @param[in] constraints The check constraints
+ */
+-void variable_index_set_constraints(
+- struct variable_info *info,
++void variable_index_update_constraints(
++ const struct variable_info *info,
+ const struct variable_constraints *constraints);
+
+ /**
+diff --git a/components/service/smm_variable/backend/variable_index_iterator.c b/components/service/smm_variable/backend/variable_index_iterator.c
+index 8f8fc74..7cc6dc7 100644
+--- a/components/service/smm_variable/backend/variable_index_iterator.c
++++ b/components/service/smm_variable/backend/variable_index_iterator.c
+@@ -31,10 +31,10 @@ bool variable_index_iterator_is_done(
+ return iter->current_pos >= iter->variable_index->max_variables;
+ }
+
+-struct variable_info *variable_index_iterator_current(
++const struct variable_info *variable_index_iterator_current(
+ const struct variable_index_iterator *iter)
+ {
+- struct variable_info *current = NULL;
++ const struct variable_info *current = NULL;
+
+ if (!variable_index_iterator_is_done(iter)) {
+
+diff --git a/components/service/smm_variable/backend/variable_index_iterator.h b/components/service/smm_variable/backend/variable_index_iterator.h
+index 7ff77c5..f64a2c4 100644
+--- a/components/service/smm_variable/backend/variable_index_iterator.h
++++ b/components/service/smm_variable/backend/variable_index_iterator.h
+@@ -54,7 +54,7 @@ bool variable_index_iterator_is_done(
+ *
+ * @return Pointer to variable_info or NULL
+ */
+-struct variable_info *variable_index_iterator_current(
++const struct variable_info *variable_index_iterator_current(
+ const struct variable_index_iterator *iter);
+
+ /**
+diff --git a/components/service/smm_variable/test/service/smm_variable_service_tests.cpp b/components/service/smm_variable/test/service/smm_variable_service_tests.cpp
+index 15556e9..38c08eb 100644
+--- a/components/service/smm_variable/test/service/smm_variable_service_tests.cpp
++++ b/components/service/smm_variable/test/service/smm_variable_service_tests.cpp
+@@ -249,30 +249,6 @@ TEST(SmmVariableServiceTests, setAndGet)
+ UNSIGNED_LONGS_EQUAL(set_data.size(), get_data.size());
+ LONGS_EQUAL(0, get_data.compare(set_data));
+
+- /* Extend the variable using an append write */
+- std::string append_data = " values added with append write";
+-
+- efi_status = m_client->set_variable(
+- m_common_guid,
+- var_name,
+- append_data,
+- EFI_VARIABLE_APPEND_WRITE);
+-
+- UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+-
+- efi_status = m_client->get_variable(
+- m_common_guid,
+- var_name,
+- get_data);
+-
+- UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+-
+- std::string appended_data = set_data + append_data;
+-
+- /* Expect the append write operation to have extended the variable */
+- UNSIGNED_LONGLONGS_EQUAL(appended_data.size(), get_data.size());
+- LONGS_EQUAL(0, appended_data.compare(get_data));
+-
+ /* Expect remove to be permitted */
+ efi_status = m_client->remove_variable(m_common_guid, var_name);
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+@@ -303,30 +279,6 @@ TEST(SmmVariableServiceTests, setAndGetNv)
+ UNSIGNED_LONGS_EQUAL(set_data.size(), get_data.size());
+ LONGS_EQUAL(0, get_data.compare(set_data));
+
+- /* Extend the variable using an append write */
+- std::string append_data = " values added with append write";
+-
+- efi_status = m_client->set_variable(
+- m_common_guid,
+- var_name,
+- append_data,
+- EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_APPEND_WRITE);
+-
+- UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+-
+- efi_status = m_client->get_variable(
+- m_common_guid,
+- var_name,
+- get_data);
+-
+- UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+-
+- std::string appended_data = set_data + append_data;
+-
+- /* Expect the append write operation to have extended the variable */
+- UNSIGNED_LONGLONGS_EQUAL(appended_data.size(), get_data.size());
+- LONGS_EQUAL(0, appended_data.compare(get_data));
+-
+ /* Expect remove to be permitted */
+ efi_status = m_client->remove_variable(m_common_guid, var_name);
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+diff --git a/protocols/service/smm_variable/parameters.h b/protocols/service/smm_variable/parameters.h
+index 233f301..1f795a9 100644
+--- a/protocols/service/smm_variable/parameters.h
++++ b/protocols/service/smm_variable/parameters.h
+@@ -47,9 +47,6 @@ typedef struct {
+ EFI_VARIABLE_HARDWARE_ERROR_RECORD | \
+ EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | \
+ EFI_VARIABLE_APPEND_WRITE)
+-#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS_MASK \
+- (EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | \
+- EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)
+
+ /**
+ * Parameter structure for SetVariable and GetVariable.
+--
+2.17.1
+
diff --git a/meta-arm-bsp/recipes-security/trusted-services/ts-corstone1000.inc b/meta-arm-bsp/recipes-security/trusted-services/ts-corstone1000.inc
index 4a18586..9693430 100644
--- a/meta-arm-bsp/recipes-security/trusted-services/ts-corstone1000.inc
+++ b/meta-arm-bsp/recipes-security/trusted-services/ts-corstone1000.inc
@@ -38,6 +38,7 @@ SRC_URI:append = " \
file://0025-Add-stub-capsule-update-service-components.patch \
file://0026-Add-logs-to-functions-in-SMM-gateway-SP.patch \
file://0027-Configure-storage-size.patch \
+ file://0028-Revert-Add-uefi-variable-append-write-support.patch \
"
SRC_URI_MBED = "git://github.com/ARMmbed/mbed-crypto.git;protocol=https;branch=development;name=mbed;destsuffix=git/mbedcrypto"
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread