diff --git a/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0012-Revert-Make-variable-index-usage-robust-with-redunda.patch b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0012-Revert-Make-variable-index-usage-robust-with-redunda.patch new file mode 100644 index 00000000..8b5f062f --- /dev/null +++ b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0012-Revert-Make-variable-index-usage-robust-with-redunda.patch @@ -0,0 +1,716 @@ +From f0c1c15fc8886349f7aeb04e1328472894b674d7 Mon Sep 17 00:00:00 2001 +From: Bence Balogh +Date: Mon, 25 Nov 2024 22:11:33 +0100 +Subject: [PATCH 1/3] Revert "Make variable index usage robust with redundancy" + +This reverts commit 8e394bbfae1dccf86a6a5428471e1b10fdaa74ec. +This revert is needed because the FMP support added for Corstone-1000 only +works if the SMM_VARIABLE_INDEX_STORAGE_UID is 0x787. See the implementation +in the 0003-FMP-Support-in-Corstone1000.patch file. + +The 0003-FMP-Support-in-Corstone1000.patch is also inappropriate and will be +redesigned. Instead of fixing that patch, revert this redundancy feature until +the FMP support is redesigned. + +Upstream-Status: Inappropriate [To be removed after new FWU design] +Signed-off-by: Bence Balogh +--- + .../backend/test/variable_index_tests.cpp | 50 ++--- + .../backend/test/variable_store_tests.cpp | 166 ---------------- + .../backend/uefi_variable_store.c | 181 +++--------------- + .../backend/uefi_variable_store.h | 1 - + .../smm_variable/backend/variable_index.c | 29 +-- + .../smm_variable/backend/variable_index.h | 3 +- + 6 files changed, 46 insertions(+), 384 deletions(-) + +diff --git a/components/service/uefi/smm_variable/backend/test/variable_index_tests.cpp b/components/service/uefi/smm_variable/backend/test/variable_index_tests.cpp +index cf0f6a12e..a52cfbf76 100644 +--- a/components/service/uefi/smm_variable/backend/test/variable_index_tests.cpp ++++ b/components/service/uefi/smm_variable/backend/test/variable_index_tests.cpp +@@ -208,8 +208,7 @@ TEST(UefiVariableIndexTests, enumerateStore) + + TEST(UefiVariableIndexTests, dumpLoadRoadtrip) + { +- uint8_t buffer[sizeof(uint32_t) + +- MAX_VARIABLES * (sizeof(struct variable_metadata) + sizeof(bool))]; ++ uint8_t buffer[MAX_VARIABLES * sizeof(struct variable_metadata)]; + + create_variables(); + +@@ -223,13 +222,7 @@ TEST(UefiVariableIndexTests, dumpLoadRoadtrip) + + CHECK_TRUE(is_dirty); + UNSIGNED_LONGS_EQUAL(EFI_SUCCESS, status); +- /* +- * Variable index counter is at the beginning, which is followed by metadata and +- * constraint status byte of both NV variables +- */ +- UNSIGNED_LONGS_EQUAL(sizeof(uint32_t) + +- ((sizeof(struct variable_metadata) + sizeof(bool)) * 2), +- dump_len); ++ UNSIGNED_LONGS_EQUAL(((sizeof(struct variable_metadata) + sizeof(bool)) * 2), dump_len); + + /* Expect no records to be dirty when the dump is repeated */ + dump_len = 0; +@@ -238,9 +231,7 @@ TEST(UefiVariableIndexTests, dumpLoadRoadtrip) + + UNSIGNED_LONGS_EQUAL(EFI_SUCCESS, status); + CHECK_FALSE(is_dirty); +- UNSIGNED_LONGS_EQUAL(sizeof(uint32_t) + +- ((sizeof(struct variable_metadata) + sizeof(bool)) * 2), +- dump_len); ++ UNSIGNED_LONGS_EQUAL(((sizeof(struct variable_metadata) + sizeof(bool)) * 2), dump_len); + + /* Tear down and reinitialize to simulate a reboot */ + variable_index_deinit(&m_variable_index); +@@ -279,8 +270,7 @@ TEST(UefiVariableIndexTests, dumpLoadRoadtrip) + + TEST(UefiVariableIndexTests, dumpLoadConstrainedVariable) + { +- uint8_t buffer[sizeof(uint32_t) + +- MAX_VARIABLES * (sizeof(struct variable_metadata) + sizeof(bool))]; ++ uint8_t buffer[MAX_VARIABLES * sizeof(struct variable_metadata)]; + + create_variables(); + +@@ -314,13 +304,8 @@ TEST(UefiVariableIndexTests, dumpLoadConstrainedVariable) + UNSIGNED_LONGS_EQUAL(EFI_SUCCESS, status); + CHECK_TRUE(is_dirty); + +- /* +- * Variable index counter is at the beginning, which is followed by metadata and +- * constraint status byte of both NV variables, but only one of them has +- * constraints +- */ +- UNSIGNED_LONGS_EQUAL(sizeof(uint32_t) + +- (sizeof(struct variable_metadata) + sizeof(bool)) * 2 + ++ /* metadata and constraint status byte are stored for both NV variables, but only one of them has constraints */ ++ UNSIGNED_LONGS_EQUAL((sizeof(struct variable_metadata) + sizeof(bool)) * 2 + + sizeof(struct variable_constraints), + dump_len); + +@@ -331,11 +316,7 @@ TEST(UefiVariableIndexTests, dumpLoadConstrainedVariable) + + TEST(UefiVariableIndexTests, dumpBufferTooSmall) + { +- /* +- * Enough to fit the variable index counter and the metadata and constraint +- * status of a single variable +- */ +- uint8_t buffer[sizeof(uint32_t) + sizeof(struct variable_metadata) + sizeof(bool)]; ++ uint8_t buffer[1 * sizeof(struct variable_metadata) + 1]; + + create_variables(); + +@@ -357,8 +338,7 @@ TEST(UefiVariableIndexTests, dumpBufferTooSmall) + + TEST(UefiVariableIndexTests, removeVariable) + { +- uint8_t buffer[sizeof(uint32_t) + +- MAX_VARIABLES * (sizeof(struct variable_metadata) + sizeof(bool))]; ++ uint8_t buffer[MAX_VARIABLES * sizeof(struct variable_metadata)]; + struct variable_info *info = NULL; + + create_variables(); +@@ -378,12 +358,7 @@ TEST(UefiVariableIndexTests, removeVariable) + + CHECK_TRUE(is_dirty); + UNSIGNED_LONGS_EQUAL(EFI_SUCCESS, status); +- /* +- * Dump to now contains the variable index counter and metadata, +- * constraint status data of a variable +- */ +- UNSIGNED_LONGS_EQUAL(sizeof(uint32_t) + sizeof(struct variable_metadata) + sizeof(bool), +- dump_len); ++ UNSIGNED_LONGS_EQUAL(sizeof(struct variable_metadata) + sizeof(bool), dump_len); + + /* Remove the volatile variable */ + info = variable_index_find(&m_variable_index, &guid_1, string_get_size_in_bytes(name_1), +@@ -398,8 +373,7 @@ TEST(UefiVariableIndexTests, removeVariable) + + CHECK_FALSE(is_dirty); + UNSIGNED_LONGS_EQUAL(EFI_SUCCESS, status); +- UNSIGNED_LONGS_EQUAL(sizeof(uint32_t) + sizeof(struct variable_metadata) + sizeof(bool), +- dump_len); ++ UNSIGNED_LONGS_EQUAL(sizeof(struct variable_metadata) + sizeof(bool), dump_len); + + /* Remove the remaining NV variable */ + info = variable_index_find(&m_variable_index, &guid_1, string_get_size_in_bytes(name_3), +@@ -407,14 +381,14 @@ TEST(UefiVariableIndexTests, removeVariable) + + variable_index_clear_variable(&m_variable_index, info); + +- /* Expect index to be dirty and dump to now contains only the variable index counter */ ++ /* Expect index to be dirty and dump to now be empty */ + dump_len = 0; + status = variable_index_dump(&m_variable_index, sizeof(buffer), buffer, &dump_len, + &is_dirty); + + CHECK_TRUE(is_dirty); + UNSIGNED_LONGS_EQUAL(EFI_SUCCESS, status); +- UNSIGNED_LONGS_EQUAL(sizeof(uint32_t), dump_len); ++ UNSIGNED_LONGS_EQUAL(0, dump_len); + + /* Enumerate and now expect an empty index */ + info = NULL; +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 0f962f206..e0f21f77a 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 +@@ -5,7 +5,6 @@ + */ + + #include +-#include + #include + #include + #include +@@ -270,17 +269,8 @@ TEST_GROUP(UefiVariableStoreTests) + static const size_t MAX_VARIABLES = 5; + static const size_t MAX_VARIABLE_SIZE = 3000; + static const size_t STORE_CAPACITY = MAX_VARIABLES * MAX_VARIABLE_SIZE; +- static const size_t VARIABLE_INDEX_MAX_SIZE = +- sizeof(uint32_t) + +- MAX_VARIABLES * (sizeof(struct variable_metadata) + +- sizeof(struct variable_constraints) + sizeof(bool)); + + static const uint32_t OWNER_ID = 100; +- +- /* Synchronize these with the variables with the store */ +- uint64_t DEFAULT_VARIABLE_INDEX_STORAGE_A_UID = 0x8000000000000001; +- uint64_t DEFAULT_VARIABLE_INDEX_STORAGE_B_UID = 0x8000000000000002; +- + /* + * Make sure the variable buffer in the test is way above the limit + * so the buffer problems will be handled by the component +@@ -873,159 +863,3 @@ TEST(UefiVariableStoreTests, fillIndex) + LONGS_EQUAL(0, input_data.compare(output_data)); + } + } +- +-TEST(UefiVariableStoreTests, variableIndexCounterOverflow) +-{ +- efi_status_t efi_status = EFI_SUCCESS; +- psa_status_t psa_status = PSA_SUCCESS; +- std::u16string var_name = u"var"; +- std::string input_data = "a"; +- uint32_t attributes = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | +- EFI_VARIABLE_RUNTIME_ACCESS; +- /* There are no variables set in the index, only the counter is there */ +- uint8_t buffer[sizeof(uint32_t)] = { 0 }; +- +- mock_store_reset(&m_persistent_store); +- +- /* Counter of index A is 0 */ +- psa_status = m_persistent_store.backend.interface->set( +- m_persistent_store.backend.context, OWNER_ID, DEFAULT_VARIABLE_INDEX_STORAGE_A_UID, +- sizeof(buffer), &buffer, PSA_STORAGE_FLAG_NONE); +- UNSIGNED_LONGLONGS_EQUAL(PSA_SUCCESS, psa_status); +- +- /* Set max counter value */ +- buffer[0] = 0xFF; +- buffer[1] = 0xFF; +- buffer[2] = 0xFF; +- buffer[3] = 0xFF; +- +- /* Counter of index B is max value */ +- psa_status = m_persistent_store.backend.interface->set( +- m_persistent_store.backend.context, OWNER_ID, DEFAULT_VARIABLE_INDEX_STORAGE_B_UID, +- sizeof(buffer), &buffer, PSA_STORAGE_FLAG_NONE); +- UNSIGNED_LONGLONGS_EQUAL(PSA_SUCCESS, psa_status); +- +- /* At next initialization of the store index A should be the latest index with counter value 0 */ +- uefi_variable_store_deinit(&m_uefi_variable_store); +- +- efi_status = uefi_variable_store_init(&m_uefi_variable_store, OWNER_ID, MAX_VARIABLES, +- m_persistent_backend, m_volatile_backend); +- UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status); +- +- UNSIGNED_LONGLONGS_EQUAL(m_uefi_variable_store.active_variable_index_uid, +- DEFAULT_VARIABLE_INDEX_STORAGE_A_UID); +- UNSIGNED_LONGLONGS_EQUAL(m_uefi_variable_store.variable_index.counter, 0); +- +- /* After setting a variable to trigger sync and rebooting index B should be the latest index with counter value 1*/ +- efi_status = set_variable(var_name, input_data, attributes); +- UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status); +- +- power_cycle(); +- +- UNSIGNED_LONGLONGS_EQUAL(m_uefi_variable_store.active_variable_index_uid, +- DEFAULT_VARIABLE_INDEX_STORAGE_B_UID); +- UNSIGNED_LONGLONGS_EQUAL(m_uefi_variable_store.variable_index.counter, 1); +-} +- +-TEST(UefiVariableStoreTests, oneEmptyVariableIndexExists) +-{ +- psa_status_t status = PSA_SUCCESS; +- +- /* Only, variable index A exists, but it is empty */ +- mock_store_reset(&m_persistent_store); +- +- status = m_persistent_store.backend.interface->create(m_persistent_store.backend.context, +- OWNER_ID, +- DEFAULT_VARIABLE_INDEX_STORAGE_A_UID, +- 100, PSA_STORAGE_FLAG_NONE); +- UNSIGNED_LONGLONGS_EQUAL(PSA_SUCCESS, status); +- +- power_cycle(); +- +- /* Empty index is considered non-existing so default index (A) is selected */ +- UNSIGNED_LONGLONGS_EQUAL(m_uefi_variable_store.active_variable_index_uid, +- DEFAULT_VARIABLE_INDEX_STORAGE_A_UID); +- UNSIGNED_LONGLONGS_EQUAL(m_uefi_variable_store.variable_index.counter, 0); +- +- /* Only, variable index B exists, but it is empty*/ +- mock_store_reset(&m_persistent_store); +- +- status = m_persistent_store.backend.interface->create(m_persistent_store.backend.context, +- OWNER_ID, +- DEFAULT_VARIABLE_INDEX_STORAGE_B_UID, +- 100, PSA_STORAGE_FLAG_NONE); +- UNSIGNED_LONGLONGS_EQUAL(PSA_SUCCESS, status); +- +- power_cycle(); +- +- /* Empty index is considered non-existing so default index (A) is selected */ +- UNSIGNED_LONGLONGS_EQUAL(m_uefi_variable_store.active_variable_index_uid, +- DEFAULT_VARIABLE_INDEX_STORAGE_A_UID); +- UNSIGNED_LONGLONGS_EQUAL(m_uefi_variable_store.variable_index.counter, 0); +-} +- +-TEST(UefiVariableStoreTests, oneVariableIndexAlreadySet) +-{ +- efi_status_t status = EFI_SUCCESS; +- /* Empty variable index with zero counter value */ +- uint8_t buffer[VARIABLE_INDEX_MAX_SIZE] = { 0 }; +- +- /* Set index A in the store with some data, so it will be found as the currently active index */ +- mock_store_reset(&m_persistent_store); +- +- status = m_persistent_store.backend.interface->set( +- m_persistent_store.backend.context, OWNER_ID, DEFAULT_VARIABLE_INDEX_STORAGE_A_UID, +- sizeof(buffer), &buffer, PSA_STORAGE_FLAG_NONE); +- UNSIGNED_LONGLONGS_EQUAL(PSA_SUCCESS, status); +- +- power_cycle(); +- +- UNSIGNED_LONGLONGS_EQUAL(m_uefi_variable_store.active_variable_index_uid, +- DEFAULT_VARIABLE_INDEX_STORAGE_A_UID); +- UNSIGNED_LONGLONGS_EQUAL(m_uefi_variable_store.variable_index.counter, 0); +- +- /* Set index B in the store with some data, so it will be found as the currently active index */ +- mock_store_reset(&m_persistent_store); +- +- status = m_persistent_store.backend.interface->set( +- m_persistent_store.backend.context, OWNER_ID, DEFAULT_VARIABLE_INDEX_STORAGE_B_UID, +- sizeof(buffer), &buffer, PSA_STORAGE_FLAG_NONE); +- UNSIGNED_LONGLONGS_EQUAL(PSA_SUCCESS, status); +- +- power_cycle(); +- +- UNSIGNED_LONGLONGS_EQUAL(m_uefi_variable_store.active_variable_index_uid, +- DEFAULT_VARIABLE_INDEX_STORAGE_B_UID); +- UNSIGNED_LONGLONGS_EQUAL(m_uefi_variable_store.variable_index.counter, 0); +-} +- +-TEST(UefiVariableStoreTests, variableIndexesWithSameData) +-{ +- psa_status_t psa_status = PSA_SUCCESS; +- efi_status_t efi_status = EFI_SUCCESS; +- /* Empty variable index with zero counter value */ +- uint8_t buffer[VARIABLE_INDEX_MAX_SIZE] = { 0 }; +- +- /* Set both indexes to the same data and counter value */ +- mock_store_reset(&m_persistent_store); +- +- psa_status = m_persistent_store.backend.interface->set( +- m_persistent_store.backend.context, OWNER_ID, DEFAULT_VARIABLE_INDEX_STORAGE_A_UID, +- sizeof(buffer), &buffer, PSA_STORAGE_FLAG_NONE); +- UNSIGNED_LONGLONGS_EQUAL(PSA_SUCCESS, psa_status); +- +- psa_status = m_persistent_store.backend.interface->set( +- m_persistent_store.backend.context, OWNER_ID, DEFAULT_VARIABLE_INDEX_STORAGE_B_UID, +- sizeof(buffer), &buffer, PSA_STORAGE_FLAG_NONE); +- UNSIGNED_LONGLONGS_EQUAL(PSA_SUCCESS, psa_status); +- +- /* +- * Initializing the store should fail, because if there are two indexes with the same counter it cannot be decided +- * which has the valid data. +- */ +- uefi_variable_store_deinit(&m_uefi_variable_store); +- +- efi_status = uefi_variable_store_init(&m_uefi_variable_store, OWNER_ID, MAX_VARIABLES, +- m_persistent_backend, m_volatile_backend); +- UNSIGNED_LONGLONGS_EQUAL(EFI_LOAD_ERROR, efi_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 48b81ff37..459ca4566 100644 +--- a/components/service/uefi/smm_variable/backend/uefi_variable_store.c ++++ b/components/service/uefi/smm_variable/backend/uefi_variable_store.c +@@ -28,12 +28,9 @@ + #include "service/crypto/client/psa/crypto_client.h" + #endif + +-static psa_status_t get_active_variable_uid(struct uefi_variable_store *context, +- uint64_t *active_index_uid, uint32_t *counter); +- + static efi_status_t load_variable_index(struct uefi_variable_store *context); + +-static efi_status_t sync_variable_index(struct uefi_variable_store *context); ++static efi_status_t sync_variable_index(const struct uefi_variable_store *context); + + static efi_status_t check_capabilities(const SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var); + +@@ -138,14 +135,6 @@ static bool compare_name_to_key_store_name(const int16_t *name1, size_t size1, + const uint16_t *name2, size_t size2); + #endif + +-/* Private UID for storing the variable index */ +-#define SMM_VARIABLE_INDEX_STORAGE_A_UID UINT64_C(0x8000000000000001) +-#define SMM_VARIABLE_INDEX_STORAGE_B_UID UINT64_C(0x8000000000000002) +- +-_Static_assert(SMM_VARIABLE_INDEX_STORAGE_A_UID != SMM_VARIABLE_INDEX_STORAGE_B_UID, +- "SMM_VARIABLE_INDEX_STORAGE_A_UID must not be the same value as " +- "SMM_VARIABLE_INDEX_STORAGE_B_UID"); +- + /* Default maximum variable size - + * may be overridden using uefi_variable_store_set_storage_limits() + */ +@@ -398,7 +387,7 @@ efi_status_t uefi_variable_store_set_variable(const struct uefi_variable_store * + * index entry. + */ + if (should_sync_index) +- status = sync_variable_index((struct uefi_variable_store *)context); ++ status = sync_variable_index(context); + + /* Store any variable data to the storage backend with the updated metadata */ + if (info->is_variable_set && (status == EFI_SUCCESS)) { +@@ -620,148 +609,40 @@ efi_status_t uefi_variable_store_get_var_check_property( + return status; + } + +-/* Checks which index contains the latest data, which shall be loaded */ +-static psa_status_t get_active_variable_uid(struct uefi_variable_store *context, +- uint64_t *active_index_uid, uint32_t *counter) +-{ +- uint32_t counter_A = 0; +- uint32_t counter_B = 0; +- size_t data_len = 0; +- psa_status_t psa_status_A = PSA_SUCCESS; +- psa_status_t psa_status_B = PSA_SUCCESS; +- struct storage_backend *persistent_store = context->persistent_store.storage_backend; +- +- /* Set default value for the case when the index does not exist yet */ +- *active_index_uid = SMM_VARIABLE_INDEX_STORAGE_A_UID; +- *counter = 0; +- +- if (persistent_store) { +- psa_status_A = persistent_store->interface->get(persistent_store->context, +- context->owner_id, +- SMM_VARIABLE_INDEX_STORAGE_A_UID, 0, +- sizeof(counter_A), &counter_A, +- &data_len); +- +- if (psa_status_A == PSA_SUCCESS && data_len == 0) { +- psa_status_A = persistent_store->interface->remove( +- persistent_store->context, context->owner_id, +- SMM_VARIABLE_INDEX_STORAGE_A_UID); +- +- if (psa_status_A == PSA_SUCCESS) +- psa_status_A = PSA_ERROR_DOES_NOT_EXIST; +- else { +- EMSG("Erronous state of variable index"); +- return PSA_ERROR_STORAGE_FAILURE; +- } +- } +- +- psa_status_B = persistent_store->interface->get(persistent_store->context, +- context->owner_id, +- SMM_VARIABLE_INDEX_STORAGE_B_UID, 0, +- sizeof(counter_B), &counter_B, +- &data_len); +- +- if (psa_status_B == PSA_SUCCESS && data_len == 0) { +- psa_status_B = persistent_store->interface->remove( +- persistent_store->context, context->owner_id, +- SMM_VARIABLE_INDEX_STORAGE_B_UID); +- +- if (psa_status_B == PSA_SUCCESS) +- psa_status_B = PSA_ERROR_DOES_NOT_EXIST; +- else { +- EMSG("Erronous state of variable index"); +- return PSA_ERROR_STORAGE_FAILURE; +- } +- } +- +- if ((psa_status_A != PSA_SUCCESS && psa_status_A != PSA_ERROR_DOES_NOT_EXIST) || +- (psa_status_B != PSA_SUCCESS && psa_status_B != PSA_ERROR_DOES_NOT_EXIST)) +- return PSA_ERROR_STORAGE_FAILURE; +- +- if (psa_status_A == PSA_ERROR_DOES_NOT_EXIST) { +- if (psa_status_B == PSA_ERROR_DOES_NOT_EXIST) +- return PSA_ERROR_DOES_NOT_EXIST; +- +- *active_index_uid = SMM_VARIABLE_INDEX_STORAGE_B_UID; +- *counter = counter_B; +- +- return PSA_SUCCESS; +- } else if (psa_status_B == PSA_ERROR_DOES_NOT_EXIST) { +- *active_index_uid = SMM_VARIABLE_INDEX_STORAGE_A_UID; +- *counter = counter_A; +- +- return PSA_SUCCESS; +- } +- +- if (counter_A + 1 == counter_B) { +- *active_index_uid = SMM_VARIABLE_INDEX_STORAGE_B_UID; +- *counter = counter_B; +- return PSA_SUCCESS; +- } else if (counter_B + 1 == counter_A) { +- *active_index_uid = SMM_VARIABLE_INDEX_STORAGE_A_UID; +- *counter = counter_A; +- return PSA_SUCCESS; +- } else { +- EMSG("UEFI metadata variable index is invalid."); +- return PSA_ERROR_STORAGE_FAILURE; +- } +- } else { +- EMSG("Store backend is not accessible"); +- return PSA_ERROR_STORAGE_FAILURE; +- } +- +- return PSA_ERROR_STORAGE_FAILURE; +-} +- + static efi_status_t load_variable_index(struct uefi_variable_store *context) + { + struct storage_backend *persistent_store = context->persistent_store.storage_backend; +- psa_status_t psa_status = PSA_SUCCESS; + + if (persistent_store) { + size_t data_len = 0; + size_t data_offset = 0; +- struct psa_storage_info_t variable_index_info = { 0 }; +- +- psa_status = get_active_variable_uid(context, &context->active_variable_index_uid, +- &context->variable_index.counter); +- switch (psa_status) { +- case PSA_SUCCESS: +- break; +- +- case PSA_ERROR_DOES_NOT_EXIST: +- IMSG("Variable index does not exist in NV store, continuing with empty index"); +- return EFI_SUCCESS; +- +- default: +- EMSG("Loading variable index failed: %d", psa_status); +- return EFI_LOAD_ERROR; +- } +- +- /* Make sure the variable index fits the buffer */ +- persistent_store->interface->get_info(persistent_store->context, context->owner_id, +- context->active_variable_index_uid, +- &variable_index_info); +- +- if (variable_index_info.size > context->index_sync_buffer_size) { +- EMSG("Variable index cannot fit the sync buffer"); +- return EFI_LOAD_ERROR; +- } + + do { +- psa_status = persistent_store->interface->get( ++ psa_status_t psa_status = persistent_store->interface->get( + persistent_store->context, context->owner_id, +- context->active_variable_index_uid, data_offset, ++ SMM_VARIABLE_INDEX_STORAGE_UID, data_offset, + RPC_CALLER_SESSION_SHARED_MEMORY_SIZE, + context->index_sync_buffer + data_offset, &data_len); + +- if (psa_status != PSA_SUCCESS) { ++ switch (psa_status) { ++ case PSA_SUCCESS: ++ data_offset += data_len; ++ ++ if (data_offset > context->index_sync_buffer_size) { ++ EMSG("Variable index cannot fit the sync buffer"); ++ return EFI_LOAD_ERROR; ++ } ++ ++ break; ++ ++ case PSA_ERROR_DOES_NOT_EXIST: ++ IMSG("Index variable does not exist in NV store, continuing with empty index"); ++ return EFI_SUCCESS; ++ ++ default: + EMSG("Loading variable index failed: %d", psa_status); + return EFI_LOAD_ERROR; + } +- +- data_offset += data_len; +- + } while (data_len == RPC_CALLER_SESSION_SHARED_MEMORY_SIZE); + + variable_index_restore(&context->variable_index, data_offset, +@@ -774,7 +655,7 @@ static efi_status_t load_variable_index(struct uefi_variable_store *context) + return EFI_SUCCESS; + } + +-static efi_status_t sync_variable_index(struct uefi_variable_store *context) ++static efi_status_t sync_variable_index(const struct uefi_variable_store *context) + { + efi_status_t status = EFI_SUCCESS; + psa_status_t psa_status = PSA_SUCCESS; +@@ -794,24 +675,19 @@ static efi_status_t sync_variable_index(struct uefi_variable_store *context) + + if (persistent_store) { + size_t data_offset = 0; +- uint64_t next_index_uid = 0; +- +- /* Write the older one */ +- next_index_uid = (context->active_variable_index_uid == +- SMM_VARIABLE_INDEX_STORAGE_A_UID ? +- SMM_VARIABLE_INDEX_STORAGE_B_UID : +- SMM_VARIABLE_INDEX_STORAGE_A_UID); + + psa_status = persistent_store->interface->remove( +- persistent_store->context, context->owner_id, next_index_uid); ++ persistent_store->context, context->owner_id, ++ SMM_VARIABLE_INDEX_STORAGE_UID); + + if (psa_status != PSA_SUCCESS && psa_status != PSA_ERROR_DOES_NOT_EXIST) + goto end; + + /* Check if the index exists and create if not yet */ + psa_status = persistent_store->interface->create( +- persistent_store->context, context->owner_id, next_index_uid, +- remaining_data_len, PSA_STORAGE_FLAG_NONE); ++ persistent_store->context, context->owner_id, ++ SMM_VARIABLE_INDEX_STORAGE_UID, remaining_data_len, ++ PSA_STORAGE_FLAG_NONE); + + if (psa_status != PSA_SUCCESS) + goto end; +@@ -822,7 +698,8 @@ static efi_status_t sync_variable_index(struct uefi_variable_store *context) + + psa_status = persistent_store->interface->set_extended( + persistent_store->context, context->owner_id, +- next_index_uid, data_offset, data_of_this_iteration, ++ SMM_VARIABLE_INDEX_STORAGE_UID, data_offset, ++ data_of_this_iteration, + context->index_sync_buffer + data_offset); + + if (psa_status != PSA_SUCCESS) +@@ -1827,7 +1704,7 @@ static void purge_orphan_index_entries(const struct uefi_variable_store *context + } + + if (any_orphans) +- sync_variable_index((struct uefi_variable_store *)context); ++ sync_variable_index(context); + } + + static struct delegate_variable_store * +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 9f2c4a00c..2493ff6b4 100644 +--- a/components/service/uefi/smm_variable/backend/uefi_variable_store.h ++++ b/components/service/uefi/smm_variable/backend/uefi_variable_store.h +@@ -52,7 +52,6 @@ struct uefi_variable_store { + uint32_t owner_id; + uint8_t *index_sync_buffer; + size_t index_sync_buffer_size; +- uint64_t active_variable_index_uid; + struct variable_index variable_index; + struct delegate_variable_store persistent_store; + struct delegate_variable_store volatile_store; +diff --git a/components/service/uefi/smm_variable/backend/variable_index.c b/components/service/uefi/smm_variable/backend/variable_index.c +index 90230426f..5fb6d08c5 100644 +--- a/components/service/uefi/smm_variable/backend/variable_index.c ++++ b/components/service/uefi/smm_variable/backend/variable_index.c +@@ -91,7 +91,6 @@ static struct variable_entry *containing_entry(const struct variable_info *info) + efi_status_t variable_index_init(struct variable_index *context, size_t max_variables) + { + context->max_variables = max_variables; +- context->counter = 0; + context->entries = + (struct variable_entry *)malloc(sizeof(struct variable_entry) * max_variables); + +@@ -109,9 +108,9 @@ void variable_index_deinit(struct variable_index *context) + + size_t variable_index_max_dump_size(struct variable_index *context) + { +- return sizeof(context->counter) + (sizeof(struct variable_metadata) + sizeof(bool) + +- sizeof(struct variable_constraints)) * +- context->max_variables; ++ return (sizeof(struct variable_metadata) + sizeof(bool) + ++ sizeof(struct variable_constraints)) * ++ context->max_variables; + } + + struct variable_info *variable_index_find(const struct variable_index *context, +@@ -288,16 +287,6 @@ efi_status_t variable_index_dump(const struct variable_index *context, size_t bu + *data_len = 0; + *any_dirty = false; + +- /* +- * Intentionally letting the counter overflow. +- * The buffer (index_sync_buffer) is provided by malloc, which allocates memory to a boundary +- * suitable for any default data type of the system (e.g uint32_t) +- */ +- *((uint32_t *)dump_pos) = context->counter + 1; +- bytes_dumped += sizeof(context->counter); +- dump_pos += sizeof(context->counter); +- +- /* Store variables */ + for (size_t pos = 0; pos < context->max_variables; pos++) { + struct variable_entry *entry = &context->entries[pos]; + struct variable_metadata *metadata = &entry->info.metadata; +@@ -344,24 +333,14 @@ efi_status_t variable_index_dump(const struct variable_index *context, size_t bu + return EFI_SUCCESS; + } + +-void variable_index_confirm_write(struct variable_index *context) +-{ +- context->counter++; +-} + +-size_t variable_index_restore(struct variable_index *context, size_t data_len, ++size_t variable_index_restore(const struct variable_index *context, size_t data_len, + const uint8_t *buffer) + { + size_t bytes_loaded = 0; + const uint8_t *load_pos = buffer; + int pos = 0; + +- if (data_len >= sizeof(context->counter)) { +- context->counter = *((uint32_t *)load_pos); +- bytes_loaded += sizeof(context->counter); +- load_pos += sizeof(context->counter); +- } +- + while (bytes_loaded < data_len) { + struct variable_entry *entry = &context->entries[pos]; + +diff --git a/components/service/uefi/smm_variable/backend/variable_index.h b/components/service/uefi/smm_variable/backend/variable_index.h +index 592dddc83..0151d636a 100644 +--- a/components/service/uefi/smm_variable/backend/variable_index.h ++++ b/components/service/uefi/smm_variable/backend/variable_index.h +@@ -75,7 +75,6 @@ struct variable_entry { + */ + struct variable_index { + size_t max_variables; +- uint32_t counter; + struct variable_entry *entries; + }; + +@@ -229,7 +228,7 @@ void variable_index_confirm_write(struct variable_index *context); + * + * @return Number of bytes loaded + */ +-size_t variable_index_restore(struct variable_index *context, size_t data_len, ++size_t variable_index_restore(const struct variable_index *context, size_t data_len, + const uint8_t *buffer); + + #ifdef __cplusplus +-- +2.25.1 + diff --git a/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0013-Revert-Load-and-store-UEFI-variable-index-in-chunks.patch b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0013-Revert-Load-and-store-UEFI-variable-index-in-chunks.patch new file mode 100644 index 00000000..09fa94fc --- /dev/null +++ b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0013-Revert-Load-and-store-UEFI-variable-index-in-chunks.patch @@ -0,0 +1,291 @@ +From c490956e50e721c8f2db5934ae5af365ba92e55a Mon Sep 17 00:00:00 2001 +From: Bence Balogh +Date: Mon, 25 Nov 2024 22:13:15 +0100 +Subject: [PATCH 2/3] Revert "Load and store UEFI variable index in chunks" + +This reverts commit a0a08571084238af2a24d4e6e580308f86ab59a2. +The PSA IPC backend for the Protected Storage doesn't support the optional +create() and set_extended() APIs. This feature has to be reverted because +of this. +Keep this inappropriate patch until the usage of create() and set_extended() +APIs are not optional in the SMM-Gateway. + +Upstream-Status: Inappropriate [To be redesigned] +Signed-off-by: Bence Balogh +--- + .../backend/test/variable_store_tests.cpp | 100 +----------------- + .../backend/uefi_variable_store.c | 84 +++------------ + deployments/smm-gateway/common/smm_gateway.c | 4 + + 3 files changed, 22 insertions(+), 166 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 e0f21f77a..2a8c8eb94 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 +@@ -56,12 +56,6 @@ TEST_GROUP(UefiVariableStoreTests) + return var_name; + } + +- std::u16string intToChar16(const int i) +- { +- auto s = std::to_string(i); +- return { s.begin(), s.end() }; +- } +- + size_t string_get_size_in_bytes(const std::u16string &string) + { + return string.size() * sizeof(uint16_t); +@@ -266,9 +260,9 @@ TEST_GROUP(UefiVariableStoreTests) + MAX_VARIABLE_SIZE); + } + +- static const size_t MAX_VARIABLES = 5; ++ static const size_t MAX_VARIABLES = 10; + static const size_t MAX_VARIABLE_SIZE = 3000; +- static const size_t STORE_CAPACITY = MAX_VARIABLES * MAX_VARIABLE_SIZE; ++ static const size_t STORE_CAPACITY = 10000; + + static const uint32_t OWNER_ID = 100; + /* +@@ -773,93 +767,3 @@ TEST(UefiVariableStoreTests, noRemoveCheck) + EFI_VARIABLE_NON_VOLATILE); + UNSIGNED_LONGLONGS_EQUAL(EFI_INVALID_PARAMETER, status); + } +- +-TEST(UefiVariableStoreTests, fillStore) +-{ +- efi_status_t status = EFI_SUCCESS; +- +- /* Fill the variable store with max size variables */ +- for (size_t i = 0; i < MAX_VARIABLES; i++) { +- std::u16string current_var = u"var_"; +- std::string input_data(MAX_VARIABLE_SIZE, 'a'); +- std::string output_data; +- current_var += intToChar16(i); +- +- status = set_variable(current_var, input_data, +- EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | +- EFI_VARIABLE_RUNTIME_ACCESS); +- UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status); +- +- /* Verify the write */ +- status = get_variable(current_var, 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)); +- } +- +- /* Try adding a small variable to an already full store */ +- status = set_variable(u"var", "a", +- EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | +- EFI_VARIABLE_RUNTIME_ACCESS); +- UNSIGNED_LONGLONGS_EQUAL(EFI_OUT_OF_RESOURCES, status); +-} +- +-TEST(UefiVariableStoreTests, fillIndex) +-{ +- efi_status_t status = EFI_SUCCESS; +- std::u16string var_name = u"var"; +- std::string input_data = "a"; +- std::string output_data; +- +- /* +- * Fill the variable store with small variables so the index +- * will be filled, but the store does not +- */ +- for (size_t i = 0; i < MAX_VARIABLES; i++) { +- std::u16string current_var = u"var_"; +- current_var += intToChar16(i); +- +- status = set_variable(current_var, input_data, +- EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | +- EFI_VARIABLE_RUNTIME_ACCESS); +- UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status); +- +- /* Verify the write */ +- status = get_variable(current_var, 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)); +- } +- +- /* Try adding a small variable to an already full store */ +- input_data.resize(1, 'a'); +- +- status = set_variable(u"var", input_data, +- EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | +- EFI_VARIABLE_RUNTIME_ACCESS); +- UNSIGNED_LONGLONGS_EQUAL(EFI_OUT_OF_RESOURCES, status); +- +- /* Simulate a power-cycle without deleting the NV store content */ +- uefi_variable_store_deinit(&m_uefi_variable_store); +- +- /* Try loading the non-volatile variables */ +- status = uefi_variable_store_init(&m_uefi_variable_store, OWNER_ID, MAX_VARIABLES, +- m_persistent_backend, m_volatile_backend); +- +- UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status); +- +- /* Try reading the previously set variables */ +- for (size_t i = 0; i < MAX_VARIABLES; i++) { +- std::u16string current_var = u"var_"; +- current_var += intToChar16(i); +- +- status = get_variable(current_var, output_data); +- UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status); +- UNSIGNED_LONGLONGS_EQUAL(input_data.size(), output_data.size()); +- LONGS_EQUAL(0, input_data.compare(output_data)); +- } +-} +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 459ca4566..e5fc32864 100644 +--- a/components/service/uefi/smm_variable/backend/uefi_variable_store.c ++++ b/components/service/uefi/smm_variable/backend/uefi_variable_store.c +@@ -615,41 +615,26 @@ static efi_status_t load_variable_index(struct uefi_variable_store *context) + + if (persistent_store) { + size_t data_len = 0; +- size_t data_offset = 0; + +- do { +- psa_status_t psa_status = persistent_store->interface->get( +- persistent_store->context, context->owner_id, +- SMM_VARIABLE_INDEX_STORAGE_UID, data_offset, +- RPC_CALLER_SESSION_SHARED_MEMORY_SIZE, +- context->index_sync_buffer + data_offset, &data_len); ++ psa_status_t psa_status = persistent_store->interface->get( ++ persistent_store->context, context->owner_id, ++ SMM_VARIABLE_INDEX_STORAGE_UID, 0, context->index_sync_buffer_size, ++ context->index_sync_buffer, &data_len); + +- switch (psa_status) { ++ switch(psa_status) { + case PSA_SUCCESS: +- data_offset += data_len; +- +- if (data_offset > context->index_sync_buffer_size) { +- EMSG("Variable index cannot fit the sync buffer"); +- return EFI_LOAD_ERROR; +- } +- ++ (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"); +- return EFI_SUCCESS; ++ break; + + default: + EMSG("Loading variable index failed: %d", psa_status); + return EFI_LOAD_ERROR; +- } +- } while (data_len == RPC_CALLER_SESSION_SHARED_MEMORY_SIZE); +- +- variable_index_restore(&context->variable_index, data_offset, +- context->index_sync_buffer); +- } else { +- EMSG("Loading variable index failed, store backend is not accessible"); +- return EFI_LOAD_ERROR; ++ } + } + + return EFI_SUCCESS; +@@ -658,14 +643,13 @@ 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) + { + efi_status_t status = EFI_SUCCESS; +- psa_status_t psa_status = PSA_SUCCESS; + bool is_dirty = false; + + /* Sync the variable index to storage if anything is dirty */ +- size_t remaining_data_len = 0; ++ size_t data_len = 0; + + status = variable_index_dump(&context->variable_index, context->index_sync_buffer_size, +- context->index_sync_buffer, &remaining_data_len, &is_dirty); ++ context->index_sync_buffer, &data_len, &is_dirty); + if (status != EFI_SUCCESS) + return status; + +@@ -674,52 +658,16 @@ static efi_status_t sync_variable_index(const struct uefi_variable_store *contex + context->persistent_store.storage_backend; + + if (persistent_store) { +- size_t data_offset = 0; +- +- psa_status = persistent_store->interface->remove( ++ psa_status_t psa_status = persistent_store->interface->set( + persistent_store->context, context->owner_id, +- SMM_VARIABLE_INDEX_STORAGE_UID); +- +- if (psa_status != PSA_SUCCESS && psa_status != PSA_ERROR_DOES_NOT_EXIST) +- goto end; ++ SMM_VARIABLE_INDEX_STORAGE_UID, data_len, ++ context->index_sync_buffer, PSA_STORAGE_FLAG_NONE); + +- /* Check if the index exists and create if not yet */ +- psa_status = persistent_store->interface->create( +- persistent_store->context, context->owner_id, +- SMM_VARIABLE_INDEX_STORAGE_UID, remaining_data_len, +- PSA_STORAGE_FLAG_NONE); +- +- if (psa_status != PSA_SUCCESS) +- goto end; +- +- do { +- size_t data_of_this_iteration = MIN( +- remaining_data_len, RPC_CALLER_SESSION_SHARED_MEMORY_SIZE); +- +- psa_status = persistent_store->interface->set_extended( +- persistent_store->context, context->owner_id, +- SMM_VARIABLE_INDEX_STORAGE_UID, data_offset, +- data_of_this_iteration, +- context->index_sync_buffer + data_offset); +- +- if (psa_status != PSA_SUCCESS) +- goto end; +- +- data_offset += RPC_CALLER_SESSION_SHARED_MEMORY_SIZE; +- remaining_data_len -= data_of_this_iteration; +- +- } while (remaining_data_len); +- +- variable_index_confirm_write(&context->variable_index); +- context->active_variable_index_uid = next_index_uid; +- } else { +- EMSG("Syncing variable index failed, store backend is not accessible"); +- return EFI_LOAD_ERROR; ++ status = psa_to_efi_storage_status(psa_status); + } + } + +- end: +- return psa_to_efi_storage_status(psa_status); ++ return status; + } + + /* Check attribute usage rules */ +diff --git a/deployments/smm-gateway/common/smm_gateway.c b/deployments/smm-gateway/common/smm_gateway.c +index 3ab45ccf5..eaa861370 100644 +--- a/deployments/smm-gateway/common/smm_gateway.c ++++ b/deployments/smm-gateway/common/smm_gateway.c +@@ -40,6 +40,10 @@ + #define SMM_UEFI_VARIABLE_STORE_INDEX_SIZE \ + UEFI_VARIABLE_STORE_INDEX_SIZE(SMM_GATEWAY_MAX_UEFI_VARIABLES) + ++_Static_assert(SMM_UEFI_VARIABLE_STORE_INDEX_SIZE < RPC_CALLER_SESSION_SHARED_MEMORY_SIZE, ++ "The UEFI variable index does not fit into the RPC shared memory, please increase " \ ++ "RPC_CALLER_SESSION_SHARED_MEMORY_SIZE"); ++ + /** + * The SP heap must be large enough for storing the UEFI variable index, the RPC shared memory and + * ~16kB of miscellaneous data. +-- +2.25.1 + diff --git a/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0014-Revert-Make-constraints-of-NV-UEFI-variables-persist.patch b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0014-Revert-Make-constraints-of-NV-UEFI-variables-persist.patch new file mode 100644 index 00000000..3f0ae436 --- /dev/null +++ b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0014-Revert-Make-constraints-of-NV-UEFI-variables-persist.patch @@ -0,0 +1,387 @@ +From c0ffa57e7628f23747d7ee947358f8a538fa5d4c Mon Sep 17 00:00:00 2001 +From: Bence Balogh +Date: Mon, 25 Nov 2024 22:17:51 +0100 +Subject: [PATCH 3/3] Revert "Make constraints of NV UEFI variables persistent" + +This reverts commit 64bbde5d9950413cf724ffb792d4d1637892fa8b. +The FMP support didn't work with this commit. See the implementation in the +0003-FMP-Support-in-Corstone1000.patch file. The +0003-FMP-Support-in-Corstone1000.patch will be redesigned but until that, this +commit has to be reverted. + +Upstream-Status: Inappropriate [To be removed after new FWU design] +Signed-off-by: Bence Balogh +--- + .../backend/test/variable_index_tests.cpp | 93 +++---------------- + .../backend/uefi_variable_store.c | 12 +-- + .../smm_variable/backend/variable_index.c | 90 +++--------------- + .../smm_variable/backend/variable_index.h | 7 +- + 4 files changed, 36 insertions(+), 166 deletions(-) + +diff --git a/components/service/uefi/smm_variable/backend/test/variable_index_tests.cpp b/components/service/uefi/smm_variable/backend/test/variable_index_tests.cpp +index a52cfbf76..1b7a6b879 100644 +--- a/components/service/uefi/smm_variable/backend/test/variable_index_tests.cpp ++++ b/components/service/uefi/smm_variable/backend/test/variable_index_tests.cpp +@@ -214,28 +214,21 @@ TEST(UefiVariableIndexTests, dumpLoadRoadtrip) + + /* Expect the info for two NV variables to have been dumped */ + size_t dump_len = 0; +- bool is_dirty = false; +- efi_status_t status = EFI_SUCCESS; +- +- status = variable_index_dump(&m_variable_index, sizeof(buffer), buffer, &dump_len, +- &is_dirty); ++ bool is_dirty = variable_index_dump(&m_variable_index, sizeof(buffer), buffer, &dump_len); + + CHECK_TRUE(is_dirty); +- UNSIGNED_LONGS_EQUAL(EFI_SUCCESS, status); +- UNSIGNED_LONGS_EQUAL(((sizeof(struct variable_metadata) + sizeof(bool)) * 2), dump_len); ++ UNSIGNED_LONGS_EQUAL((sizeof(struct variable_metadata) * 2), dump_len); + + /* Expect no records to be dirty when the dump is repeated */ + dump_len = 0; +- status = variable_index_dump(&m_variable_index, sizeof(buffer), buffer, &dump_len, +- &is_dirty); ++ is_dirty = variable_index_dump(&m_variable_index, sizeof(buffer), buffer, &dump_len); + +- UNSIGNED_LONGS_EQUAL(EFI_SUCCESS, status); + CHECK_FALSE(is_dirty); +- UNSIGNED_LONGS_EQUAL(((sizeof(struct variable_metadata) + sizeof(bool)) * 2), dump_len); ++ UNSIGNED_LONGS_EQUAL((sizeof(struct variable_metadata) * 2), dump_len); + + /* Tear down and reinitialize to simulate a reboot */ + variable_index_deinit(&m_variable_index); +- status = variable_index_init(&m_variable_index, MAX_VARIABLES); ++ efi_status_t status = variable_index_init(&m_variable_index, MAX_VARIABLES); + UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status); + + /* Load the dumped contents */ +@@ -268,52 +261,6 @@ TEST(UefiVariableIndexTests, dumpLoadRoadtrip) + UNSIGNED_LONGLONGS_EQUAL(EFI_NOT_FOUND, status); + } + +-TEST(UefiVariableIndexTests, dumpLoadConstrainedVariable) +-{ +- uint8_t buffer[MAX_VARIABLES * sizeof(struct variable_metadata)]; +- +- create_variables(); +- +- struct variable_constraints constraints; +- constraints.revision = 10; +- constraints.property = VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY; +- constraints.attributes = 0; +- constraints.min_size = 1; +- constraints.max_size = 100; +- +- /* Set check constraints on one of the variables */ +- struct variable_info *info = variable_index_find(&m_variable_index, &guid_2, +- string_get_size_in_bytes(name_2), +- (const int16_t *)name_2.data()); +- +- CHECK_TRUE(info); +- CHECK_TRUE(info->is_variable_set); +- CHECK_FALSE(info->is_constraints_set); +- +- variable_index_set_constraints(info, &constraints); +- +- CHECK_TRUE(info->is_constraints_set); +- CHECK_TRUE(info->is_variable_set); +- +- size_t dump_len = 0; +- bool is_dirty = false; +- efi_status_t status = EFI_SUCCESS; +- status = variable_index_dump(&m_variable_index, sizeof(buffer), buffer, &dump_len, +- &is_dirty); +- +- UNSIGNED_LONGS_EQUAL(EFI_SUCCESS, status); +- CHECK_TRUE(is_dirty); +- +- /* metadata and constraint status byte are stored for both NV variables, but only one of them has constraints */ +- UNSIGNED_LONGS_EQUAL((sizeof(struct variable_metadata) + sizeof(bool)) * 2 + +- sizeof(struct variable_constraints), +- dump_len); +- +- /* Load the dumped contents */ +- size_t load_len = variable_index_restore(&m_variable_index, dump_len, buffer); +- UNSIGNED_LONGS_EQUAL(dump_len, load_len); +-} +- + TEST(UefiVariableIndexTests, dumpBufferTooSmall) + { + uint8_t buffer[1 * sizeof(struct variable_metadata) + 1]; +@@ -325,15 +272,10 @@ TEST(UefiVariableIndexTests, dumpBufferTooSmall) + * exceed the length of the buffer. + */ + size_t dump_len = 0; +- bool is_dirty = false; +- efi_status_t status = EFI_SUCCESS; +- +- status = variable_index_dump(&m_variable_index, sizeof(buffer), buffer, &dump_len, +- &is_dirty); ++ bool is_dirty = variable_index_dump(&m_variable_index, sizeof(buffer), buffer, &dump_len); + + CHECK_TRUE(is_dirty); +- UNSIGNED_LONGS_EQUAL(EFI_BUFFER_TOO_SMALL, status); +- UNSIGNED_LONGS_EQUAL(0, dump_len); ++ UNSIGNED_LONGS_EQUAL(sizeof(struct variable_metadata) * 1, dump_len); + } + + TEST(UefiVariableIndexTests, removeVariable) +@@ -351,14 +293,10 @@ TEST(UefiVariableIndexTests, removeVariable) + + /* Expect index to be dirty and for only one NV variable to be left */ + size_t dump_len = 0; +- bool is_dirty = false; +- efi_status_t status = EFI_SUCCESS; +- status = variable_index_dump(&m_variable_index, sizeof(buffer), buffer, &dump_len, +- &is_dirty); ++ bool is_dirty = variable_index_dump(&m_variable_index, sizeof(buffer), buffer, &dump_len); + + CHECK_TRUE(is_dirty); +- UNSIGNED_LONGS_EQUAL(EFI_SUCCESS, status); +- UNSIGNED_LONGS_EQUAL(sizeof(struct variable_metadata) + sizeof(bool), dump_len); ++ UNSIGNED_LONGS_EQUAL((sizeof(struct variable_metadata) * 1), dump_len); + + /* Remove the volatile variable */ + info = variable_index_find(&m_variable_index, &guid_1, string_get_size_in_bytes(name_1), +@@ -368,12 +306,10 @@ TEST(UefiVariableIndexTests, removeVariable) + + /* Expect index not to be dirty because there was no change to any NV variable */ + dump_len = 0; +- status = variable_index_dump(&m_variable_index, sizeof(buffer), buffer, &dump_len, +- &is_dirty); ++ is_dirty = variable_index_dump(&m_variable_index, sizeof(buffer), buffer, &dump_len); + + CHECK_FALSE(is_dirty); +- UNSIGNED_LONGS_EQUAL(EFI_SUCCESS, status); +- UNSIGNED_LONGS_EQUAL(sizeof(struct variable_metadata) + sizeof(bool), dump_len); ++ UNSIGNED_LONGS_EQUAL((sizeof(struct variable_metadata) * 1), dump_len); + + /* Remove the remaining NV variable */ + info = variable_index_find(&m_variable_index, &guid_1, string_get_size_in_bytes(name_3), +@@ -383,15 +319,14 @@ TEST(UefiVariableIndexTests, removeVariable) + + /* Expect index to be dirty and dump to now be empty */ + dump_len = 0; +- status = variable_index_dump(&m_variable_index, sizeof(buffer), buffer, &dump_len, +- &is_dirty); ++ is_dirty = variable_index_dump(&m_variable_index, sizeof(buffer), buffer, &dump_len); + + CHECK_TRUE(is_dirty); +- UNSIGNED_LONGS_EQUAL(EFI_SUCCESS, status); +- UNSIGNED_LONGS_EQUAL(0, dump_len); ++ UNSIGNED_LONGS_EQUAL((sizeof(struct variable_metadata) * 0), dump_len); + + /* Enumerate and now expect an empty index */ + info = NULL; ++ efi_status_t status = EFI_SUCCESS; + + info = variable_index_find_next(&m_variable_index, &guid_1, + string_get_size_in_bytes(null_name), (const int16_t *) null_name.data(), +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 e5fc32864..7da2d1e71 100644 +--- a/components/service/uefi/smm_variable/backend/uefi_variable_store.c ++++ b/components/service/uefi/smm_variable/backend/uefi_variable_store.c +@@ -575,10 +575,8 @@ efi_status_t uefi_variable_store_set_var_check_property( + status = variable_checker_set_constraints(&constraints, info->is_constraints_set, + &property->VariableProperty); + +- if (status == EFI_SUCCESS) { ++ if (status == EFI_SUCCESS) + variable_index_set_constraints(info, &constraints); +- status = sync_variable_index(context); +- } + + variable_index_remove_unused_entry(&context->variable_index, info); + +@@ -643,15 +641,13 @@ 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) + { + efi_status_t status = EFI_SUCCESS; +- bool is_dirty = false; + + /* Sync the variable index to storage if anything is dirty */ + size_t data_len = 0; + +- status = variable_index_dump(&context->variable_index, context->index_sync_buffer_size, +- context->index_sync_buffer, &data_len, &is_dirty); +- if (status != EFI_SUCCESS) +- return status; ++ bool is_dirty = variable_index_dump(&context->variable_index, ++ context->index_sync_buffer_size, ++ context->index_sync_buffer, &data_len); + + if (is_dirty) { + struct storage_backend *persistent_store = +diff --git a/components/service/uefi/smm_variable/backend/variable_index.c b/components/service/uefi/smm_variable/backend/variable_index.c +index 5fb6d08c5..c39f7394b 100644 +--- a/components/service/uefi/smm_variable/backend/variable_index.c ++++ b/components/service/uefi/smm_variable/backend/variable_index.c +@@ -108,9 +108,7 @@ void variable_index_deinit(struct variable_index *context) + + size_t variable_index_max_dump_size(struct variable_index *context) + { +- return (sizeof(struct variable_metadata) + sizeof(bool) + +- sizeof(struct variable_constraints)) * +- context->max_variables; ++ return sizeof(struct variable_metadata) * context->max_variables; + } + + struct variable_info *variable_index_find(const struct variable_index *context, +@@ -269,68 +267,37 @@ void variable_index_set_constraints(struct variable_info *info, + const struct variable_constraints *constraints) + { + if (info) { +- struct variable_entry *entry = containing_entry(info); +- + info->check_constraints = *constraints; + info->is_constraints_set = true; +- +- mark_dirty(entry); + } + } + +-efi_status_t variable_index_dump(const struct variable_index *context, size_t buffer_size, +- uint8_t *buffer, size_t *data_len, bool *any_dirty) ++bool variable_index_dump(const struct variable_index *context, size_t buffer_size, uint8_t *buffer, ++ size_t *data_len) + { ++ bool any_dirty = false; + uint8_t *dump_pos = buffer; + size_t bytes_dumped = 0; + +- *data_len = 0; +- *any_dirty = false; +- + for (size_t pos = 0; pos < context->max_variables; pos++) { + struct variable_entry *entry = &context->entries[pos]; + struct variable_metadata *metadata = &entry->info.metadata; +- struct variable_constraints *constraints = &entry->info.check_constraints; + + if (entry->in_use && entry->info.is_variable_set && +- (metadata->attributes & EFI_VARIABLE_NON_VOLATILE)) { +- /* Store metadata */ +- if (bytes_dumped + sizeof(struct variable_metadata) > buffer_size) +- return EFI_BUFFER_TOO_SMALL; +- ++ (metadata->attributes & EFI_VARIABLE_NON_VOLATILE) && ++ ((bytes_dumped + sizeof(struct variable_metadata)) <= buffer_size)) { + memcpy(dump_pos, metadata, sizeof(struct variable_metadata)); + bytes_dumped += sizeof(struct variable_metadata); + dump_pos += sizeof(struct variable_metadata); +- +- /* Store constraints' status */ +- if (bytes_dumped + sizeof(entry->info.is_constraints_set) > buffer_size) +- return EFI_BUFFER_TOO_SMALL; +- +- memcpy(dump_pos, &entry->info.is_constraints_set, +- sizeof(entry->info.is_constraints_set)); +- bytes_dumped += sizeof(entry->info.is_constraints_set); +- dump_pos += sizeof(entry->info.is_constraints_set); +- +- /* Store constraints, if they are set */ +- if (entry->info.is_constraints_set) { +- if (bytes_dumped + sizeof(entry->info.check_constraints) > +- buffer_size) +- return EFI_BUFFER_TOO_SMALL; +- +- memcpy(dump_pos, constraints, +- sizeof(entry->info.check_constraints)); +- bytes_dumped += sizeof(entry->info.check_constraints); +- dump_pos += sizeof(entry->info.check_constraints); +- } + } + +- *any_dirty |= entry->dirty; ++ any_dirty |= entry->dirty; + entry->dirty = false; + } + + *data_len = bytes_dumped; + +- return EFI_SUCCESS; ++ return any_dirty; + } + + +@@ -342,50 +309,23 @@ size_t variable_index_restore(const struct variable_index *context, size_t data_ + int pos = 0; + + while (bytes_loaded < data_len) { +- struct variable_entry *entry = &context->entries[pos]; +- + if ((data_len - bytes_loaded) >= sizeof(struct variable_metadata)) { ++ struct variable_entry *entry = &context->entries[pos]; + struct variable_metadata *metadata = &entry->info.metadata; + +- /* Load metadata */ + memcpy(metadata, load_pos, sizeof(struct variable_metadata)); ++ ++ entry->info.is_variable_set = true; ++ entry->in_use = true; ++ + bytes_loaded += sizeof(struct variable_metadata); + load_pos += sizeof(struct variable_metadata); +- } else { +- /* Not a whole number of variable_metadata structs! */ +- break; +- } + +- if ((data_len - bytes_loaded) >= sizeof(entry->info.is_constraints_set)) { +- /* Load constraints' status */ +- memcpy(&entry->info.is_constraints_set, load_pos, +- sizeof(entry->info.is_constraints_set)); +- bytes_loaded += sizeof(entry->info.is_constraints_set); +- load_pos += sizeof(entry->info.is_constraints_set); ++ ++pos; + } else { +- /* Not enough space for constraints' status! */ ++ /* Not a whole number of variable_metadata structs! */ + break; + } +- +- if (entry->info.is_constraints_set) { +- if ((data_len - bytes_loaded) >= sizeof(struct variable_constraints)) { +- struct variable_constraints *constraints = +- &entry->info.check_constraints; +- +- /* Load constraints if they are set */ +- memcpy(constraints, load_pos, sizeof(struct variable_constraints)); +- bytes_loaded += sizeof(struct variable_constraints); +- load_pos += sizeof(struct variable_constraints); +- } else { +- /* Not a whole number of variable_constraints structs! */ +- break; +- } +- } +- +- entry->info.is_variable_set = true; +- entry->in_use = true; +- +- ++pos; + } + + return bytes_loaded; +diff --git a/components/service/uefi/smm_variable/backend/variable_index.h b/components/service/uefi/smm_variable/backend/variable_index.h +index 0151d636a..da6ed2476 100644 +--- a/components/service/uefi/smm_variable/backend/variable_index.h ++++ b/components/service/uefi/smm_variable/backend/variable_index.h +@@ -201,12 +201,11 @@ void variable_index_set_constraints(struct variable_info *info, + * @param[in] buffer_size Size of destination buffer + * @param[in] buffer Dump to this buffer + * @param[out] data_len Length of serialized data +- * @param[out] any_dirty True if there is unsaved data + * +- * @return EFI_SUCCESS if all the changes are dumped successfully ++ * @return True if there is unsaved data + */ +-efi_status_t variable_index_dump(const struct variable_index *context, size_t buffer_size, +- uint8_t *buffer, size_t *data_len, bool *any_dirty); ++bool variable_index_dump(const struct variable_index *context, size_t buffer_size, uint8_t *buffer, ++ size_t *data_len); + + /** + * @brief Confirms the successful write of the variable index into the storage +-- +2.25.1 + diff --git a/meta-arm-bsp/recipes-security/trusted-services/ts-arm-platforms.inc b/meta-arm-bsp/recipes-security/trusted-services/ts-arm-platforms.inc index 89362ead..d9a73bc7 100644 --- a/meta-arm-bsp/recipes-security/trusted-services/ts-arm-platforms.inc +++ b/meta-arm-bsp/recipes-security/trusted-services/ts-arm-platforms.inc @@ -13,9 +13,11 @@ SRC_URI:append:corstone1000 = " \ file://0009-Remove-PLATFORM_HAS_ATTEST_PK-define-from-IAT-test.patch \ file://0010-Make-RSS-and-MHU-sizes-compile-time-definitions-user.patch \ file://0011-Align-PSA-Crypto-with-TF-Mv2.1.patch \ + file://0012-Revert-Make-variable-index-usage-robust-with-redunda.patch \ + file://0013-Revert-Load-and-store-UEFI-variable-index-in-chunks.patch \ + file://0014-Revert-Make-constraints-of-NV-UEFI-variables-persist.patch \ file://0015-se-proxy-protobuf-change.patch \ " - # The patches above introduce errors with GCC 14.1, silence them for now CFLAGS:append:corstone1000 = " -Wno-int-conversion -Wno-implicit-function-declaration"