1From c62e728bb86981219984c8b39819fb8926a41e10 Mon Sep 17 00:00:00 2001 2From: Gabor Toth <gabor.toth2@arm.com> 3Date: Fri, 19 Apr 2024 18:25:23 +0200 4Subject: [PATCH 3/3] Fix error handling of variable index loading 5 6If loading of the variable index from Protected Storage fails, SmmGW 7will silently continue with empty variable store. This is a serious 8fault and a potential security risk. 9Change the code to produce a log output when this happens and stop 10loading the SP. 11 12Signed-off-by: Gabor Toth <gabor.toth2@arm.com> 13Upstream-Status: Submitted [https://review.trustedfirmware.org/c/TS/trusted-services/+/28300] 14--- 15 .../backend/uefi_variable_store.c | 28 ++++++++++++++----- 16 1 file changed, 21 insertions(+), 7 deletions(-) 17 18diff --git a/components/service/uefi/smm_variable/backend/uefi_variable_store.c b/components/service/uefi/smm_variable/backend/uefi_variable_store.c 19index caf6698aa..c1691dc8f 100644 20--- a/components/service/uefi/smm_variable/backend/uefi_variable_store.c 21+++ b/components/service/uefi/smm_variable/backend/uefi_variable_store.c 22@@ -27,7 +27,7 @@ 23 #include "service/crypto/client/psa/crypto_client.h" 24 #endif 25 26-static void load_variable_index(struct uefi_variable_store *context); 27+static efi_status_t load_variable_index(struct uefi_variable_store *context); 28 29 static efi_status_t sync_variable_index(const struct uefi_variable_store *context); 30 31@@ -165,8 +165,10 @@ efi_status_t uefi_variable_store_init(struct uefi_variable_store *context, uint3 32 33 /* Load the variable index with NV variable info from the persistent store */ 34 if (context->index_sync_buffer) { 35- load_variable_index(context); 36- purge_orphan_index_entries(context); 37+ status = load_variable_index(context); 38+ 39+ if (status == EFI_SUCCESS) 40+ purge_orphan_index_entries(context); 41 } 42 } 43 44@@ -571,7 +573,7 @@ efi_status_t uefi_variable_store_get_var_check_property( 45 return status; 46 } 47 48-static void load_variable_index(struct uefi_variable_store *context) 49+static efi_status_t load_variable_index(struct uefi_variable_store *context) 50 { 51 struct storage_backend *persistent_store = context->persistent_store.storage_backend; 52 53@@ -583,11 +585,23 @@ static void load_variable_index(struct uefi_variable_store *context) 54 SMM_VARIABLE_INDEX_STORAGE_UID, 0, context->index_sync_buffer_size, 55 context->index_sync_buffer, &data_len); 56 57- if (psa_status == PSA_SUCCESS) { 58- variable_index_restore(&context->variable_index, data_len, 59- context->index_sync_buffer); 60+ switch(psa_status) { 61+ case PSA_SUCCESS: 62+ (void) variable_index_restore(&context->variable_index, data_len, 63+ context->index_sync_buffer); 64+ break; 65+ 66+ case PSA_ERROR_DOES_NOT_EXIST: 67+ IMSG("Index variable does not exist in NV store, continuing with empty index"); 68+ break; 69+ 70+ default: 71+ EMSG("Loading variable index failed: %d", psa_status); 72+ return EFI_LOAD_ERROR; 73 } 74 } 75+ 76+ return EFI_SUCCESS; 77 } 78 79 static efi_status_t sync_variable_index(const struct uefi_variable_store *context) 80-- 812.25.1 82 83