Skip to content

Commit 9d0a7e3

Browse files
[Silabs]Fix a memory leak in Efr32PsaOperationalKeystore (project-chip#30292)
* Free mPendingKeypair before setting it to null * add clarifications. rename function
1 parent 6d548ef commit 9d0a7e3

4 files changed

+16
-14
lines changed

src/platform/silabs/efr32/Efr32OpaqueKeypair.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ class EFR32OpaqueKeypair
132132
*
133133
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
134134
**/
135-
CHIP_ERROR Delete();
135+
CHIP_ERROR DestroyKey();
136136

137137
protected:
138138
void * mContext = nullptr;

src/platform/silabs/efr32/Efr32PsaOpaqueKeypair.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ EFR32OpaqueKeypair::~EFR32OpaqueKeypair()
124124
// Delete volatile keys, since nobody else can after we drop the key ID.
125125
if (!mIsPersistent)
126126
{
127-
Delete();
127+
DestroyKey();
128128
}
129129

130130
MemoryFree(mContext);
@@ -145,7 +145,7 @@ CHIP_ERROR EFR32OpaqueKeypair::Load(EFR32OpaqueKeyId opaque_id)
145145
// If the object contains a volatile key, clean it up before reusing the object storage
146146
if (mHasKey && !mIsPersistent)
147147
{
148-
Delete();
148+
DestroyKey();
149149
}
150150

151151
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
342342
return error;
343343
}
344344

345-
CHIP_ERROR EFR32OpaqueKeypair::Delete()
345+
CHIP_ERROR EFR32OpaqueKeypair::DestroyKey()
346346
{
347347
CHIP_ERROR error = CHIP_NO_ERROR;
348348
psa_status_t status = PSA_ERROR_BAD_STATE;

src/platform/silabs/efr32/Efr32PsaOperationalKeystore.cpp

+4-6
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ CHIP_ERROR Efr32PsaOperationalKeystore::NewOpKeypairForFabric(FabricIndex fabric
209209
}
210210
else
211211
{
212-
mPendingKeypair->Delete();
212+
mPendingKeypair->DestroyKey();
213213
if (id == kEFR32OpaqueKeyIdVolatile)
214214
{
215215
id = kEFR32OpaqueKeyIdUnknown;
@@ -313,9 +313,7 @@ CHIP_ERROR Efr32PsaOperationalKeystore::CommitOpKeypairForFabric(FabricIndex fab
313313
// There's a good chance we'll need the key again soon
314314
mCachedKey->Load(id);
315315

316-
mPendingKeypair = nullptr;
317-
mIsPendingKeypairActive = false;
318-
mPendingFabricIndex = kUndefinedFabricIndex;
316+
ResetPendingKey(true /* keepKeyPairInStorage */);
319317

320318
return CHIP_NO_ERROR;
321319
}
@@ -361,7 +359,7 @@ CHIP_ERROR Efr32PsaOperationalKeystore::RemoveOpKeypairForFabric(FabricIndex fab
361359
if (id == cachedId)
362360
{
363361
// Delete from persistent storage and unload
364-
mCachedKey->Delete();
362+
mCachedKey->DestroyKey();
365363
return CHIP_NO_ERROR;
366364
}
367365

@@ -372,7 +370,7 @@ CHIP_ERROR Efr32PsaOperationalKeystore::RemoveOpKeypairForFabric(FabricIndex fab
372370
return CHIP_ERROR_INTERNAL;
373371
}
374372

375-
mCachedKey->Delete();
373+
mCachedKey->DestroyKey();
376374

377375
return CHIP_NO_ERROR;
378376
}

src/platform/silabs/efr32/Efr32PsaOperationalKeystore.h

+8-4
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,17 @@ class Efr32PsaOperationalKeystore : public chip::Crypto::OperationalKeystore
9494
bool mIsInitialized = false;
9595

9696
private:
97-
void ResetPendingKey()
97+
void ResetPendingKey(bool keepKeyPairInStorage = false)
9898
{
99-
if (mPendingKeypair != nullptr)
99+
if (mPendingKeypair != nullptr && !keepKeyPairInStorage)
100100
{
101-
mPendingKeypair->Delete();
102-
Platform::Delete(mPendingKeypair);
101+
// This removes the PSA Keypair from storage and unloads it
102+
// using the EFR32OpaqueKeypair context.
103+
// We destroy it when the OperationKeyStore process failed.
104+
mPendingKeypair->DestroyKey();
103105
}
106+
107+
Platform::Delete(mPendingKeypair);
104108
mPendingKeypair = nullptr;
105109
mIsPendingKeypairActive = false;
106110
mPendingFabricIndex = kUndefinedFabricIndex;

0 commit comments

Comments
 (0)