From 137643646afe8d024cbe43db11d7a89670322412 Mon Sep 17 00:00:00 2001
From: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com>
Date: Wed, 8 Nov 2023 11:24:53 -0500
Subject: [PATCH] [Silabs]Fix a memory leak in Efr32PsaOperationalKeystore
 (#30292)

* Free mPendingKeypair before setting it to null

* add clarifications. rename function
---
 src/platform/silabs/efr32/Efr32OpaqueKeypair.h       |  2 +-
 src/platform/silabs/efr32/Efr32PsaOpaqueKeypair.cpp  |  6 +++---
 .../silabs/efr32/Efr32PsaOperationalKeystore.cpp     | 10 ++++------
 .../silabs/efr32/Efr32PsaOperationalKeystore.h       | 12 ++++++++----
 4 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/src/platform/silabs/efr32/Efr32OpaqueKeypair.h b/src/platform/silabs/efr32/Efr32OpaqueKeypair.h
index 9b17d42b590fbe..97c4a5a2e1a572 100644
--- a/src/platform/silabs/efr32/Efr32OpaqueKeypair.h
+++ b/src/platform/silabs/efr32/Efr32OpaqueKeypair.h
@@ -132,7 +132,7 @@ class EFR32OpaqueKeypair
      *
      * @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
      **/
-    CHIP_ERROR Delete();
+    CHIP_ERROR DestroyKey();
 
 protected:
     void * mContext      = nullptr;
diff --git a/src/platform/silabs/efr32/Efr32PsaOpaqueKeypair.cpp b/src/platform/silabs/efr32/Efr32PsaOpaqueKeypair.cpp
index 5eb9f1f28a7ace..7a74c137596070 100644
--- a/src/platform/silabs/efr32/Efr32PsaOpaqueKeypair.cpp
+++ b/src/platform/silabs/efr32/Efr32PsaOpaqueKeypair.cpp
@@ -124,7 +124,7 @@ EFR32OpaqueKeypair::~EFR32OpaqueKeypair()
         // Delete volatile keys, since nobody else can after we drop the key ID.
         if (!mIsPersistent)
         {
-            Delete();
+            DestroyKey();
         }
 
         MemoryFree(mContext);
@@ -145,7 +145,7 @@ CHIP_ERROR EFR32OpaqueKeypair::Load(EFR32OpaqueKeyId opaque_id)
     // If the object contains a volatile key, clean it up before reusing the object storage
     if (mHasKey && !mIsPersistent)
     {
-        Delete();
+        DestroyKey();
     }
 
     key_id = psa_key_id_from_opaque(opaque_id);
@@ -342,7 +342,7 @@ CHIP_ERROR EFR32OpaqueKeypair::Derive(const uint8_t * their_key, size_t their_ke
     return error;
 }
 
-CHIP_ERROR EFR32OpaqueKeypair::Delete()
+CHIP_ERROR EFR32OpaqueKeypair::DestroyKey()
 {
     CHIP_ERROR error    = CHIP_NO_ERROR;
     psa_status_t status = PSA_ERROR_BAD_STATE;
diff --git a/src/platform/silabs/efr32/Efr32PsaOperationalKeystore.cpp b/src/platform/silabs/efr32/Efr32PsaOperationalKeystore.cpp
index 314c0ad4d3f54b..67b7c5e7d48fcc 100644
--- a/src/platform/silabs/efr32/Efr32PsaOperationalKeystore.cpp
+++ b/src/platform/silabs/efr32/Efr32PsaOperationalKeystore.cpp
@@ -209,7 +209,7 @@ CHIP_ERROR Efr32PsaOperationalKeystore::NewOpKeypairForFabric(FabricIndex fabric
         }
         else
         {
-            mPendingKeypair->Delete();
+            mPendingKeypair->DestroyKey();
             if (id == kEFR32OpaqueKeyIdVolatile)
             {
                 id = kEFR32OpaqueKeyIdUnknown;
@@ -313,9 +313,7 @@ CHIP_ERROR Efr32PsaOperationalKeystore::CommitOpKeypairForFabric(FabricIndex fab
     // There's a good chance we'll need the key again soon
     mCachedKey->Load(id);
 
-    mPendingKeypair         = nullptr;
-    mIsPendingKeypairActive = false;
-    mPendingFabricIndex     = kUndefinedFabricIndex;
+    ResetPendingKey(true /* keepKeyPairInStorage */);
 
     return CHIP_NO_ERROR;
 }
@@ -361,7 +359,7 @@ CHIP_ERROR Efr32PsaOperationalKeystore::RemoveOpKeypairForFabric(FabricIndex fab
     if (id == cachedId)
     {
         // Delete from persistent storage and unload
-        mCachedKey->Delete();
+        mCachedKey->DestroyKey();
         return CHIP_NO_ERROR;
     }
 
@@ -372,7 +370,7 @@ CHIP_ERROR Efr32PsaOperationalKeystore::RemoveOpKeypairForFabric(FabricIndex fab
         return CHIP_ERROR_INTERNAL;
     }
 
-    mCachedKey->Delete();
+    mCachedKey->DestroyKey();
 
     return CHIP_NO_ERROR;
 }
diff --git a/src/platform/silabs/efr32/Efr32PsaOperationalKeystore.h b/src/platform/silabs/efr32/Efr32PsaOperationalKeystore.h
index 4cb035e1459d2c..868535b97a63dd 100644
--- a/src/platform/silabs/efr32/Efr32PsaOperationalKeystore.h
+++ b/src/platform/silabs/efr32/Efr32PsaOperationalKeystore.h
@@ -94,13 +94,17 @@ class Efr32PsaOperationalKeystore : public chip::Crypto::OperationalKeystore
     bool mIsInitialized                      = false;
 
 private:
-    void ResetPendingKey()
+    void ResetPendingKey(bool keepKeyPairInStorage = false)
     {
-        if (mPendingKeypair != nullptr)
+        if (mPendingKeypair != nullptr && !keepKeyPairInStorage)
         {
-            mPendingKeypair->Delete();
-            Platform::Delete(mPendingKeypair);
+            // This removes the PSA Keypair from storage and unloads it
+            // using the EFR32OpaqueKeypair context.
+            // We destroy it when the OperationKeyStore process failed.
+            mPendingKeypair->DestroyKey();
         }
+
+        Platform::Delete(mPendingKeypair);
         mPendingKeypair         = nullptr;
         mIsPendingKeypairActive = false;
         mPendingFabricIndex     = kUndefinedFabricIndex;