new file mode 100644
@@ -0,0 +1,407 @@
+Upstream-Status: Pending
+Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+
+From 2d975e5ec5df6f81d6c35fe927f72d49181142f8 Mon Sep 17 00:00:00 2001
+From: Julian Hall <julian.hall@arm.com>
+Date: Tue, 19 Jul 2022 12:43:30 +0100
+Subject: [PATCH] Fix UEFI get_variable with small buffer
+
+The handling of the UEFI get_variable operation was incorrect when
+a small or zero data length was specified by a requester. A zero
+length data length is a legitimate way to discover the size of a
+variable without actually retrieving its data. This change adds
+test cases that reproduce the problem and a fix.
+
+Signed-off-by: Julian Hall <julian.hall@arm.com>
+Change-Id: Iec087fbf9305746d1438888e871602ec0ce15824
+---
+ .../backend/test/variable_store_tests.cpp | 60 ++++++++++++++++--
+ .../backend/uefi_variable_store.c | 46 +++++++++++---
+ .../client/cpp/smm_variable_client.cpp | 33 +++++-----
+ .../client/cpp/smm_variable_client.h | 8 ++-
+ .../provider/smm_variable_provider.c | 2 +-
+ .../service/smm_variable_service_tests.cpp | 62 +++++++++++++++++++
+ 6 files changed, 179 insertions(+), 32 deletions(-)
+
+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 235642e6..98faf761 100644
+--- a/components/service/smm_variable/backend/test/variable_store_tests.cpp
++++ b/components/service/smm_variable/backend/test/variable_store_tests.cpp
+@@ -128,7 +128,8 @@ TEST_GROUP(UefiVariableStoreTests)
+
+ efi_status_t get_variable(
+ const std::wstring &name,
+- std::string &data)
++ std::string &data,
++ size_t data_len_clamp = VARIABLE_BUFFER_SIZE)
+ {
+ std::vector<int16_t> var_name = to_variable_name(name);
+ size_t name_size = var_name.size() * sizeof(int16_t);
+@@ -144,21 +145,40 @@ TEST_GROUP(UefiVariableStoreTests)
+ access_variable->NameSize = name_size;
+ memcpy(access_variable->Name, var_name.data(), name_size);
+
+- access_variable->DataSize = 0;
++ size_t max_data_len = (data_len_clamp == VARIABLE_BUFFER_SIZE) ?
++ VARIABLE_BUFFER_SIZE -
++ SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_DATA_OFFSET(access_variable) :
++ data_len_clamp;
++
++ access_variable->DataSize = max_data_len;
+
+ efi_status_t status = uefi_variable_store_get_variable(
+ &m_uefi_variable_store,
+ access_variable,
+- VARIABLE_BUFFER_SIZE -
+- SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_DATA_OFFSET(access_variable),
++ max_data_len,
+ &total_size);
+
++ data.clear();
++
+ if (status == EFI_SUCCESS) {
+
+ const char *data_start = (const char*)(msg_buffer +
+ SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_DATA_OFFSET(access_variable));
+
+ data = std::string(data_start, access_variable->DataSize);
++
++ UNSIGNED_LONGLONGS_EQUAL(
++ SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_TOTAL_SIZE(access_variable),
++ total_size);
++ }
++ else if (status == EFI_BUFFER_TOO_SMALL) {
++
++ /* String length set to reported variable length */
++ data.insert(0, access_variable->DataSize, '!');
++
++ UNSIGNED_LONGLONGS_EQUAL(
++ SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_DATA_OFFSET(access_variable),
++ total_size);
+ }
+
+ return status;
+@@ -336,6 +356,38 @@ TEST(UefiVariableStoreTests, persistentSetGet)
+ LONGS_EQUAL(0, input_data.compare(output_data));
+ }
+
++TEST(UefiVariableStoreTests, getWithSmallBuffer)
++{
++ efi_status_t status = EFI_SUCCESS;
++ std::wstring var_name = L"test_variable";
++ std::string input_data = "quick brown fox";
++ std::string output_data;
++
++ /* A get with a zero length buffer is a legitimate way to
++ * discover the variable size. This test performs GetVariable
++ * operations with various buffer small buffer sizes. */
++ status = set_variable(var_name, input_data, 0);
++ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
++
++ /* First get the variable without a constrained buffer */
++ status = get_variable(var_name, output_data);
++ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
++
++ /* 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));
++
++ /* Now try with a zero length buffer */
++ status = get_variable(var_name, output_data, 0);
++ UNSIGNED_LONGLONGS_EQUAL(EFI_BUFFER_TOO_SMALL, status);
++ UNSIGNED_LONGLONGS_EQUAL(input_data.size(), output_data.size());
++
++ /* Try with a non-zero length but too small buffer */
++ status = get_variable(var_name, output_data, input_data.size() -1);
++ UNSIGNED_LONGLONGS_EQUAL(EFI_BUFFER_TOO_SMALL, status);
++ UNSIGNED_LONGLONGS_EQUAL(input_data.size(), output_data.size());
++}
++
+ TEST(UefiVariableStoreTests, removeVolatile)
+ {
+ efi_status_t status = EFI_SUCCESS;
+diff --git a/components/service/smm_variable/backend/uefi_variable_store.c b/components/service/smm_variable/backend/uefi_variable_store.c
+index e8771c21..90d648de 100644
+--- a/components/service/smm_variable/backend/uefi_variable_store.c
++++ b/components/service/smm_variable/backend/uefi_variable_store.c
+@@ -1,5 +1,5 @@
+ /*
+- * Copyright (c) 2021, Arm Limited. All rights reserved.
++ * Copyright (c) 2021-2022, Arm Limited. All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ *
+@@ -294,7 +294,10 @@ efi_status_t uefi_variable_store_get_variable(
+
+ status = load_variable_data(context, info, var, max_data_len);
+ var->Attributes = info->metadata.attributes;
+- *total_length = SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_TOTAL_SIZE(var);
++
++ *total_length = (status == EFI_SUCCESS) ?
++ SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_TOTAL_SIZE(var) :
++ SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_DATA_OFFSET(var);
+ }
+ }
+
+@@ -682,7 +685,6 @@ static efi_status_t load_variable_data(
+ {
+ EMSG("In func %s\n", __func__);
+ psa_status_t psa_status = PSA_SUCCESS;
+- size_t data_len = 0;
+ uint8_t *data = (uint8_t*)var +
+ SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_DATA_OFFSET(var);
+
+@@ -692,17 +694,41 @@ static efi_status_t load_variable_data(
+
+ if (delegate_store->storage_backend) {
+
+- psa_status = delegate_store->storage_backend->interface->get(
++ struct psa_storage_info_t storage_info;
++
++ psa_status = delegate_store->storage_backend->interface->get_info(
+ delegate_store->storage_backend->context,
+ context->owner_id,
+ info->metadata.uid,
+- 0,
+- max_data_len,
+- data,
+- &data_len);
+- EMSG("In func %s get status is %d\n", __func__, psa_status);
++ &storage_info);
++
++ if (psa_status == PSA_SUCCESS) {
+
+- var->DataSize = data_len;
++ size_t get_limit = (var->DataSize < max_data_len) ?
++ var->DataSize :
++ max_data_len;
++
++ if (get_limit >= storage_info.size) {
++
++ size_t got_len = 0;
++
++ psa_status = delegate_store->storage_backend->interface->get(
++ delegate_store->storage_backend->context,
++ context->owner_id,
++ info->metadata.uid,
++ 0,
++ max_data_len,
++ data,
++ &got_len);
++
++ var->DataSize = got_len;
++ }
++ else {
++
++ var->DataSize = storage_info.size;
++ psa_status = PSA_ERROR_BUFFER_TOO_SMALL;
++ }
++ }
+ }
+
+ return psa_to_efi_storage_status(psa_status);
+diff --git a/components/service/smm_variable/client/cpp/smm_variable_client.cpp b/components/service/smm_variable/client/cpp/smm_variable_client.cpp
+index 8438285b..b6b4ed90 100644
+--- a/components/service/smm_variable/client/cpp/smm_variable_client.cpp
++++ b/components/service/smm_variable/client/cpp/smm_variable_client.cpp
+@@ -1,5 +1,5 @@
+ /*
+- * Copyright (c) 2021, Arm Limited and Contributors. All rights reserved.
++ * Copyright (c) 2021-2022, Arm Limited and Contributors. All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+@@ -122,21 +122,22 @@ efi_status_t smm_variable_client::get_variable(
+ guid,
+ name,
+ data,
+- 0);
++ 0,
++ MAX_VAR_DATA_SIZE);
+ }
+
+ efi_status_t smm_variable_client::get_variable(
+ const EFI_GUID &guid,
+ const std::wstring &name,
+ std::string &data,
+- size_t override_name_size)
++ size_t override_name_size,
++ size_t max_data_size)
+ {
+ efi_status_t efi_status = EFI_NOT_READY;
+
+ std::vector<int16_t> var_name = to_variable_name(name);
+ size_t name_size = var_name.size() * sizeof(int16_t);
+- size_t data_size = 0;
+- size_t req_len = SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_SIZE(name_size, data_size);
++ size_t req_len = SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_SIZE(name_size, 0);
+
+ rpc_call_handle call_handle;
+ uint8_t *req_buf;
+@@ -154,7 +155,7 @@ efi_status_t smm_variable_client::get_variable(
+
+ access_var->Guid = guid;
+ access_var->NameSize = name_size;
+- access_var->DataSize = data_size;
++ access_var->DataSize = max_data_size;
+
+ memcpy(access_var->Name, var_name.data(), name_size);
+
+@@ -168,26 +169,28 @@ efi_status_t smm_variable_client::get_variable(
+
+ efi_status = opstatus;
+
+- if (efi_status == EFI_SUCCESS) {
+-
+- efi_status = EFI_PROTOCOL_ERROR;
++ if (resp_len >= SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET) {
+
+- if (resp_len >= SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET) {
++ access_var = (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE*)resp_buf;
++ size_t data_size = access_var->DataSize;
+
+- access_var = (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE*)resp_buf;
++ if (resp_len >=
++ SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_TOTAL_SIZE(access_var)) {
+
+- if (resp_len >=
+- SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_TOTAL_SIZE(access_var)) {
++ if (efi_status == EFI_SUCCESS) {
+
+- data_size = access_var->DataSize;
+ const char *data_start = (const char*)
+ &resp_buf[
+ SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_DATA_OFFSET(access_var)];
+
+ data.assign(data_start, data_size);
+- efi_status = EFI_SUCCESS;
+ }
+ }
++ else if (efi_status == EFI_BUFFER_TOO_SMALL) {
++
++ data.clear();
++ data.insert(0, data_size, '!');
++ }
+ }
+ }
+ else {
+diff --git a/components/service/smm_variable/client/cpp/smm_variable_client.h b/components/service/smm_variable/client/cpp/smm_variable_client.h
+index c7973916..3d2371a8 100644
+--- a/components/service/smm_variable/client/cpp/smm_variable_client.h
++++ b/components/service/smm_variable/client/cpp/smm_variable_client.h
+@@ -1,5 +1,5 @@
+ /*
+- * Copyright (c) 2021, Arm Limited and Contributors. All rights reserved.
++ * Copyright (c) 2021-2022, Arm Limited and Contributors. All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+@@ -56,7 +56,8 @@ public:
+ const EFI_GUID &guid,
+ const std::wstring &name,
+ std::string &data,
+- size_t override_name_size);
++ size_t override_name_size,
++ size_t max_data_size = MAX_VAR_DATA_SIZE);
+
+ /* Remove a variable */
+ efi_status_t remove_variable(
+@@ -113,6 +114,9 @@ public:
+
+
+ private:
++
++ static const size_t MAX_VAR_DATA_SIZE = 65536;
++
+ efi_status_t rpc_to_efi_status() const;
+
+ static std::vector<int16_t> to_variable_name(const std::wstring &string);
+diff --git a/components/service/smm_variable/provider/smm_variable_provider.c b/components/service/smm_variable/provider/smm_variable_provider.c
+index 1f362c17..95c4fdc9 100644
+--- a/components/service/smm_variable/provider/smm_variable_provider.c
++++ b/components/service/smm_variable/provider/smm_variable_provider.c
+@@ -165,7 +165,7 @@ static rpc_status_t get_variable_handler(void *context, struct call_req *req)
+ }
+ else {
+
+- /* Reponse buffer not big enough */
++ /* Response buffer not big enough */
+ efi_status = EFI_BAD_BUFFER_SIZE;
+ }
+ }
+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 38c08ebe..989a3e63 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
+@@ -284,6 +284,68 @@ TEST(SmmVariableServiceTests, setAndGetNv)
+ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+ }
+
++TEST(SmmVariableServiceTests, getVarSize)
++{
++ efi_status_t efi_status = EFI_SUCCESS;
++ std::wstring var_name = L"test_variable";
++ std::string set_data = "UEFI variable data string";
++ std::string get_data;
++
++ efi_status = m_client->set_variable(
++ m_common_guid,
++ var_name,
++ set_data,
++ 0);
++
++ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
++
++ /* Get with the data size set to zero. This is the standard way
++ * to discover the variable size. */
++ efi_status = m_client->get_variable(
++ m_common_guid,
++ var_name,
++ get_data,
++ 0, 0);
++
++ UNSIGNED_LONGLONGS_EQUAL(EFI_BUFFER_TOO_SMALL, efi_status);
++ UNSIGNED_LONGS_EQUAL(set_data.size(), get_data.size());
++
++ /* Expect remove to be permitted */
++ efi_status = m_client->remove_variable(m_common_guid, var_name);
++ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
++}
++
++TEST(SmmVariableServiceTests, getVarSizeNv)
++{
++ efi_status_t efi_status = EFI_SUCCESS;
++ std::wstring var_name = L"test_variable";
++ std::string set_data = "UEFI variable data string";
++ std::string get_data;
++
++ efi_status = m_client->set_variable(
++ m_common_guid,
++ var_name,
++ set_data,
++ EFI_VARIABLE_NON_VOLATILE);
++
++ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
++
++ /* Get with the data size set to zero. This is the standard way
++ * to discover the variable size. */
++ efi_status = m_client->get_variable(
++ m_common_guid,
++ var_name,
++ get_data,
++ 0, 0);
++
++ UNSIGNED_LONGLONGS_EQUAL(EFI_BUFFER_TOO_SMALL, efi_status);
++ UNSIGNED_LONGS_EQUAL(set_data.size(), get_data.size());
++
++ /* Expect remove to be permitted */
++ efi_status = m_client->remove_variable(m_common_guid, var_name);
++ UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
++}
++
+ TEST(SmmVariableServiceTests, enumerateStoreContents)
+ {
+ efi_status_t efi_status = EFI_SUCCESS;
+--
+2.17.1
+
@@ -59,6 +59,7 @@ SRC_URI:append = " \
file://0046-Fix-update-psa_set_key_usage_flags-definition-to-the.patch \
file://0047-Fixes-in-AEAD-for-psa-arch-test-54-and-58.patch \
file://0003-corstone1000-port-crypto-config.patch;patchdir=../psa-arch-tests \
+ file://0048-Fix-UEFI-get_variable-with-small-buffer.patch \
"
SRC_URI_MBEDTLS = "git://github.com/ARMmbed/mbedtls.git;protocol=https;branch=development;name=mbedtls;destsuffix=git/mbedtls"