Browse Source

Drop release times for CNode

It seems there were two mechanisms for assessing whether a CNode
was still in use: a refcount and a release timestamp. The latter
seems to have been there for a long time, as a safety mechanism.

However, this timer also keeps CNode objects alive for far longer
than necessary after disconnects, potentially opening up a DoS
window.

This commit removes the timestamp-based mechanism, and replaces
it with an assert(nRefCount >= 0), to verify that the refcounting
is indeed correctly working.
miguelfreitas
Pieter Wuille 12 years ago committed by Pieter Wuille
parent
commit
cedaa71446
  1. 10
      src/net.cpp
  2. 15
      src/net.h
  3. 1
      src/rpcnet.cpp

10
src/net.cpp

@ -453,7 +453,7 @@ CNode* FindNode(const CService& addr)
return NULL; return NULL;
} }
CNode* ConnectNode(CAddress addrConnect, const char *pszDest, int64 nTimeout) CNode* ConnectNode(CAddress addrConnect, const char *pszDest)
{ {
if (pszDest == NULL) { if (pszDest == NULL) {
if (IsLocal(addrConnect)) if (IsLocal(addrConnect))
@ -463,9 +463,6 @@ CNode* ConnectNode(CAddress addrConnect, const char *pszDest, int64 nTimeout)
CNode* pnode = FindNode((CService)addrConnect); CNode* pnode = FindNode((CService)addrConnect);
if (pnode) if (pnode)
{ {
if (nTimeout != 0)
pnode->AddRef(nTimeout);
else
pnode->AddRef(); pnode->AddRef();
return pnode; return pnode;
} }
@ -498,9 +495,6 @@ CNode* ConnectNode(CAddress addrConnect, const char *pszDest, int64 nTimeout)
// Add node // Add node
CNode* pnode = new CNode(hSocket, addrConnect, pszDest ? pszDest : "", false); CNode* pnode = new CNode(hSocket, addrConnect, pszDest ? pszDest : "", false);
if (nTimeout != 0)
pnode->AddRef(nTimeout);
else
pnode->AddRef(); pnode->AddRef();
{ {
@ -615,7 +609,6 @@ void CNode::copyStats(CNodeStats &stats)
X(nVersion); X(nVersion);
X(strSubVer); X(strSubVer);
X(fInbound); X(fInbound);
X(nReleaseTime);
X(nStartingHeight); X(nStartingHeight);
X(nMisbehavior); X(nMisbehavior);
} }
@ -773,7 +766,6 @@ void ThreadSocketHandler()
pnode->Cleanup(); pnode->Cleanup();
// hold in disconnected pool until all refs are released // hold in disconnected pool until all refs are released
pnode->nReleaseTime = max(pnode->nReleaseTime, GetTime() + 15 * 60);
if (pnode->fNetworkNode || pnode->fInbound) if (pnode->fNetworkNode || pnode->fInbound)
pnode->Release(); pnode->Release();
vNodesDisconnected.push_back(pnode); vNodesDisconnected.push_back(pnode);

15
src/net.h

@ -37,7 +37,7 @@ bool GetMyExternalIP(CNetAddr& ipRet);
void AddressCurrentlyConnected(const CService& addr); void AddressCurrentlyConnected(const CService& addr);
CNode* FindNode(const CNetAddr& ip); CNode* FindNode(const CNetAddr& ip);
CNode* FindNode(const CService& ip); CNode* FindNode(const CService& ip);
CNode* ConnectNode(CAddress addrConnect, const char *strDest = NULL, int64 nTimeout=0); CNode* ConnectNode(CAddress addrConnect, const char *strDest = NULL);
void MapPort(bool fUseUPnP); void MapPort(bool fUseUPnP);
unsigned short GetListenPort(); unsigned short GetListenPort();
bool BindListenPort(const CService &bindAddr, std::string& strError=REF(std::string())); bool BindListenPort(const CService &bindAddr, std::string& strError=REF(std::string()));
@ -99,7 +99,6 @@ public:
int nVersion; int nVersion;
std::string strSubVer; std::string strSubVer;
bool fInbound; bool fInbound;
int64 nReleaseTime;
int nStartingHeight; int nStartingHeight;
int nMisbehavior; int nMisbehavior;
}; };
@ -187,8 +186,8 @@ public:
CSemaphoreGrant grantOutbound; CSemaphoreGrant grantOutbound;
CCriticalSection cs_filter; CCriticalSection cs_filter;
CBloomFilter* pfilter; CBloomFilter* pfilter;
protected:
int nRefCount; int nRefCount;
protected:
// Denial-of-service detection/prevention // Denial-of-service detection/prevention
// Key is IP address, value is banned-until-time // Key is IP address, value is banned-until-time
@ -197,7 +196,6 @@ protected:
int nMisbehavior; int nMisbehavior;
public: public:
int64 nReleaseTime;
uint256 hashContinue; uint256 hashContinue;
CBlockIndex* pindexLastGetBlocksBegin; CBlockIndex* pindexLastGetBlocksBegin;
uint256 hashLastGetBlocksEnd; uint256 hashLastGetBlocksEnd;
@ -235,7 +233,6 @@ public:
fSuccessfullyConnected = false; fSuccessfullyConnected = false;
fDisconnect = false; fDisconnect = false;
nRefCount = 0; nRefCount = 0;
nReleaseTime = 0;
nSendSize = 0; nSendSize = 0;
nSendOffset = 0; nSendOffset = 0;
hashContinue = 0; hashContinue = 0;
@ -272,7 +269,8 @@ public:
int GetRefCount() int GetRefCount()
{ {
return std::max(nRefCount, 0) + (GetTime() < nReleaseTime ? 1 : 0); assert(nRefCount >= 0);
return nRefCount;
} }
// requires LOCK(cs_vRecvMsg) // requires LOCK(cs_vRecvMsg)
@ -295,11 +293,8 @@ public:
msg.SetVersion(nVersionIn); msg.SetVersion(nVersionIn);
} }
CNode* AddRef(int64 nTimeout=0) CNode* AddRef()
{ {
if (nTimeout != 0)
nReleaseTime = std::max(nReleaseTime, GetTime() + nTimeout);
else
nRefCount++; nRefCount++;
return this; return this;
} }

1
src/rpcnet.cpp

@ -55,7 +55,6 @@ Value getpeerinfo(const Array& params, bool fHelp)
obj.push_back(Pair("version", stats.nVersion)); obj.push_back(Pair("version", stats.nVersion));
obj.push_back(Pair("subver", stats.strSubVer)); obj.push_back(Pair("subver", stats.strSubVer));
obj.push_back(Pair("inbound", stats.fInbound)); obj.push_back(Pair("inbound", stats.fInbound));
obj.push_back(Pair("releasetime", (boost::int64_t)stats.nReleaseTime));
obj.push_back(Pair("startingheight", stats.nStartingHeight)); obj.push_back(Pair("startingheight", stats.nStartingHeight));
obj.push_back(Pair("banscore", stats.nMisbehavior)); obj.push_back(Pair("banscore", stats.nMisbehavior));

Loading…
Cancel
Save