[mdns-mdnssd] fix access of freed mHostsRef in Stop() (#2013)

This commit updates the `Stop()` method to behave differently based on a
`StopMode` input. If called from `DNSServiceProcessResult()` call where
we get a `kDNSServiceErr_ServiceNotRunning` error, we need to restart
the `Publisher` and should immediately de-allocate all `ServiceRefs`,
including `mHostsRef`. Otherwise, during a `kNormalStop`, we first
clear the `m{Host/Service}Registrations` lists before de-allocating the
`mHostsRef`. This ensures that `mHostsRef` and all its related child
`DNSRecordRef`s remain valid when `DnssdHostRegistration` destructor is
called, which accesses and uses them to update the published AAAA (IPv6
address) records.

src/mdns/mdns_mdnssd.cpp

@@ -222,7 +222,7 @@ PublisherMDnsSd::PublisherMDnsSd(StateCallback aCallback)
 
 PublisherMDnsSd::~PublisherMDnsSd(void)
 {
-    Stop();
+    Stop(kNormalStop);
 }
 
 otbrError PublisherMDnsSd::Start(void)
@@ -237,26 +237,31 @@ bool PublisherMDnsSd::IsStarted(void) const
     return mState == State::kReady;
 }
 
-void PublisherMDnsSd::Stop(void)
+void PublisherMDnsSd::Stop(StopMode aStopMode)
 {
-    ServiceRegistrationMap serviceRegistrations;
-    HostRegistrationMap    hostRegistrations;
-
     VerifyOrExit(mState == State::kReady);
 
-    std::swap(mServiceRegistrations, serviceRegistrations);
-    std::swap(mHostRegistrations, hostRegistrations);
+    // If we get a `kDNSServiceErr_ServiceNotRunning` and need to
+    // restart the `Publisher`, we should immediately de-allocate
+    // all `ServiceRef`. Otherwise, we first clear the `Registrations`
+    // list so that `DnssdHostRegisteration` destructor gets the chance
+    // to update registered records if needed.
 
-    if (mHostsRef != nullptr)
+    switch (aStopMode)
     {
-        HandleServiceRefDeallocating(mHostsRef);
-        DNSServiceRefDeallocate(mHostsRef);
-        otbrLogDebug("Deallocated DNSServiceRef for hosts: %p", mHostsRef);
-        mHostsRef = nullptr;
+    case kNormalStop:
+        break;
+
+    case kStopOnServiceNotRunningError:
+        DeallocateHostsRef();
+        break;
     }
 
-    mSubscribedServices.clear();
+    mServiceRegistrations.clear();
+    mHostRegistrations.clear();
+    DeallocateHostsRef();
 
+    mSubscribedServices.clear();
     mSubscribedHosts.clear();
 
     mState = State::kIdle;
@@ -265,6 +270,19 @@ void PublisherMDnsSd::Stop(void)
     return;
 }
 
+void PublisherMDnsSd::DeallocateHostsRef(void)
+{
+    VerifyOrExit(mHostsRef != nullptr);
+
+    HandleServiceRefDeallocating(mHostsRef);
+    DNSServiceRefDeallocate(mHostsRef);
+    otbrLogDebug("Deallocated DNSServiceRef for hosts: %p", mHostsRef);
+    mHostsRef = nullptr;
+
+exit:
+    return;
+}
+
 void PublisherMDnsSd::Update(MainloopContext &aMainloop)
 {
     for (auto &kv : mServiceRegistrations)
@@ -370,7 +388,7 @@ void PublisherMDnsSd::Process(const MainloopContext &aMainloop)
         if (error == kDNSServiceErr_ServiceNotRunning)
         {
             otbrLogWarning("Need to reconnect to mdnsd");
-            Stop();
+            Stop(kStopOnServiceNotRunningError);
             Start();
             ExitNow();
         }
@@ -403,6 +421,7 @@ PublisherMDnsSd::DnssdHostRegistration::~DnssdHostRegistration(void)
 {
     int dnsError;
 
+    VerifyOrExit(GetPublisher().mHostsRef != nullptr);
     VerifyOrExit(mServiceRef != nullptr);
 
     for (const auto &recordRefAndAddress : GetRecordRefMap())

src/mdns/mdns_mdnssd.hpp

@@ -76,7 +76,7 @@ class PublisherMDnsSd : public MainloopProcessor, public Publisher
     void      UnsubscribeHost(const std::string &aHostName) override;
     otbrError Start(void) override;
     bool      IsStarted(void) const override;
-    void      Stop(void) override;
+    void      Stop(void) override { Stop(kNormalStop); }
 
     // Implementation of MainloopProcessor.
 
@@ -103,6 +103,12 @@ class PublisherMDnsSd : public MainloopProcessor, public Publisher
 private:
     static constexpr uint32_t kDefaultTtl = 10;
 
+    enum StopMode : uint8_t
+    {
+        kNormalStop,
+        kStopOnServiceNotRunningError,
+    };
+
     class DnssdServiceRegistration : public ServiceRegistration
     {
     public:
@@ -156,6 +162,8 @@ class PublisherMDnsSd : public MainloopProcessor, public Publisher
         std::map<DNSRecordRef, Ip6Address>       &GetRecordRefMap() { return mRecordRefMap; }
 
     private:
+        PublisherMDnsSd &GetPublisher(void) { return *static_cast<PublisherMDnsSd *>(mPublisher); }
+
         DNSServiceRef mServiceRef;
 
     public:
@@ -344,6 +352,8 @@ class PublisherMDnsSd : public MainloopProcessor, public Publisher
 
     static std::string MakeRegType(const std::string &aType, SubTypeList aSubTypeList);
 
+    void Stop(StopMode aStopMode);
+    void DeallocateHostsRef(void);
     void HandleServiceRefDeallocating(const DNSServiceRef &aServiceRef);
 
     ServiceRegistration *FindServiceRegistration(const DNSServiceRef &aServiceRef);