Skip to content

Commit 1099136

Browse files
bzbarsky-applepull[bot]
authored andcommitted
Make sure DnssdServer does not dereference a dangling fabric table. (#26354)
We could end up not calling StopServer if the last release of a system state was a direct release on the state itself, not a call to ReleaseSystemState. To fix this, move the DnssdServer::StopServer bits into the actual system state shutdown, where we know whether we are destroying the fabric table that DnssdServer uses and can clean up appropriately.
1 parent ba1c695 commit 1099136

File tree

4 files changed

+24
-10
lines changed

4 files changed

+24
-10
lines changed

src/app/server/Dnssd.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,9 @@ void DnssdServer::StartServer()
345345

346346
void DnssdServer::StopServer()
347347
{
348+
// Make sure we don't hold on to a dangling fabric table pointer.
349+
mFabricTable = nullptr;
350+
348351
DeviceLayer::PlatformMgr().RemoveEventHandler(OnPlatformEventWrapper, 0);
349352

350353
#if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY

src/app/server/Dnssd.h

+5-3
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,9 @@ class DLL_EXPORT DnssdServer
6060
Inet::InterfaceId GetInterfaceId() { return mInterfaceId; }
6161

6262
//
63-
// Override the referenced fabric table from the default that is present
64-
// in Server::GetInstance().GetFabricTable() to something else.
63+
// Set the fabric table the DnssdServer should use for operational
64+
// advertising. This must be set before StartServer() is called for the
65+
// first time.
6566
//
6667
void SetFabricTable(FabricTable * table)
6768
{
@@ -94,7 +95,8 @@ class DLL_EXPORT DnssdServer
9495
/// (Re-)starts the Dnssd server, using the provided commissioning mode.
9596
void StartServer(Dnssd::CommissioningMode mode);
9697

97-
//// Stop the Dnssd server.
98+
//// Stop the Dnssd server. After this call, SetFabricTable must be called
99+
//// again before calling StartServer().
98100
void StopServer();
99101

100102
CHIP_ERROR GenerateRotatingDeviceId(char rotatingDeviceIdHexBuffer[], size_t rotatingDeviceIdHexBufferSize);

src/controller/CHIPDeviceControllerFactory.cpp

+9-6
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
272272

273273
// store the system state
274274
mSystemState = chip::Platform::New<DeviceControllerSystemState>(std::move(stateParams));
275-
mSystemState->SetTempFabricTable(tempFabricTable);
275+
mSystemState->SetTempFabricTable(tempFabricTable, params.enableServerInteractions);
276276
ChipLogDetail(Controller, "System State Initialized...");
277277
return CHIP_NO_ERROR;
278278
}
@@ -348,11 +348,6 @@ void DeviceControllerFactory::RetainSystemState()
348348
void DeviceControllerFactory::ReleaseSystemState()
349349
{
350350
mSystemState->Release();
351-
352-
if (!mSystemState->IsInitialized() && mEnableServerInteractions)
353-
{
354-
app::DnssdServer::Instance().StopServer();
355-
}
356351
}
357352

358353
DeviceControllerFactory::~DeviceControllerFactory()
@@ -384,6 +379,14 @@ void DeviceControllerSystemState::Shutdown()
384379

385380
ChipLogDetail(Controller, "Shutting down the System State, this will teardown the CHIP Stack");
386381

382+
if (mTempFabricTable && mEnableServerInteractions)
383+
{
384+
// The DnssdServer is holding a reference to our temp fabric table,
385+
// which we are about to destroy. Stop it, so that it will stop trying
386+
// to use it.
387+
app::DnssdServer::Instance().StopServer();
388+
}
389+
387390
if (mFabricTableDelegate != nullptr)
388391
{
389392
if (mFabrics != nullptr)

src/controller/CHIPDeviceControllerSystemState.h

+7-1
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,11 @@ class DeviceControllerSystemState
185185
CASESessionManager * CASESessionMgr() const { return mCASESessionManager; }
186186
Credentials::GroupDataProvider * GetGroupDataProvider() const { return mGroupDataProvider; }
187187
Crypto::SessionKeystore * GetSessionKeystore() const { return mSessionKeystore; }
188-
void SetTempFabricTable(FabricTable * tempFabricTable) { mTempFabricTable = tempFabricTable; }
188+
void SetTempFabricTable(FabricTable * tempFabricTable, bool enableServerInteractions)
189+
{
190+
mTempFabricTable = tempFabricTable;
191+
mEnableServerInteractions = enableServerInteractions;
192+
}
189193

190194
private:
191195
DeviceControllerSystemState() {}
@@ -220,6 +224,8 @@ class DeviceControllerSystemState
220224

221225
bool mHaveShutDown = false;
222226

227+
bool mEnableServerInteractions = false;
228+
223229
void Shutdown();
224230
};
225231

0 commit comments

Comments
 (0)