Use ReferenceCount<ResolverDelegateProxy> to make sure the stack is n…

…ot trying to access a dangling pointer once a response is received

src/controller/AbstractDnssdDiscoveryController.h

@@ -50,7 +50,7 @@ class DLL_EXPORT AbstractDnssdDiscoveryController : public Dnssd::ResolverDelega
     const Dnssd::DiscoveredNodeData * GetDiscoveredNode(int idx);
     virtual DiscoveredNodeList GetDiscoveredNodes()    = 0;
     DeviceDiscoveryDelegate * mDeviceDiscoveryDelegate = nullptr;
-    Dnssd::ResolverProxy mDNSResolver{ this };
+    Dnssd::ResolverProxy mDNSResolver;
 };
 
 } // namespace Controller

src/controller/CHIPCommissionableNodeController.cpp

@@ -34,6 +34,10 @@ CHIP_ERROR CommissionableNodeController::DiscoverCommissioners(Dnssd::DiscoveryF
 
     if (mResolver == nullptr)
     {
+#if CONFIG_DEVICE_LAYER
+        ReturnErrorOnFailure(mDNSResolver.Init(&DeviceLayer::InetLayer()));
+#endif
+        mDNSResolver.SetResolverDelegate(this);
         return mDNSResolver.FindCommissioners(discoveryFilter);
     }
     else

src/controller/CHIPDeviceController.cpp

@@ -131,7 +131,8 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params)
     VerifyOrReturnError(params.systemState->TransportMgr() != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
 
 #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
-    mDNSResolver.Init();
+    ReturnErrorOnFailure(mDNSResolver.Init(params.systemState->InetLayer()));
+    mDNSResolver.SetResolverDelegate(this);
     RegisterDeviceAddressUpdateDelegate(params.deviceAddressUpdateDelegate);
     RegisterDeviceDiscoveryDelegate(params.deviceDiscoveryDelegate);
 #endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD

src/controller/tests/TestCommissionableNodeController.cpp

@@ -180,9 +180,30 @@ const nlTest sTests[] =
 
 } // namespace
 
+int TestCommissionableNodeController_Setup(void * inContext)
+{
+    if (CHIP_NO_ERROR != chip::Platform::MemoryInit())
+    {
+        return FAILURE;
+    }
+
+    return SUCCESS;
+}
+
+int TestCommissionableNodeController_Teardown(void * inContext)
+{
+    chip::Platform::MemoryShutdown();
+    return SUCCESS;
+}
+
 int TestCommissionableNodeController()
 {
-    nlTestSuite theSuite = { "CommissionableNodeController", &sTests[0], NULL, NULL };
+    nlTestSuite theSuite = {
+        "CommissionableNodeController",
+        &sTests[0],
+        TestCommissionableNodeController_Setup,
+        TestCommissionableNodeController_Teardown
+    };
     nlTestRunner(&theSuite, nullptr);
     return nlTestRunnerStats(&theSuite);
 }

src/lib/dnssd/Discovery_ImplPlatform.cpp

