[darwin-framework-tool] heap-use-after-free when calling chip::app::DnssdServer::StartServer at startup (#26000)

Author: vivien-apple

src/app/server/Dnssd.cpp

@@ -343,6 +343,30 @@ void DnssdServer::StartServer()
     return StartServer(mode);
 }
 
+void DnssdServer::StopServer()
+{
+    DeviceLayer::PlatformMgr().RemoveEventHandler(OnPlatformEventWrapper, 0);
+
+#if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
+    if (mExtendedDiscoveryExpiration != kTimeoutCleared)
+    {
+        DeviceLayer::SystemLayer().CancelTimer(HandleExtendedDiscoveryExpiration, nullptr);
+        mExtendedDiscoveryExpiration = kTimeoutCleared;
+    }
+#endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
+
+    if (Dnssd::ServiceAdvertiser::Instance().IsInitialized())
+    {
+        auto err = Dnssd::ServiceAdvertiser::Instance().RemoveServices();
+        if (err != CHIP_NO_ERROR)
+        {
+            ChipLogError(Discovery, "Failed to remove advertised services: %" CHIP_ERROR_FORMAT, err.Format());
+        }
+
+        Dnssd::ServiceAdvertiser::Instance().Shutdown();
+    }
+}
+
 void DnssdServer::StartServer(Dnssd::CommissioningMode mode)
 {
     ChipLogProgress(Discovery, "Updating services using commissioning mode %d", static_cast<int>(mode));

src/app/server/Dnssd.h

@@ -94,6 +94,9 @@ class DLL_EXPORT DnssdServer
     /// (Re-)starts the Dnssd server, using the provided commissioning mode.
     void StartServer(Dnssd::CommissioningMode mode);
 
+    //// Stop the Dnssd server.
+    void StopServer();
+
     CHIP_ERROR GenerateRotatingDeviceId(char rotatingDeviceIdHexBuffer[], size_t rotatingDeviceIdHexBufferSize);
 
     /// Generates the (random) instance name that a CHIP device is to use for pre-commissioning DNS-SD

src/controller/CHIPDeviceControllerFactory.cpp

@@ -340,6 +340,21 @@ CHIP_ERROR DeviceControllerFactory::ServiceEvents()
     return CHIP_NO_ERROR;
 }
 
