From c023f265b716d2eeb8209f9719548feae2bbb131 Mon Sep 17 00:00:00 2001 From: Kevin Schoedel Date: Thu, 18 Nov 2021 11:24:20 -0500 Subject: [PATCH] Fix CommissioningWindowManager DNS UDP leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### Problem `chip::Server::Shutdown()` contains ``` chip::Dnssd::ServiceAdvertiser::Instance().Shutdown(); ⋮ mCommissioningWindowManager.Cleanup(); ``` but the latter restarts `Dnssd::ServiceAdvertiser`. Instance of #11880 _Possible use of destroyed pool objects_ #### Change overview Add `CommissioningWindowManager::Shutdown()` which does not restart DNS. #### Testing If `ObjectPool` checks that objects do not outlive it (originally part of PR #11698 but deferred due to current leaks), then `TestCommissionManager` fails without this change. --- src/app/server/CommissioningWindowManager.cpp | 7 ++++++- src/app/server/CommissioningWindowManager.h | 1 + src/app/server/Server.cpp | 2 +- src/app/tests/TestCommissionManager.cpp | 3 ++- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/app/server/CommissioningWindowManager.cpp b/src/app/server/CommissioningWindowManager.cpp index a5cf7222170f6a..e19cfcd7cfd4b1 100644 --- a/src/app/server/CommissioningWindowManager.cpp +++ b/src/app/server/CommissioningWindowManager.cpp @@ -71,7 +71,7 @@ void CommissioningWindowManager::OnPlatformEvent(const DeviceLayer::ChipDeviceEv } } -void CommissioningWindowManager::Cleanup() +void CommissioningWindowManager::Shutdown() { StopAdvertisement(); @@ -84,6 +84,11 @@ void CommissioningWindowManager::Cleanup() memset(&mECMPASEVerifier, 0, sizeof(mECMPASEVerifier)); memset(mECMSalt, 0, sizeof(mECMSalt)); +} + +void CommissioningWindowManager::Cleanup() +{ + Shutdown(); // reset all advertising app::DnssdServer::Instance().StartServer(Dnssd::CommissioningMode::kDisabled); diff --git a/src/app/server/CommissioningWindowManager.h b/src/app/server/CommissioningWindowManager.h index 58e518469861aa..50e299cb3394a8 100644 --- a/src/app/server/CommissioningWindowManager.h +++ b/src/app/server/CommissioningWindowManager.h @@ -65,6 +65,7 @@ class CommissioningWindowManager : public SessionEstablishmentDelegate void OnSessionEstablishmentStarted() override; void OnSessionEstablished() override; + void Shutdown(); void Cleanup(); void OnPlatformEvent(const DeviceLayer::ChipDeviceEvent * event); diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index fcab2e31444250..5bfa9e18114b88 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -184,7 +184,7 @@ void Server::Shutdown() mExchangeMgr.Shutdown(); mSessions.Shutdown(); mTransports.Close(); - mCommissioningWindowManager.Cleanup(); + mCommissioningWindowManager.Shutdown(); chip::Platform::MemoryShutdown(); } diff --git a/src/app/tests/TestCommissionManager.cpp b/src/app/tests/TestCommissionManager.cpp index 14f3ec35fb6bde..32fa2774b5b0bc 100644 --- a/src/app/tests/TestCommissionManager.cpp +++ b/src/app/tests/TestCommissionManager.cpp @@ -138,7 +138,6 @@ void CheckCommissioningWindowManagerEnhancedWindow(nlTestSuite * suite, void *) void TearDownTask(intptr_t context) { chip::Server::GetInstance().Shutdown(); - chip::DeviceLayer::PlatformMgr().Shutdown(); } const nlTest sTests[] = { @@ -167,6 +166,8 @@ int TestCommissioningWindowManager() // TODO: The platform memory was intentionally left not deinitialized so that minimal mdns can destruct chip::DeviceLayer::PlatformMgr().ScheduleWork(TearDownTask, 0); sleep(kTestTaskWaitSeconds); + chip::DeviceLayer::PlatformMgr().StopEventLoopTask(); + chip::DeviceLayer::PlatformMgr().Shutdown(); return (nlTestRunnerStats(&theSuite)); }