Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[darwin-framework-tool] heap-use-after-free when calling chip::app::D… #26000

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/app/server/Dnssd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
3 changes: 3 additions & 0 deletions src/app/server/Dnssd.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions src/controller/CHIPDeviceControllerFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions src/controller/CHIPDeviceControllerFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
7 changes: 7 additions & 0 deletions src/lib/dnssd/Advertiser.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
17 changes: 16 additions & 1 deletion src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand All @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions src/lib/dnssd/Advertiser_ImplNone.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions src/lib/dnssd/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]; \
Expand Down Expand Up @@ -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();
}

Expand Down
2 changes: 1 addition & 1 deletion src/lib/dnssd/Discovery_ImplPlatform.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
{
Expand Down