+void DeviceControllerFactory::RetainSystemState()
+{
+    (void) mSystemState->Retain();
+}
+
+void DeviceControllerFactory::ReleaseSystemState()
+{
+    mSystemState->Release();
+
+    if (!mSystemState->IsInitialized() && mEnableServerInteractions)
+    {
+        app::DnssdServer::Instance().StopServer();
+    }
+}
+
 DeviceControllerFactory::~DeviceControllerFactory()
 {
     Shutdown();

src/controller/CHIPDeviceControllerFactory.h

@@ -174,7 +174,7 @@ class DeviceControllerFactory
     // created to permit retention of the underlying system state.
     //
     // NB: The system state will still be freed in Shutdown() regardless of this call.
-    void RetainSystemState() { (void) mSystemState->Retain(); }
+    void RetainSystemState();
 
     //
     // To initiate shutdown of the stack upon termination of all resident controllers in the
@@ -183,7 +183,7 @@ class DeviceControllerFactory
     //
     // This should only be invoked if a matching call to RetainSystemState() was called prior.
     //
-    void ReleaseSystemState() { mSystemState->Release(); }
+    void ReleaseSystemState();
 
     //
     // Retrieve a read-only pointer to the system state object that contains pointers to key stack

src/lib/dnssd/Advertiser.h

@@ -299,6 +299,13 @@ class ServiceAdvertiser
      */
     virtual CHIP_ERROR Init(chip::Inet::EndPointManager<chip::Inet::UDPEndPoint> * udpEndPointManager) = 0;
 
+    /**
+     * Returns whether the advertiser has completed the initialization.
+     *
+     * Returns true if the advertiser is ready to advertise services.
+     */
+    virtual bool IsInitialized() = 0;
+
     /**
      * Shuts down the advertiser.
      */

src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp

@@ -181,11 +181,12 @@ class AdvertiserMinMdns : public ServiceAdvertiser,
 
     // Service advertiser
     CHIP_ERROR Init(chip::Inet::EndPointManager<chip::Inet::UDPEndPoint> * udpEndPointManager) override;
+    bool IsInitialized() override { return mIsInitialized; }
     void Shutdown() override;
     CHIP_ERROR RemoveServices() override;
     CHIP_ERROR Advertise(const OperationalAdvertisingParameters & params) override;
     CHIP_ERROR Advertise(const CommissionAdvertisingParameters & params) override;
-    CHIP_ERROR FinalizeServiceUpdate() override { return CHIP_NO_ERROR; }
+    CHIP_ERROR FinalizeServiceUpdate() override;
     CHIP_ERROR GetCommissionableInstanceName(char * instanceName, size_t maxLength) const override;
     CHIP_ERROR UpdateCommissionableInstanceName() override;
 
@@ -363,6 +364,8 @@ CHIP_ERROR AdvertiserMinMdns::Init(chip::Inet::EndPointManager<chip::Inet::UDPEn
 
 void AdvertiserMinMdns::Shutdown()
 {
+    VerifyOrReturn(mIsInitialized);
+
     AdvertiseRecords(BroadcastAdvertiseType::kRemovingAll);
 
     GlobalMinimalMdnsServer::Server().Shutdown();
@@ -371,6 +374,8 @@ void AdvertiserMinMdns::Shutdown()
 
 CHIP_ERROR AdvertiserMinMdns::RemoveServices()
 {
+    VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE);
+
     // Send a "goodbye" packet for each RR being removed, as defined in RFC 6762.
     // This allows mDNS clients to remove stale cached records which may not be re-added with
     // subsequent Advertise() calls. In the case the same records are re-added, this extra
@@ -446,6 +451,8 @@ OperationalQueryAllocator::Allocator * AdvertiserMinMdns::FindEmptyOperationalAl
 
 CHIP_ERROR AdvertiserMinMdns::Advertise(const OperationalAdvertisingParameters & params)
 {
+    VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE);
+
     char nameBuffer[Operational::kInstanceNameMaxLength + 1] = "";
 
     // need to set server name
@@ -547,6 +554,12 @@ CHIP_ERROR AdvertiserMinMdns::Advertise(const OperationalAdvertisingParameters &
     return CHIP_NO_ERROR;
 }
 
+CHIP_ERROR AdvertiserMinMdns::FinalizeServiceUpdate()
+{
+    VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE);
+    return CHIP_NO_ERROR;
+}
+
 CHIP_ERROR AdvertiserMinMdns::GetCommissionableInstanceName(char * instanceName, size_t maxLength) const
 {
     if (maxLength < (Commission::kInstanceNameMaxLength + 1))
@@ -568,6 +581,8 @@ CHIP_ERROR AdvertiserMinMdns::UpdateCommissionableInstanceName()
 
 CHIP_ERROR AdvertiserMinMdns::Advertise(const CommissionAdvertisingParameters & params)
 {
+    VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE);
+
     if (params.GetCommissionAdvertiseMode() == CommssionAdvertiseMode::kCommissionableNode)
     {
         mQueryResponderAllocatorCommissionable.Clear();

src/lib/dnssd/Advertiser_ImplNone.cpp

@@ -32,6 +32,8 @@ class NoneAdvertiser : public ServiceAdvertiser
         return CHIP_ERROR_NOT_IMPLEMENTED;
     }
 
+    bool IsInitialized() override { return false; }
+
     void Shutdown() override {}
 
     CHIP_ERROR RemoveServices() override

src/lib/dnssd/Discovery_ImplPlatform.cpp

@@ -486,8 +486,6 @@ CHIP_ERROR DiscoveryImplPlatform::PublishService(const char * serviceType, TextE
                                                  Inet::InterfaceId interfaceId, const chip::ByteSpan & mac,
                                                  DnssdServiceProtocol protocol, PeerId peerId)
 {
-    VerifyOrReturnError(mState == State::kInitialized, CHIP_ERROR_INCORRECT_STATE);
-
     DnssdService service;
     ReturnErrorOnFailure(MakeHostName(service.mHostName, sizeof(service.mHostName), mac));
     ReturnErrorOnFailure(protocol == DnssdServiceProtocol::kDnssdProtocolTcp
@@ -525,6 +523,7 @@ CHIP_ERROR DiscoveryImplPlatform::PublishService(const char * serviceType, TextE
 }
 
 #define PREPARE_RECORDS(Type)                                                                                                      \
+    VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE);                                                              \
     TextEntry textEntries[Type##AdvertisingParameters::kTxtMaxNumber];                                                             \
     size_t textEntrySize = 0;                                                                                                      \
     const char * subTypes[Type::kSubTypeMaxNumber];                                                                                \
@@ -593,13 +592,15 @@ CHIP_ERROR DiscoveryImplPlatform::Advertise(const CommissionAdvertisingParameter
 
 CHIP_ERROR DiscoveryImplPlatform::RemoveServices()
 {
+    VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE);
     ReturnErrorOnFailure(ChipDnssdRemoveServices());
 
     return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR DiscoveryImplPlatform::FinalizeServiceUpdate()
 {
+    VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE);
     return ChipDnssdFinalizeServiceUpdate();
 }
 

src/lib/dnssd/Discovery_ImplPlatform.h

@@ -38,6 +38,7 @@ class DiscoveryImplPlatform : public ServiceAdvertiser, public Resolver
 public:
     // Members that implement both ServiceAdveriser and Resolver interfaces.
     CHIP_ERROR Init(Inet::EndPointManager<Inet::UDPEndPoint> *) override { return InitImpl(); }
+    bool IsInitialized() override;
     void Shutdown() override;
 
     // Members that implement ServiceAdvertiser interface.
@@ -49,7 +50,6 @@ class DiscoveryImplPlatform : public ServiceAdvertiser, public Resolver
     CHIP_ERROR UpdateCommissionableInstanceName() override;
 
     // Members that implement Resolver interface.
-    bool IsInitialized() override;
     void SetOperationalDelegate(OperationalResolveDelegate * delegate) override { mResolverProxy.SetOperationalDelegate(delegate); }
     void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override
     {