Skip to content

Commit

Permalink
[NXP][third_party] Fixed memory leak in BR mDNS code (#36543)
Browse files Browse the repository at this point in the history
* [NXP][third_party] Fixed memory leak in BR mDNS code

This commit fixes potential memory leaks that could happen
if multiple resolve of browse operations are done without
calling the appropriate stop function to clean up the
allocated resources. If using the Matter CLI this issue
is not possible as the CLI always stops the previous
operation but other processes calling the resolve/browse
API might not follow this flow.

Also fixed the return value of the StopBrowse function.

Signed-off-by: Marius Preda <[email protected]>

* Restyled by clang-format

---------

Signed-off-by: Marius Preda <[email protected]>
Co-authored-by: Restyled.io <[email protected]>
  • Loading branch information
2 people authored and pull[bot] committed Nov 22, 2024
1 parent 2b4c67e commit 1236116
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 6 deletions.
4 changes: 2 additions & 2 deletions src/platform/nxp/common/DnssdImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ CHIP_ERROR ChipDnssdBrowse(const char * type, DnssdServiceProtocol protocol, chi
chip::Inet::InterfaceId interface, DnssdBrowseCallback callback, void * context,
intptr_t * browseIdentifier)
{
if (ConnectivityMgr().IsWiFiStationConnected()) //|| ESP32Utils::HasIPv6LinkLocalAddress(ESP32Utils::kDefaultEthernetNetifKey))
if (ConnectivityMgr().IsWiFiStationConnected())
{
ReturnErrorOnFailure(NxpChipDnssdBrowse(type, protocol, addressType, interface, callback, context, browseIdentifier));
}
Expand All @@ -119,7 +119,7 @@ CHIP_ERROR ChipDnssdStopBrowse(intptr_t browseIdentifier)
CHIP_ERROR ChipDnssdResolve(DnssdService * service, chip::Inet::InterfaceId interface, DnssdResolveCallback callback,
void * context)
{
if (ConnectivityMgr().IsWiFiStationConnected()) //|| ESP32Utils::HasIPv6LinkLocalAddress(ESP32Utils::kDefaultEthernetNetifKey))
if (ConnectivityMgr().IsWiFiStationConnected())
{
ReturnErrorOnFailure(NxpChipDnssdResolve(service, interface, callback, context));
}
Expand Down
27 changes: 23 additions & 4 deletions src/platform/nxp/common/DnssdImplBr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
#include <openthread/mdns.h>
#include <openthread/srp_server.h>

#include <DnssdImplBr.h>

using namespace ::chip::DeviceLayer;
using namespace chip::DeviceLayer::Internal;

Expand Down Expand Up @@ -363,6 +365,11 @@ CHIP_ERROR NxpChipDnssdBrowse(const char * type, DnssdServiceProtocol protocol,
if (type == nullptr || callback == nullptr)
return CHIP_ERROR_INVALID_ARGUMENT;

if (mBrowseContext != nullptr)
{
NxpChipDnssdStopBrowse(reinterpret_cast<intptr_t>(mBrowseContext));
}

mBrowseContext = Platform::New<mDnsQueryCtx>(context, callback);
VerifyOrReturnError(mBrowseContext != nullptr, CHIP_ERROR_NO_MEMORY);

Expand Down Expand Up @@ -412,7 +419,8 @@ CHIP_ERROR NxpChipDnssdStopBrowse(intptr_t browseIdentifier)
// that has been freed in DispatchBrowseEmpty.
if ((true == bBrowseInProgress) && (browseContext))
{
browseContext->error = MapOpenThreadError(otMdnsStopBrowser(thrInstancePtr, &browseContext->mBrowseInfo));
error = otMdnsStopBrowser(thrInstancePtr, &browseContext->mBrowseInfo);
browseContext->error = MapOpenThreadError(error);

// browse context will be freed in DispatchBrowseEmpty
DispatchBrowseEmpty(reinterpret_cast<intptr_t>(browseContext));
Expand All @@ -430,6 +438,13 @@ CHIP_ERROR NxpChipDnssdResolve(DnssdService * browseResult, Inet::InterfaceId in

otInstance * thrInstancePtr = ThreadStackMgrImpl().OTInstance();

if (mResolveContext != nullptr)
{
// In case there is an ongoing query and NxpChipDnssdResolveNoLongerNeeded has not been called yet
// free the allocated context and do a proper cleanup of the previous transaction
NxpChipDnssdResolveNoLongerNeeded(mResolveContext->mMdnsService.mName);
}

mResolveContext = Platform::New<mDnsQueryCtx>(context, callback);
VerifyOrReturnError(mResolveContext != nullptr, CHIP_ERROR_NO_MEMORY);

Expand All @@ -450,12 +465,16 @@ CHIP_ERROR NxpChipDnssdResolve(DnssdService * browseResult, Inet::InterfaceId in
mResolveContext->mSrvInfo.mServiceInstance = mResolveContext->mMdnsService.mName;
mResolveContext->mSrvInfo.mServiceType = mResolveContext->mServiceType;

return MapOpenThreadError(otMdnsStartSrvResolver(thrInstancePtr, &mResolveContext->mSrvInfo));
error = MapOpenThreadError(otMdnsStartSrvResolver(thrInstancePtr, &mResolveContext->mSrvInfo));
}
else

if (error != CHIP_NO_ERROR)
{
return error;
Platform::Delete<mDnsQueryCtx>(mResolveContext);
mResolveContext = nullptr;
}

return error;
}
void NxpChipDnssdResolveNoLongerNeeded(const char * instanceName)
{
Expand Down

0 comments on commit 1236116

Please sign in to comment.