From 5029698186445bf3cd69d0e720f019c472661bff Mon Sep 17 00:00:00 2001 From: kazcw Date: Wed, 16 Jul 2014 14:31:41 -0700 Subject: prevent peer flooding request queue for an inv mapAlreadyAskedFor does not keep track of which peer has a request queued for a particular tx. As a result, a peer can blind a node to a tx indefinitely by sending many invs for the same tx, and then never replying to getdatas for it. Each inv received will be placed 2 minutes farther back in mapAlreadyAskedFor, so a short message containing 10 invs would render that tx unavailable for 20 minutes. This is fixed by disallowing a peer from having more than one entry for a particular inv in mapAlreadyAskedFor at a time. --- src/main.cpp | 1 + src/net.cpp | 4 ++++ src/net.h | 1 + 3 files changed, 6 insertions(+) (limited to 'src') diff --git a/src/main.cpp b/src/main.cpp index 2579b642b8..05dedb5631 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -5226,6 +5226,7 @@ bool SendMessages(CNode* pto, bool fSendTrickle) vGetData.clear(); } } + pto->setAskFor.erase(inv.hash); pto->mapAskFor.erase(pto->mapAskFor.begin()); } if (!vGetData.empty()) diff --git a/src/net.cpp b/src/net.cpp index cff4c54505..04119e9ddd 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2410,6 +2410,10 @@ void CNode::AskFor(const CInv& inv) { if (mapAskFor.size() > MAPASKFOR_MAX_SZ) return; + // a peer may not occupy multiple positions in an inv's request queue + if (!setAskFor.insert(inv.hash).second) + return; + // We're using mapAskFor as a priority queue, // the key is the earliest time the request can be sent int64_t nRequestTime; diff --git a/src/net.h b/src/net.h index 559cdf0878..046811d54c 100644 --- a/src/net.h +++ b/src/net.h @@ -382,6 +382,7 @@ public: mruset setInventoryKnown; std::vector vInventoryToSend; CCriticalSection cs_inventory; + std::set setAskFor; std::multimap mapAskFor; // Ping time measurement: -- cgit v1.2.3 From ebb25f4c23adbcb55796c402bafd6064a136f16f Mon Sep 17 00:00:00 2001 From: Gregory Maxwell Date: Mon, 23 Nov 2015 01:54:23 +0000 Subject: Limit setAskFor and retire requested entries only when a getdata returns. The setAskFor duplicate elimination was too eager and removed entries when we still had no getdata response, allowing the peer to keep INVing and not responding. --- src/main.cpp | 5 ++++- src/net.cpp | 4 ++-- src/net.h | 2 ++ 3 files changed, 8 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/main.cpp b/src/main.cpp index 05dedb5631..2bcc4cbc54 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4406,6 +4406,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, bool fMissingInputs = false; CValidationState state; + pfrom->setAskFor.erase(inv.hash); mapAlreadyAskedFor.erase(inv); // Check for recently rejected (and do other quick existence checks) @@ -5225,8 +5226,10 @@ bool SendMessages(CNode* pto, bool fSendTrickle) pto->PushMessage("getdata", vGetData); vGetData.clear(); } + } else { + //If we're not going to ask, don't expect a response. + pto->setAskFor.erase(inv.hash); } - pto->setAskFor.erase(inv.hash); pto->mapAskFor.erase(pto->mapAskFor.begin()); } if (!vGetData.empty()) diff --git a/src/net.cpp b/src/net.cpp index 04119e9ddd..a8b6ca9c5b 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2408,9 +2408,9 @@ CNode::~CNode() void CNode::AskFor(const CInv& inv) { - if (mapAskFor.size() > MAPASKFOR_MAX_SZ) + if (mapAskFor.size() > MAPASKFOR_MAX_SZ || setAskFor.size() > SETASKFOR_MAX_SZ) return; - // a peer may not occupy multiple positions in an inv's request queue + // a peer may not have multiple non-responded queue positions for a single inv item if (!setAskFor.insert(inv.hash).second) return; diff --git a/src/net.h b/src/net.h index 046811d54c..bf75899a40 100644 --- a/src/net.h +++ b/src/net.h @@ -58,6 +58,8 @@ static const bool DEFAULT_UPNP = false; #endif /** The maximum number of entries in mapAskFor */ static const size_t MAPASKFOR_MAX_SZ = MAX_INV_SZ; +/** The maximum number of entries in setAskFor (larger due to getdata latency)*/ +static const size_t SETASKFOR_MAX_SZ = 2 * MAX_INV_SZ; /** The maximum number of peer connections to maintain. */ static const unsigned int DEFAULT_MAX_PEER_CONNECTIONS = 125; /** The default for -maxuploadtarget. 0 = Unlimited */ -- cgit v1.2.3