Skip to content

Commit 34ba6ec

Browse files
yunhanw-googlerestyled-commitsbzbarsky-apple
authored
[IM]Fix leaked readClient in onFabricRemoved call (project-chip#37199)
* Fix leaked readClient in onFabricRemoved When ReadClient::Close is called from onFabricRemove in InteractionModel Engine, readClient is destoryed and becomes not valid so that readClient->GetNextClient() will be use-after-free. * Restyled by clang-format * address comments * Update TestRead.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Clean up namespace in TestRead --Remove unnecessary chip::Test and chip::app and app namespace Restyled by clang-format * address comments --------- Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
1 parent 39fc790 commit 34ba6ec

File tree

3 files changed

+890
-923
lines changed

3 files changed

+890
-923
lines changed

src/app/InteractionModelEngine.cpp

+9-1
Original file line numberDiff line numberDiff line change
@@ -1967,12 +1967,20 @@ void InteractionModelEngine::OnFabricRemoved(const FabricTable & fabricTable, Fa
19671967
});
19681968

19691969
#if CHIP_CONFIG_ENABLE_READ_CLIENT
1970-
for (auto * readClient = mpActiveReadClientList; readClient != nullptr; readClient = readClient->GetNextClient())
1970+
for (auto * readClient = mpActiveReadClientList; readClient != nullptr;)
19711971
{
1972+
// ReadClient::Close may delete the read client so that readClient->GetNextClient() will be use-after-free.
1973+
// We need save readClient as nextReadClient before closing.
19721974
if (readClient->GetFabricIndex() == fabricIndex)
19731975
{
19741976
ChipLogProgress(InteractionModel, "Fabric removed, deleting obsolete read client with FabricIndex: %u", fabricIndex);
1977+
auto * nextReadClient = readClient->GetNextClient();
19751978
readClient->Close(CHIP_ERROR_IM_FABRIC_DELETED, false);
1979+
readClient = nextReadClient;
1980+
}
1981+
else
1982+
{
1983+
readClient = readClient->GetNextClient();
19761984
}
19771985
}
19781986
#endif // CHIP_CONFIG_ENABLE_READ_CLIENT

src/app/InteractionModelEngine.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
324324
/**
325325
* @brief Function decrements the number of subscriptions to resume counter - mNumOfSubscriptionsToResume.
326326
* This should be called after we have completed a re-subscribe attempt on a persisted subscription wether the attempt
327-
* was succesful or not.
327+
* was successful or not.
328328
*/
329329
void DecrementNumSubscriptionsToResume();
330330
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
@@ -714,7 +714,7 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
714714
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
715715
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
716716

717-
FabricTable * mpFabricTable;
717+
FabricTable * mpFabricTable = nullptr;
718718

719719
CASESessionManager * mpCASESessionMgr = nullptr;
720720

0 commit comments

Comments
 (0)