@@ -44,7 +44,13 @@ static DnssdCache<CHIP_CONFIG_MDNS_CACHE_SIZE> sDnssdCache;
 
 static void HandleNodeResolve(void * context, DnssdService * result, CHIP_ERROR error)
 {
-    VerifyOrReturn(CHIP_NO_ERROR == error);
+    ResolverDelegateProxy * proxy = static_cast<ResolverDelegateProxy *>(context);
+
+    if (CHIP_NO_ERROR != error)
+    {
+        proxy->Release();
+        return;
+    }
 
     DiscoveredNodeData nodeData;
     Platform::CopyString(nodeData.hostName, result->mHostName);
@@ -66,13 +72,12 @@ static void HandleNodeResolve(void * context, DnssdService * result, CHIP_ERROR
         FillNodeDataFromTxt(key, val, nodeData);
     }
 
-    ResolverProxy * proxy = static_cast<ResolverProxy *>(context);
     proxy->OnNodeDiscoveryComplete(nodeData);
 }
 
 static void HandleNodeIdResolve(void * context, DnssdService * result, CHIP_ERROR error)
 {
-    ResolverProxy * proxy = static_cast<ResolverProxy *>(context);
+    ResolverDelegateProxy * proxy = static_cast<ResolverDelegateProxy *>(context);
     VerifyOrReturn(CHIP_NO_ERROR == error, proxy->OnNodeIdResolutionFailed(PeerId(), error));
     VerifyOrReturn(result != nullptr, proxy->OnNodeIdResolutionFailed(PeerId(), CHIP_ERROR_UNKNOWN_RESOURCE_ID));
 
@@ -108,8 +113,12 @@ static void HandleNodeIdResolve(void * context, DnssdService * result, CHIP_ERRO
 
 static void HandleNodeBrowse(void * context, DnssdService * services, size_t servicesSize, CHIP_ERROR error)
 {
+    ResolverDelegateProxy * proxy = static_cast<ResolverDelegateProxy *>(context);
+    proxy->Release();
+
     for (size_t i = 0; i < servicesSize; ++i)
     {
+        proxy->Retain();
         // For some platforms browsed services are already resolved, so verify if resolve is really needed or call resolve callback
         if (!services[i].mAddress.HasValue())
         {
@@ -576,7 +585,8 @@ Resolver & chip::Dnssd::Resolver::Instance()
 
 CHIP_ERROR ResolverProxy::ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type, Resolver::CacheBypass dnssdCacheBypass)
 {
-    ReturnErrorOnFailure(chip::Dnssd::Resolver::Instance().Init(nullptr));
+    VerifyOrReturnError(mDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE);
+    mDelegate->Retain();
 
 #if CHIP_CONFIG_MDNS_CACHE_SIZE > 0
     if (dnssdCacheBypass == Resolver::CacheBypass::Off)
@@ -585,7 +595,7 @@ CHIP_ERROR ResolverProxy::ResolveNodeId(const PeerId & peerId, Inet::IPAddressTy
         ResolvedNodeData nodeData;
         if (sDnssdCache.Lookup(peerId, nodeData) == CHIP_NO_ERROR)
         {
-            mResolverDelegate->OnNodeIdResolved(nodeData);
+            mDelegate->OnNodeIdResolved(nodeData);
             return CHIP_NO_ERROR;
         }
     }
@@ -597,29 +607,31 @@ CHIP_ERROR ResolverProxy::ResolveNodeId(const PeerId & peerId, Inet::IPAddressTy
     strncpy(service.mType, kOperationalServiceName, sizeof(service.mType));
     service.mProtocol    = DnssdServiceProtocol::kDnssdProtocolTcp;
     service.mAddressType = type;
-    return ChipDnssdResolve(&service, Inet::InterfaceId::Null(), HandleNodeIdResolve, this);
+    return ChipDnssdResolve(&service, Inet::InterfaceId::Null(), HandleNodeIdResolve, mDelegate);
 }
 
 CHIP_ERROR ResolverProxy::FindCommissionableNodes(DiscoveryFilter filter)
 {
-    ReturnErrorOnFailure(chip::Dnssd::Resolver::Instance().Init(nullptr));
+    VerifyOrReturnError(mDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE);
+    mDelegate->Retain();
 
     char serviceName[kMaxCommissionableServiceNameSize];
     ReturnErrorOnFailure(MakeServiceTypeName(serviceName, sizeof(serviceName), filter, DiscoveryType::kCommissionableNode));
 
     return ChipDnssdBrowse(serviceName, DnssdServiceProtocol::kDnssdProtocolUdp, Inet::IPAddressType::kAny,
-                           Inet::InterfaceId::Null(), HandleNodeBrowse, this);
+                           Inet::InterfaceId::Null(), HandleNodeBrowse, mDelegate);
 }
 
 CHIP_ERROR ResolverProxy::FindCommissioners(DiscoveryFilter filter)
 {
-    ReturnErrorOnFailure(chip::Dnssd::Resolver::Instance().Init(nullptr));
+    VerifyOrReturnError(mDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE);
+    mDelegate->Retain();
 
     char serviceName[kMaxCommissionerServiceNameSize];
     ReturnErrorOnFailure(MakeServiceTypeName(serviceName, sizeof(serviceName), filter, DiscoveryType::kCommissionerNode));
 
     return ChipDnssdBrowse(serviceName, DnssdServiceProtocol::kDnssdProtocolUdp, Inet::IPAddressType::kAny,
-                           Inet::InterfaceId::Null(), HandleNodeBrowse, this);
+                           Inet::InterfaceId::Null(), HandleNodeBrowse, mDelegate);
 }
 
 } // namespace Dnssd

src/lib/dnssd/ResolverProxy.h

@@ -17,52 +17,86 @@
 
 #pragma once
 
+#include <lib/core/ReferenceCounted.h>
 #include <lib/dnssd/Resolver.h>
-#include <lib/dnssd/platform/Dnssd.h>
 
 namespace chip {
 namespace Dnssd {
 
-class ResolverProxy : public Resolver, public ResolverDelegate
+namespace {
+class ResolverDelegateProxy : public ReferenceCounted<ResolverDelegateProxy>, public ResolverDelegate
 {
 public:
-    ResolverProxy() {}
-    ResolverProxy(ResolverDelegate * delegate) : mResolverDelegate(delegate) {}
-
-    // Resolver interface.
-    CHIP_ERROR Init(Inet::InetLayer * = nullptr) override { return CHIP_NO_ERROR; }
-    void Shutdown() override { mResolverDelegate = nullptr; };
-    void SetResolverDelegate(ResolverDelegate * delegate) override { mResolverDelegate = delegate; }
-    CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type,
-                             Resolver::CacheBypass dnssdCacheBypass = CacheBypass::Off) override;
-    CHIP_ERROR FindCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override;
-    CHIP_ERROR FindCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override;
+    void SetDelegate(ResolverDelegate * delegate) { mDelegate = delegate; }
 
     /// ResolverDelegate interface
     void OnNodeIdResolved(const ResolvedNodeData & nodeData) override
     {
-        if (mResolverDelegate != nullptr)
+        if (mDelegate != nullptr)
         {
-            mResolverDelegate->OnNodeIdResolved(nodeData);
+            mDelegate->OnNodeIdResolved(nodeData);
         }
+        Release();
     }
+
     void OnNodeIdResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override
     {
-        if (mResolverDelegate != nullptr)
+        if (mDelegate != nullptr)
         {
-            mResolverDelegate->OnNodeIdResolutionFailed(peerId, error);
+            mDelegate->OnNodeIdResolutionFailed(peerId, error);
         }
+        Release();
     }
+
     void OnNodeDiscoveryComplete(const DiscoveredNodeData & nodeData) override
     {
-        if (mResolverDelegate != nullptr)
+        if (mDelegate != nullptr)
         {
-            mResolverDelegate->OnNodeDiscoveryComplete(nodeData);
+            mDelegate->OnNodeDiscoveryComplete(nodeData);
         }
+        Release();
     }
 
 private:
-    ResolverDelegate * mResolverDelegate = nullptr;
+    ResolverDelegate * mDelegate = nullptr;
+};
+} // namespace
+
+class ResolverProxy : public Resolver
+{
+public:
+    ResolverProxy() {}
+
+    // Resolver interface.
+    CHIP_ERROR Init(Inet::InetLayer * inetLayer = nullptr) override
+    {
+        ReturnErrorOnFailure(chip::Dnssd::Resolver::Instance().Init(inetLayer));
+        VerifyOrReturnError(mDelegate == nullptr, CHIP_ERROR_INCORRECT_STATE);
+        mDelegate = chip::Platform::New<ResolverDelegateProxy>();
+        return CHIP_NO_ERROR;
+    }
+
+    void SetResolverDelegate(ResolverDelegate * delegate) override
+    {
+        VerifyOrReturn(mDelegate != nullptr);
+        mDelegate->SetDelegate(delegate);
+    }
+
+    void Shutdown() override
+    {
+        VerifyOrReturn(mDelegate != nullptr);
+        mDelegate->SetDelegate(nullptr);
+        mDelegate->Release();
+        mDelegate = nullptr;
+    }
+
+    CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type,
+                             Resolver::CacheBypass dnssdCacheBypass = CacheBypass::Off) override;
+    CHIP_ERROR FindCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override;
+    CHIP_ERROR FindCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override;
+
+private:
+    ResolverDelegateProxy * mDelegate = nullptr;
 };
 
 } // namespace Dnssd

src/lib/dnssd/Resolver_ImplMinimalMdns.cpp

@@ -601,19 +601,22 @@ Resolver & chip::Dnssd::Resolver::Instance()
 
 CHIP_ERROR ResolverProxy::ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type, Resolver::CacheBypass dnssdCacheBypass)
 {
-    chip::Dnssd::Resolver::Instance().SetResolverDelegate(this);
+    VerifyOrReturnError(mDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE);
+    chip::Dnssd::Resolver::Instance().SetResolverDelegate(mDelegate);
     return chip::Dnssd::Resolver::Instance().ResolveNodeId(peerId, type, dnssdCacheBypass);
 }
 
 CHIP_ERROR ResolverProxy::FindCommissionableNodes(DiscoveryFilter filter)
 {
-    chip::Dnssd::Resolver::Instance().SetResolverDelegate(this);
+    VerifyOrReturnError(mDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE);
+    chip::Dnssd::Resolver::Instance().SetResolverDelegate(mDelegate);
     return chip::Dnssd::Resolver::Instance().FindCommissionableNodes(filter);
 }
 
 CHIP_ERROR ResolverProxy::FindCommissioners(DiscoveryFilter filter)
 {
-    chip::Dnssd::Resolver::Instance().SetResolverDelegate(this);
+    VerifyOrReturnError(mDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE);
+    chip::Dnssd::Resolver::Instance().SetResolverDelegate(mDelegate);
     return chip::Dnssd::Resolver::Instance().FindCommissioners(filter);
 }