espressif/openthread
1
0
Fork 0
mirror of https://github.com/espressif/openthread.git synced 2026-08-25 19:59:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

[srp-server] replace SRP Host pointer with use SRP service update ID (#6407)

Browse Source 
We are using the SRP Host pointer for indexing an outstanding SRP
update when the Advertising Proxy result is passed back with
otSrpServerHandleServiceUpdateResult. There are chances that an old,
freed Host object may be matched again with a newly allocated Host
object which coincidentally has the same address with the old Host
object.

This commit solves this issue by using a random generated uint32_t SRP
service update ID for indexing outstanding updates.
This commit is contained in:
kangping
2021-04-09 22:09:30 -07:00
committed by GitHub
parent 4acc997bf5
commit 8987ae4c21
5 changed files with 52 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
include/openthread/instance.h
+1 -1
View File
@@ -53,7 +53,7 @@ extern "C" {
* @note This number versions both OpenThread platform and user APIs.
*
*/
#define OPENTHREAD_API_VERSION (97)
#define OPENTHREAD_API_VERSION (98)
/**
* @addtogroup api-instance
include/openthread/srp_server.h
+18 -8
View File
@@ -67,6 +67,12 @@ typedef void otSrpServerHost;
*/
typedef void otSrpServerService;
/**
* The ID of a SRP service update transaction on the SRP Server.
*
*/
typedef uint32_t otSrpServerServiceUpdateId;
/**
* This method returns the domain authorized to the SRP server.
*
@@ -144,11 +150,11 @@ otError otSrpServerSetLeaseRange(otInstance *aInstance,
* if any validation fails. For example, an Advertising Proxy should advertise (or remove) the host and
* services on a multicast-capable link and returns specific error code if any failure occurs.
*
* @param[in] aHost A pointer to the otSrpServerHost object which contains the SRP updates.
* The pointer should be passed back to otSrpServerHandleServiceUpdateResult, but
* the content MUST not be accessed after this method returns. The handler
* should publish/un-publish the host and each service points to this host
* with below rules:
* @param[in] aId The service update transaction ID. This ID must be passed back with
* `otSrpServerHandleServiceUpdateResult`.
* @param[in] aHost A pointer to the otSrpServerHost object which contains the SRP updates. The
* handler should publish/un-publish the host and each service points to this
* host with below rules:
* 1. If the host is not deleted (indicated by `otSrpServerHostIsDeleted`),
* then it should be published or updated with mDNS. Otherwise, the host
* should be un-published (remove AAAA RRs).
@@ -163,7 +169,10 @@ otError otSrpServerSetLeaseRange(otInstance *aInstance,
* @sa otSrpServerHandleServiceUpdateResult
*
*/
typedef void (*otSrpServerServiceUpdateHandler)(const otSrpServerHost *aHost, uint32_t aTimeout, void *aContext);
typedef void (*otSrpServerServiceUpdateHandler)(otSrpServerServiceUpdateId aId,
const otSrpServerHost * aHost,
uint32_t aTimeout,
void * aContext);
/**
* This method sets the SRP service updates handler on SRP server.
@@ -185,12 +194,13 @@ void otSrpServerSetServiceUpdateHandler(otInstance * aInstanc
* processing of a SRP update.
*
* @param[in] aInstance A pointer to an OpenThread instance.
* @param[in] aHost A pointer to the Host object which represents a SRP update.
* @param[in] aId The service update transaction ID. This should be the same ID
* provided via `otSrpServerServiceUpdateHandler`.
* @param[in] aError An error to be returned to the SRP server. Use OT_ERROR_DUPLICATED
* to represent DNS name conflicts.
*
*/
void otSrpServerHandleServiceUpdateResult(otInstance *aInstance, const otSrpServerHost *aHost, otError aError);
void otSrpServerHandleServiceUpdateResult(otInstance *aInstance, otSrpServerServiceUpdateId aId, otError aError);
/**
* This method returns the next registered host on the SRP server.
src/core/api/srp_server_api.cpp
+2 -2
View File
@@ -83,11 +83,11 @@ void otSrpServerSetServiceUpdateHandler(otInstance * aInstanc
instance.Get<Srp::Server>().SetServiceHandler(aServiceHandler, aContext);
}
void otSrpServerHandleServiceUpdateResult(otInstance *aInstance, const otSrpServerHost *aHost, otError aError)
void otSrpServerHandleServiceUpdateResult(otInstance *aInstance, otSrpServerServiceUpdateId aId, otError aError)
{
Instance &instance = *static_cast<Instance *>(aInstance);
instance.Get<Srp::Server>().HandleServiceUpdateResult(static_cast<const Srp::Server::Host *>(aHost), aError);
instance.Get<Srp::Server>().HandleServiceUpdateResult(aId, aError);
}
const otSrpServerHost *otSrpServerGetNextHost(otInstance *aInstance, const otSrpServerHost *aHost)
src/core/net/srp_server.cpp
+10 -6
View File
@@ -39,6 +39,7 @@
#include "common/locator-getters.hpp"
#include "common/logging.hpp"
#include "common/new.hpp"
#include "common/random.hpp"
#include "net/dns_types.hpp"
#include "thread/network_data_service.hpp"
#include "thread/thread_netif.hpp"
@@ -86,6 +87,7 @@ Server::Server(Instance &aInstance)
, mMaxKeyLease(kDefaultMaxKeyLease)
, mLeaseTimer(aInstance, HandleLeaseTimer)
, mOutstandingUpdatesTimer(aInstance, HandleOutstandingUpdatesTimer)
, mServiceUpdateId(Random::NonCrypto::GetUint32())
, mEnabled(false)
{
IgnoreError(SetDomain(kDefaultDomain));
@@ -234,7 +236,7 @@ void Server::RemoveHost(Host *aHost, bool aRetainName, bool aNotifyServiceHandle
if (aNotifyServiceHandler && mServiceUpdateHandler != nullptr)
{
mServiceUpdateHandler(aHost, kDefaultEventsHandlerTimeout, mServiceUpdateHandlerContext);
mServiceUpdateHandler(AllocateId(), aHost, kDefaultEventsHandlerTimeout, mServiceUpdateHandlerContext);
// We don't wait for the reply from the service update handler,
// but always remove the host (and its services) regardless of
// host/service update result. Because removing a host should fail
@@ -292,9 +294,9 @@ exit:
return hasConflicts;
}
void Server::HandleServiceUpdateResult(const Host *aHost, Error aError)
void Server::HandleServiceUpdateResult(ServiceUpdateId aId, Error aError)
{
UpdateMetadata *update = mOutstandingUpdates.FindMatching(aHost);
UpdateMetadata *update = mOutstandingUpdates.FindMatching(aId);
if (update != nullptr)
{
@@ -1022,7 +1024,7 @@ exit:
IgnoreError(mOutstandingUpdates.Add(*update));
mOutstandingUpdatesTimer.StartAt(mOutstandingUpdates.GetTail()->GetExpireTime(), 0);
mServiceUpdateHandler(aHost, kDefaultEventsHandlerTimeout, mServiceUpdateHandlerContext);
mServiceUpdateHandler(update->GetId(), aHost, kDefaultEventsHandlerTimeout, mServiceUpdateHandlerContext);
}
else
{
@@ -1543,7 +1545,7 @@ exit:
void Server::Host::RemoveService(Service *aService, bool aRetainName, bool aNotifyServiceHandler)
{
const Server &server = Get<Server>();
Server &server = Get<Server>();
VerifyOrExit(aService != nullptr);
@@ -1568,7 +1570,8 @@ void Server::Host::RemoveService(Service *aService, bool aRetainName, bool aNoti
mServices.Clear();
IgnoreError(mServices.Add(*aService));
server.mServiceUpdateHandler(this, kDefaultEventsHandlerTimeout, server.mServiceUpdateHandlerContext);
server.mServiceUpdateHandler(server.AllocateId(), this, kDefaultEventsHandlerTimeout,
server.mServiceUpdateHandlerContext);
// We don't wait for the reply from the service update handler,
// but always remove the service regardless of service update result.
// Because removing a service should fail only when there is system
@@ -1691,6 +1694,7 @@ Server::UpdateMetadata::UpdateMetadata(Instance & aInstance,
: InstanceLocator(aInstance)
, mExpireTime(TimerMilli::GetNow() + kDefaultEventsHandlerTimeout)
, mDnsHeader(aHeader)
, mId(Get<Server>().AllocateId())
, mHost(aHost)
, mMessageInfo(aMessageInfo)
, mNext(nullptr)
src/core/net/srp_server.hpp
+21 -9
View File
@@ -71,6 +71,9 @@ namespace Srp {
class Server : public InstanceLocator, private NonCopyable
{
friend class ot::Notifier;
friend class UpdateMetadata;
friend class Service;
friend class Host;
public:
enum : uint16_t
@@ -78,6 +81,12 @@ public:
kUdpPort = OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT, ///< The SRP Server UDP listening port.
};
/**
* The ID of SRP service update transaction.
*
*/
typedef otSrpServerServiceUpdateId ServiceUpdateId;
class Host;
class Service;
@@ -243,6 +252,7 @@ public:
{
friend class LinkedListEntry<Host>;
friend class Server;
friend class UpdateMetadata;
public:
/**
@@ -490,11 +500,11 @@ public:
* This method receives the service update result from service handler set by
* SetServiceHandler.
*
* @param[in] aHost A pointer to the Host object which contains the SRP service updates.
* @param[in] aId The ID of the service update transaction.
* @param[in] aError The service update result.
*
*/
void HandleServiceUpdateResult(const Host *aHost, Error aError);
void HandleServiceUpdateResult(ServiceUpdateId aId, Error aError);
private:
enum : uint16_t
@@ -511,11 +521,8 @@ private:
kDefaultEventsHandlerTimeout = OPENTHREAD_CONFIG_SRP_SERVER_SERVICE_UPDATE_TIMEOUT,
};
/**
* This class includes metadata for processing a SRP update (register, deregister)
* and sending DNS response to the client.
*
*/
// This class includes metadata for processing a SRP update (register, deregister)
// and sending DNS response to the client.
class UpdateMetadata : public InstanceLocator, public LinkedListEntry<UpdateMetadata>
{
friend class LinkedListEntry<UpdateMetadata>;
@@ -528,9 +535,10 @@ private:
void Free(void);
TimeMilli GetExpireTime(void) const { return mExpireTime; }
const Dns::UpdateHeader &GetDnsHeader(void) const { return mDnsHeader; }
ServiceUpdateId GetId(void) const { return mId; }
Host & GetHost(void) { return *mHost; }
const Ip6::MessageInfo & GetMessageInfo(void) const { return mMessageInfo; }
bool Matches(const Host *aHost) const { return mHost == aHost; }
bool Matches(ServiceUpdateId aId) const { return mId == aId; }
private:
UpdateMetadata(Instance & aInstance,
@@ -540,6 +548,7 @@ private:
TimeMilli mExpireTime;
Dns::UpdateHeader mDnsHeader;
ServiceUpdateId mId; // The ID of this service update transaction.
Host * mHost; // The host will be updated. The UpdateMetadata has no ownership of this host.
Ip6::MessageInfo mMessageInfo; // The message info of the DNS update request.
UpdateMetadata * mNext;
@@ -553,6 +562,8 @@ private:
uint32_t GrantLease(uint32_t aLease) const;
uint32_t GrantKeyLease(uint32_t aKeyLease) const;
ServiceUpdateId AllocateId(void) { return mServiceUpdateId++; }
void CommitSrpUpdate(Error aError,
const Dns::UpdateHeader &aDnsHeader,
Host & aHost,
@@ -638,7 +649,8 @@ private:
TimerMilli mOutstandingUpdatesTimer;
LinkedList<UpdateMetadata> mOutstandingUpdates;
bool mEnabled;
ServiceUpdateId mServiceUpdateId;
bool mEnabled;
};
} // namespace Srp