new file mode 100644
@@ -0,0 +1,28 @@
+From c7f2861e5c5ee209373a8dba15a608f78a97078b Mon Sep 17 00:00:00 2001
+From: Gabor Toth <gabor.toth2@arm.com>
+Date: Wed, 10 Apr 2024 11:17:50 +0200
+Subject: [PATCH 1/3] Fix: Avoid redefinition of variables
+
+Remove variable redefinition which shadows the original one.
+
+Signed-off-by: Gabor Toth <gabor.toth2@arm.com>
+Upstream-Status: Submitted [https://review.trustedfirmware.org/c/TS/trusted-services/+/27954]
+---
+ .../service/uefi/smm_variable/client/cpp/smm_variable_client.cpp | 1 -
+ 1 file changed, 1 deletion(-)
+
+diff --git a/components/service/uefi/smm_variable/client/cpp/smm_variable_client.cpp b/components/service/uefi/smm_variable/client/cpp/smm_variable_client.cpp
+index f71d0c864..d39448900 100644
+--- a/components/service/uefi/smm_variable/client/cpp/smm_variable_client.cpp
++++ b/components/service/uefi/smm_variable/client/cpp/smm_variable_client.cpp
+@@ -166,7 +166,6 @@ efi_status_t smm_variable_client::get_variable(const EFI_GUID &guid, const std::
+
+ if (call_handle) {
+ uint8_t *resp_buf;
+- size_t resp_len;
+ service_status_t service_status;
+
+ SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *access_var =
+--
+2.25.1
+
new file mode 100644
@@ -0,0 +1,495 @@
+From cc4cc9f3f5f02f713cf4da1854f3085bf31e71cf Mon Sep 17 00:00:00 2001
+From: Gabor Toth <gabor.toth2@arm.com>
+Date: Sat, 13 Apr 2024 14:52:23 +0200
+Subject: [PATCH 2/3] Fix GetNextVariableName NameSize input
+
+Based on the specification the NameSize shall be set to the available
+buffer size at the first call instead of the NameSize of the
+provided variable.
+Change smm-gateway and the tests according this. Also remove
+sanitize_get_next_var_name_param utility function, which is not
+compilant with this solution.
+
+Signed-off-by: Gabor Toth <gabor.toth2@arm.com>
+Upstream-Status: Submitted [https://review.trustedfirmware.org/c/TS/trusted-services/+/28022]
+---
+ .../backend/test/variable_store_tests.cpp | 48 +++++++--------
+ .../backend/uefi_variable_store.c | 60 ++++++++++++-------
+ .../backend/uefi_variable_store.h | 5 +-
+ .../smm_variable/backend/variable_index.c | 3 +
+ .../provider/smm_variable_provider.c | 59 +++++-------------
+ .../service/smm_variable_attack_tests.cpp | 29 ++++-----
+ .../service/smm_variable_service_tests.cpp | 7 ++-
+ 7 files changed, 98 insertions(+), 113 deletions(-)
+
+diff --git a/components/service/uefi/smm_variable/backend/test/variable_store_tests.cpp b/components/service/uefi/smm_variable/backend/test/variable_store_tests.cpp
+index fd48f13fb..72772821c 100644
+--- a/components/service/uefi/smm_variable/backend/test/variable_store_tests.cpp
++++ b/components/service/uefi/smm_variable/backend/test/variable_store_tests.cpp
+@@ -501,15 +501,13 @@ TEST(UefiVariableStoreTests, bootServiceAccess)
+ std::vector<uint8_t> msg_buffer(VARIABLE_BUFFER_SIZE);
+ SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *next_name =
+ (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *) msg_buffer.data();
+- size_t max_name_len =
+- VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;
+
+ size_t total_len = 0;
+- next_name->NameSize = sizeof(int16_t);
++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;
+ next_name->Name[0] = 0;
+
+ status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name,
+- max_name_len, &total_len);
++ &total_len);
+
+ UNSIGNED_LONGLONGS_EQUAL(EFI_NOT_FOUND, status);
+ }
+@@ -574,47 +572,48 @@ TEST(UefiVariableStoreTests, enumerateStoreContents)
+ std::vector<uint8_t> msg_buffer(VARIABLE_BUFFER_SIZE);
+ SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *next_name =
+ (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *) msg_buffer.data();
+- size_t max_name_len =
+- VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;
+
+ /* First check handling of invalid variable name */
+ std::u16string bogus_name = to_variable_name(u"bogus_variable");
+ size_t bogus_name_size = string_get_size_in_bytes(bogus_name);
+ next_name->Guid = m_common_guid;
+- next_name->NameSize = bogus_name_size;
++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;
+ memcpy(next_name->Name, bogus_name.data(), bogus_name_size);
+
+ status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name,
+- max_name_len, &total_len);
++ &total_len);
+ UNSIGNED_LONGLONGS_EQUAL(EFI_INVALID_PARAMETER, status);
+
+ /* Enumerate store contents */
+ next_name->NameSize = sizeof(int16_t);
+ next_name->Name[0] = 0;
+- /* Check if the correct NameSize is returned if max_name_len is too small */
++ /* Check if the correct NameSize is returned if namesize is too small */
+ status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name,
+- 0, &total_len);
++ &total_len);
+ UNSIGNED_LONGLONGS_EQUAL(EFI_BUFFER_TOO_SMALL, status);
+ UNSIGNED_LONGLONGS_EQUAL(sizeof(var_name_1), next_name->NameSize);
+
+- /* And then used the previously received next_name->NameSize as max_name_len */
++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;
+ status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name,
+- next_name->NameSize, &total_len);
++ &total_len);
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+ CHECK_TRUE(compare_variable_name(var_name_1, next_name->Name, next_name->NameSize));
+
++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;
+ status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name,
+- max_name_len, &total_len);
++ &total_len);
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+ CHECK_TRUE(compare_variable_name(var_name_2, next_name->Name, next_name->NameSize));
+
++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;
+ status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name,
+- max_name_len, &total_len);
++ &total_len);
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+ CHECK_TRUE(compare_variable_name(var_name_3, next_name->Name, next_name->NameSize));
+
++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;
+ status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name,
+- max_name_len, &total_len);
++ &total_len);
+ UNSIGNED_LONGLONGS_EQUAL(EFI_NOT_FOUND, status);
+
+ power_cycle();
+@@ -622,21 +621,23 @@ TEST(UefiVariableStoreTests, enumerateStoreContents)
+ /* Enumerate again - should be left with just NV variables.
+ * Use a different but equally valid null name.
+ */
+- next_name->NameSize = 10 * sizeof(int16_t);
++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;
+ memset(next_name->Name, 0, next_name->NameSize);
+
+ status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name,
+- max_name_len, &total_len);
++ &total_len);
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+ CHECK_TRUE(compare_variable_name(var_name_1, next_name->Name, next_name->NameSize));
+
++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;
+ status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name,
+- max_name_len, &total_len);
++ &total_len);
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+ CHECK_TRUE(compare_variable_name(var_name_3, next_name->Name, next_name->NameSize));
+
++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;
+ status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name,
+- max_name_len, &total_len);
++ &total_len);
+ UNSIGNED_LONGLONGS_EQUAL(EFI_NOT_FOUND, status);
+ }
+
+@@ -672,21 +673,20 @@ TEST(UefiVariableStoreTests, failedNvSet)
+ std::vector<uint8_t> msg_buffer(VARIABLE_BUFFER_SIZE);
+ SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *next_name =
+ (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *) msg_buffer.data();
+- size_t max_name_len =
+- VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;
+
+ /* Enumerate store contents */
+ size_t total_len = 0;
+- next_name->NameSize = sizeof(int16_t);
++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;
+ next_name->Name[0] = 0;
+
+ status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name,
+- max_name_len, &total_len);
++ &total_len);
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+ CHECK_TRUE(compare_variable_name(var_name_1, next_name->Name, next_name->NameSize));
+
++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;
+ status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name,
+- max_name_len, &total_len);
++ &total_len);
+ UNSIGNED_LONGLONGS_EQUAL(EFI_NOT_FOUND, status);
+ }
+
+diff --git a/components/service/uefi/smm_variable/backend/uefi_variable_store.c b/components/service/uefi/smm_variable/backend/uefi_variable_store.c
+index 5b46c1371..caf6698aa 100644
+--- a/components/service/uefi/smm_variable/backend/uefi_variable_store.c
++++ b/components/service/uefi/smm_variable/backend/uefi_variable_store.c
+@@ -404,9 +404,27 @@ efi_status_t uefi_variable_store_get_variable(const struct uefi_variable_store *
+ efi_status_t
+ uefi_variable_store_get_next_variable_name(const struct uefi_variable_store *context,
+ SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *cur,
+- size_t max_name_len, size_t *total_length)
++ size_t *total_length)
+ {
+- efi_status_t status = check_name_terminator(cur->Name, cur->NameSize);
++ efi_status_t status = EFI_SUCCESS;
++ size_t buffer_size = 0;
++
++ if (!cur)
++ return EFI_INVALID_PARAMETER;
++ /*
++ * NameSize is set to the buffer size to store the names,
++ * let's calculate the size actually being used.
++ */
++ buffer_size = cur->NameSize;
++ for (int i = 0; i < buffer_size / sizeof(int16_t); i++) {
++ if (cur->Name[i] == 0) {
++ /* With null terminator */
++ cur->NameSize = 2*(i+1);
++ break;
++ }
++ }
++
++ status = check_name_terminator(cur->Name, cur->NameSize);
+
+ if (status != EFI_SUCCESS)
+ return status;
+@@ -418,21 +436,11 @@ uefi_variable_store_get_next_variable_name(const struct uefi_variable_store *con
+ &context->variable_index, &cur->Guid, cur->NameSize, cur->Name, &status);
+
+ if (info && (status == EFI_SUCCESS)) {
+- /* The NameSize has to be set in every case according to the UEFI specs.
+- * In case of EFI_BUFFER_TOO_SMALL it has to reflect the size of buffer
+- * needed.
+- */
+- cur->NameSize = info->metadata.name_size;
+- *total_length = sizeof(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME);
+-
+- if (info->metadata.name_size <= max_name_len) {
++ if (info->metadata.name_size <= buffer_size) {
+ cur->Guid = info->metadata.guid;
++ cur->NameSize = info->metadata.name_size;
+ memcpy(cur->Name, info->metadata.name, info->metadata.name_size);
+
+- *total_length =
+- SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME_TOTAL_SIZE(
+- cur);
+-
+ /*
+ * Check if variable is accessible (e.g boot variable is not
+ * accessible at runtime)
+@@ -442,6 +450,10 @@ uefi_variable_store_get_next_variable_name(const struct uefi_variable_store *con
+ if (status == EFI_SUCCESS)
+ break;
+ } else {
++ /* The VariableNameSize is updated to reflect the size of buffer needed */
++ cur->NameSize = info->metadata.name_size;
++ memset(cur->Name, 0, buffer_size);
++ memset(&cur->Guid, 0, sizeof(EFI_GUID));
+ status = EFI_BUFFER_TOO_SMALL;
+ break;
+ }
+@@ -450,18 +462,24 @@ uefi_variable_store_get_next_variable_name(const struct uefi_variable_store *con
+ /* Do not hide original error if there is any */
+ if (status == EFI_SUCCESS)
+ status = EFI_NOT_FOUND;
++
++ memset(cur->Name, 0, buffer_size);
++ memset(&cur->Guid, 0, sizeof(EFI_GUID));
++ cur->NameSize = 0;
+ break;
+ }
+ }
+
+- /* If we found no accessible variable clear the fields for security */
+- if (status != EFI_SUCCESS) {
+- memset(cur->Name, 0, max_name_len);
+- memset(&cur->Guid, 0, sizeof(EFI_GUID));
+- if (status != EFI_BUFFER_TOO_SMALL)
+- cur->NameSize = 0;
++ if (status == EFI_SUCCESS) {
++ /* Store everything including the name */
++ *total_length =
++ SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME_TOTAL_SIZE(
++ cur);
++ } else {
++ /* Do not store the name, only the size */
++ *total_length =
++ SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME_NAME_OFFSET;
+ }
+-
+ return status;
+ }
+
+diff --git a/components/service/uefi/smm_variable/backend/uefi_variable_store.h b/components/service/uefi/smm_variable/backend/uefi_variable_store.h
+index 8be5f36e6..2493ff6b4 100644
+--- a/components/service/uefi/smm_variable/backend/uefi_variable_store.h
++++ b/components/service/uefi/smm_variable/backend/uefi_variable_store.h
+@@ -134,8 +134,7 @@ efi_status_t uefi_variable_store_get_variable(const struct uefi_variable_store *
+ * Used for enumerating the store contents
+ *
+ * @param[in] context uefi_variable_store instance
+- * @param[out] cur Current variable name
+- * @param[in] max_name_len The maximum variable name length
++ * @param[inout] cur The size of the VariableName buffer
+ * @param[out] total_len The total length of the output
+ *
+ * @return EFI_SUCCESS if successful
+@@ -143,7 +142,7 @@ efi_status_t uefi_variable_store_get_variable(const struct uefi_variable_store *
+ efi_status_t
+ uefi_variable_store_get_next_variable_name(const struct uefi_variable_store *context,
+ SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *cur,
+- size_t max_name_len, size_t *total_length);
++ size_t *total_length);
+
+ /**
+ * @brief Query for variable info
+diff --git a/components/service/uefi/smm_variable/backend/variable_index.c b/components/service/uefi/smm_variable/backend/variable_index.c
+index d850dbe18..e2fe6dd38 100644
+--- a/components/service/uefi/smm_variable/backend/variable_index.c
++++ b/components/service/uefi/smm_variable/backend/variable_index.c
+@@ -27,6 +27,9 @@ static uint64_t name_hash(const EFI_GUID *guid, size_t name_size, const int16_t
+
+ /* Extend to cover name up to but not including null terminator */
+ for (size_t i = 0; i < (name_size - sizeof(int16_t)) / sizeof(int16_t); ++i) {
++ /* Only hash till the first null terminator */
++ if (name[i] == 0)
++ break;
+ hash = ((hash << 5) + hash) + name[i];
+ }
+
+diff --git a/components/service/uefi/smm_variable/provider/smm_variable_provider.c b/components/service/uefi/smm_variable/provider/smm_variable_provider.c
+index ca3f7e5e5..1a5269338 100644
+--- a/components/service/uefi/smm_variable/provider/smm_variable_provider.c
++++ b/components/service/uefi/smm_variable/provider/smm_variable_provider.c
+@@ -81,30 +81,6 @@ static efi_status_t sanitize_access_variable_param(struct rpc_request *req, size
+ return efi_status;
+ }
+
+-static efi_status_t sanitize_get_next_var_name_param(struct rpc_request *req, size_t *param_len)
+-{
+- efi_status_t efi_status = EFI_INVALID_PARAMETER;
+- *param_len = 0;
+- const struct rpc_buffer *req_buf = &req->request;
+-
+- if (req_buf->data_length >= SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME_NAME_OFFSET) {
+- const SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *param =
+- (const SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *)req_buf->data;
+-
+- size_t max_space_for_name =
+- req_buf->data_length -
+- SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME_NAME_OFFSET;
+-
+- if (param->NameSize <= max_space_for_name) {
+- *param_len =
+- SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME_TOTAL_SIZE(param);
+- efi_status = EFI_SUCCESS;
+- }
+- }
+-
+- return efi_status;
+-}
+-
+ static efi_status_t sanitize_var_check_property_param(struct rpc_request *req, size_t *param_len)
+ {
+ efi_status_t efi_status = EFI_INVALID_PARAMETER;
+@@ -146,7 +122,7 @@ static rpc_status_t get_variable_handler(void *context, struct rpc_request *req)
+ struct rpc_buffer *req_buf = &req->request;
+ size_t max_data_len = resp_buf->size - param_len;
+
+- memmove(resp_buf->data, req_buf->data, param_len);
++ memcpy(resp_buf->data, req_buf->data, param_len);
+
+ efi_status = uefi_variable_store_get_variable(
+ &this_instance->variable_store,
+@@ -167,28 +143,21 @@ static rpc_status_t get_next_variable_name_handler(void *context, struct rpc_req
+ {
+ struct smm_variable_provider *this_instance = (struct smm_variable_provider *)context;
+
+- size_t param_len = 0;
+- efi_status_t efi_status = sanitize_get_next_var_name_param(req, ¶m_len);
++ efi_status_t efi_status = EFI_SUCCESS;
++ size_t variable_size = 0;
+
+- if (efi_status == EFI_SUCCESS) {
+- /* Valid get next variable name header */
+- struct rpc_buffer *resp_buf = &req->response;
++ /* Valid get next variable name header */
++ struct rpc_buffer *resp_buf = &req->response;
++ struct rpc_buffer *req_buf = &req->request;
+
+- if (resp_buf->size >= param_len) {
+- struct rpc_buffer *req_buf = &req->request;
++ memcpy(resp_buf->data, req_buf->data, req_buf->data_length);
+
+- memmove(resp_buf->data, req_buf->data, param_len);
++ efi_status = uefi_variable_store_get_next_variable_name(
++ &this_instance->variable_store,
++ (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *)resp_buf->data,
++ &variable_size);
+
+- efi_status = uefi_variable_store_get_next_variable_name(
+- &this_instance->variable_store,
+- (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *)resp_buf->data,
+- ((SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME*)resp_buf->data)->NameSize,
+- &resp_buf->data_length);
+- } else {
+- /* Reponse buffer not big enough */
+- efi_status = EFI_BAD_BUFFER_SIZE;
+- }
+- }
++ resp_buf->data_length = variable_size;
+
+ req->service_status = efi_status;
+
+@@ -240,7 +209,7 @@ static rpc_status_t query_variable_info_handler(void *context, struct rpc_reques
+ struct rpc_buffer *resp_buf = &req->response;
+
+ if (resp_buf->size >= req_buf->data_length) {
+- memmove(resp_buf->data, req_buf->data, req_buf->data_length);
++ memcpy(resp_buf->data, req_buf->data, req_buf->data_length);
+
+ efi_status = uefi_variable_store_query_variable_info(
+ &this_instance->variable_store,
+@@ -308,7 +277,7 @@ static rpc_status_t get_var_check_property_handler(void *context, struct rpc_req
+
+ if (resp_buf->size >= param_len) {
+ struct rpc_buffer *req_buf = &req->request;
+- memmove(resp_buf->data, req_buf->data, param_len);
++ memcpy(resp_buf->data, req_buf->data, param_len);
+ resp_buf->data_length = param_len;
+
+ efi_status = uefi_variable_store_get_var_check_property(
+diff --git a/components/service/uefi/smm_variable/test/service/smm_variable_attack_tests.cpp b/components/service/uefi/smm_variable/test/service/smm_variable_attack_tests.cpp
+index 76b62fd35..98e61fec0 100644
+--- a/components/service/uefi/smm_variable/test/service/smm_variable_attack_tests.cpp
++++ b/components/service/uefi/smm_variable/test/service/smm_variable_attack_tests.cpp
+@@ -176,19 +176,6 @@ TEST(SmmVariableAttackTests, setAndGetWithSizeMaxNameSize)
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+ }
+
+-TEST(SmmVariableAttackTests, enumerateWithOversizeName)
+-{
+- efi_status_t efi_status = EFI_SUCCESS;
+- std::u16string var_name = null_name;
+- EFI_GUID guid;
+- memset(&guid, 0, sizeof(guid));
+-
+- efi_status = m_client->get_next_variable_name(guid, var_name,
+- (var_name.size() + 1) * sizeof(int16_t) + 1);
+-
+- UNSIGNED_LONGLONGS_EQUAL(EFI_INVALID_PARAMETER, efi_status);
+-}
+-
+ TEST(SmmVariableAttackTests, enumerateWithSizeMaxNameSize)
+ {
+ efi_status_t efi_status = EFI_SUCCESS;
+@@ -202,17 +189,23 @@ TEST(SmmVariableAttackTests, enumerateWithSizeMaxNameSize)
+
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+
+- /* Initial iteration uses good name length */
+- efi_status = m_client->get_next_variable_name(guid, var_name);
++ /* Initial iteration uses good name length for next variable */
++ efi_status = m_client->get_next_variable_name(guid, var_name, std::numeric_limits<size_t>::max());
+
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+
+- /* Next iteration uses invalid name length */
+- efi_status = m_client->get_next_variable_name(guid, var_name,
+- std::numeric_limits<size_t>::max());
++ /* Next iteration uses invalid name length, so a null terminator can not fit */
++ var_name = null_name;
++ efi_status = m_client->get_next_variable_name(guid, var_name, 1);
+
+ UNSIGNED_LONGLONGS_EQUAL(EFI_INVALID_PARAMETER, efi_status);
+
++ /* Next iteration uses invalid name length, so a null terminator can not fit */
++ var_name = null_name;
++ efi_status = m_client->get_next_variable_name(guid, var_name, 2);
++
++ UNSIGNED_LONGLONGS_EQUAL(EFI_BUFFER_TOO_SMALL, efi_status);
++
+ /* Expect to be able to remove the variable */
+ efi_status = m_client->remove_variable(m_common_guid, var_name_1);
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+diff --git a/components/service/uefi/smm_variable/test/service/smm_variable_service_tests.cpp b/components/service/uefi/smm_variable/test/service/smm_variable_service_tests.cpp
+index e82a90c37..8fa4f8077 100644
+--- a/components/service/uefi/smm_variable/test/service/smm_variable_service_tests.cpp
++++ b/components/service/uefi/smm_variable/test/service/smm_variable_service_tests.cpp
+@@ -9,6 +9,7 @@
+ #include <cstring>
+ #include <locale>
+ #include <sstream>
++#include <limits>
+
+ #include "util.h"
+
+@@ -154,7 +155,7 @@ TEST_GROUP(SmmVariableServiceTests)
+ #endif
+
+ do {
+- status = m_client->get_next_variable_name(guid, var_name);
++ status = m_client->get_next_variable_name(guid, var_name, max_variable_size);
+
+ /* There are no more variables in the persistent store */
+ if (status == EFI_NOT_FOUND) {
+@@ -223,6 +224,8 @@ TEST_GROUP(SmmVariableServiceTests)
+ std::u16string m_ro_variable = to_variable_name(u"ro_variable");
+ std::u16string m_boot_finished_var_name = to_variable_name(u"finished");
+
++ uint32_t max_variable_size = 4096;
++
+ /* Cleanup skips these variables */
+ std::vector<std::u16string *> m_non_rm_vars{ &m_ro_variable, &m_boot_finished_var_name };
+
+@@ -654,7 +657,7 @@ TEST(SmmVariableServiceTests, enumerateStoreContents)
+ std::u16string *expected_variables[] = { &var_name_1, &var_name_2, &var_name_3 };
+
+ do {
+- efi_status = m_client->get_next_variable_name(guid, var_name);
++ efi_status = m_client->get_next_variable_name(guid, var_name, max_variable_size);
+ if (efi_status != EFI_SUCCESS)
+ break;
+
+--
+2.25.1
+
new file mode 100644
@@ -0,0 +1,82 @@
+From c62e728bb86981219984c8b39819fb8926a41e10 Mon Sep 17 00:00:00 2001
+From: Gabor Toth <gabor.toth2@arm.com>
+Date: Fri, 19 Apr 2024 18:25:23 +0200
+Subject: [PATCH 3/3] Fix error handling of variable index loading
+
+If loading of the variable index from Protected Storage fails, SmmGW
+will silently continue with empty variable store. This is a serious
+fault and a potential security risk.
+Change the code to produce a log output when this happens and stop
+loading the SP.
+
+Signed-off-by: Gabor Toth <gabor.toth2@arm.com>
+Upstream-Status: Submitted [https://review.trustedfirmware.org/c/TS/trusted-services/+/28300]
+---
+ .../backend/uefi_variable_store.c | 28 ++++++++++++++-----
+ 1 file changed, 21 insertions(+), 7 deletions(-)
+
+diff --git a/components/service/uefi/smm_variable/backend/uefi_variable_store.c b/components/service/uefi/smm_variable/backend/uefi_variable_store.c
+index caf6698aa..c1691dc8f 100644
+--- a/components/service/uefi/smm_variable/backend/uefi_variable_store.c
++++ b/components/service/uefi/smm_variable/backend/uefi_variable_store.c
+@@ -27,7 +27,7 @@
+ #include "service/crypto/client/psa/crypto_client.h"
+ #endif
+
+-static void load_variable_index(struct uefi_variable_store *context);
++static efi_status_t load_variable_index(struct uefi_variable_store *context);
+
+ static efi_status_t sync_variable_index(const struct uefi_variable_store *context);
+
+@@ -165,8 +165,10 @@ efi_status_t uefi_variable_store_init(struct uefi_variable_store *context, uint3
+
+ /* Load the variable index with NV variable info from the persistent store */
+ if (context->index_sync_buffer) {
+- load_variable_index(context);
+- purge_orphan_index_entries(context);
++ status = load_variable_index(context);
++
++ if (status == EFI_SUCCESS)
++ purge_orphan_index_entries(context);
+ }
+ }
+
+@@ -571,7 +573,7 @@ efi_status_t uefi_variable_store_get_var_check_property(
+ return status;
+ }
+
+-static void load_variable_index(struct uefi_variable_store *context)
++static efi_status_t load_variable_index(struct uefi_variable_store *context)
+ {
+ struct storage_backend *persistent_store = context->persistent_store.storage_backend;
+
+@@ -583,11 +585,23 @@ static void load_variable_index(struct uefi_variable_store *context)
+ SMM_VARIABLE_INDEX_STORAGE_UID, 0, context->index_sync_buffer_size,
+ context->index_sync_buffer, &data_len);
+
+- if (psa_status == PSA_SUCCESS) {
+- variable_index_restore(&context->variable_index, data_len,
+- context->index_sync_buffer);
++ switch(psa_status) {
++ case PSA_SUCCESS:
++ (void) variable_index_restore(&context->variable_index, data_len,
++ context->index_sync_buffer);
++ break;
++
++ case PSA_ERROR_DOES_NOT_EXIST:
++ IMSG("Index variable does not exist in NV store, continuing with empty index");
++ break;
++
++ default:
++ EMSG("Loading variable index failed: %d", psa_status);
++ return EFI_LOAD_ERROR;
+ }
+ }
++
++ return EFI_SUCCESS;
+ }
+
+ static efi_status_t sync_variable_index(const struct uefi_variable_store *context)
+--
+2.25.1
+
@@ -12,6 +12,9 @@ SRC_URI:append:corstone1000 = " \
file://0008-plat-corstone1000-add-client_id-for-FMP-service.patch \
file://0009-Remove-Werror-flag.patch \
file://0010-Remove-PLATFORM_HAS_ATTEST_PK-define-from-IAT-test.patch \
+ file://0011-Fix-Avoid-redefinition-of-variables.patch \
+ file://0012-Fix-GetNextVariableName-NameSize-input.patch \
+ file://0013-Fix-error-handling-of-variable-index-loading.patch \
"
COMPATIBLE_MACHINE:fvp-base = "fvp-base"