From b2159a9e4f40fcbe755f8c0d2e6a7cbfcfd09f4f Mon Sep 17 00:00:00 2001 From: danieldg Date: Thu, 7 May 2009 14:51:10 +0000 Subject: Remove handshake timer on server sockets that die before completing handshake This fixes some very subtle and hard-to-trace bugs that are triggered when a file descriptor and memory address of an EventHandler* are reused after being deallocated. Impossible to trigger in valgrind; has been seen in live networks. git-svn-id: http://svn.inspircd.org/repository/trunk/inspircd@11369 e03df62e-2008-0410-955e-edbf42e46eb7 --- src/modules/m_spanningtree/handshaketimer.cpp | 34 ++++++++++++--------------- src/modules/m_spanningtree/handshaketimer.h | 1 + src/modules/m_spanningtree/treesocket.h | 2 ++ src/modules/m_spanningtree/treesocket1.cpp | 11 +++++++-- 4 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/modules/m_spanningtree/handshaketimer.cpp b/src/modules/m_spanningtree/handshaketimer.cpp index ae41bad0a..14b27472c 100644 --- a/src/modules/m_spanningtree/handshaketimer.cpp +++ b/src/modules/m_spanningtree/handshaketimer.cpp @@ -32,28 +32,24 @@ HandshakeTimer::HandshakeTimer(InspIRCd* Inst, TreeSocket* s, Link* l, SpanningT thefd = sock->GetFd(); } +HandshakeTimer::~HandshakeTimer() +{ + sock->hstimer = NULL; +} + void HandshakeTimer::Tick(time_t TIME) { - if (Instance->SE->GetRef(thefd) == sock) + if (!sock->GetHook()) + { + CancelRepeat(); + sock->SendCapabilities(); + } + else if (BufferedSocketHSCompleteRequest(sock, (Module*)Utils->Creator, sock->GetHook()).Send()) { - if (!sock->GetHook()) - { - CancelRepeat(); - sock->SendCapabilities(); - } - else - { - if (sock->GetHook() && BufferedSocketHSCompleteRequest(sock, (Module*)Utils->Creator, sock->GetHook()).Send()) - { - CancelRepeat(); - BufferedSocketAttachCertRequest(sock, (Module*)Utils->Creator, sock->GetHook()).Send(); - sock->SendCapabilities(); - } - else - { - // Try again later... - } - } + CancelRepeat(); + BufferedSocketAttachCertRequest(sock, (Module*)Utils->Creator, sock->GetHook()).Send(); + sock->SendCapabilities(); } + // otherwise, try again later } diff --git a/src/modules/m_spanningtree/handshaketimer.h b/src/modules/m_spanningtree/handshaketimer.h index 26379c8a6..0a60c78fc 100644 --- a/src/modules/m_spanningtree/handshaketimer.h +++ b/src/modules/m_spanningtree/handshaketimer.h @@ -31,6 +31,7 @@ class HandshakeTimer : public Timer int thefd; public: HandshakeTimer(InspIRCd* Inst, TreeSocket* s, Link* l, SpanningTreeUtilities* u, int delay); + ~HandshakeTimer(); virtual void Tick(time_t TIME); }; diff --git a/src/modules/m_spanningtree/treesocket.h b/src/modules/m_spanningtree/treesocket.h index 31a3d8cbc..d4def42bc 100644 --- a/src/modules/m_spanningtree/treesocket.h +++ b/src/modules/m_spanningtree/treesocket.h @@ -22,6 +22,7 @@ #include "transport.h" #include "m_spanningtree/utils.h" +#include "m_spanningtree/handshaketimer.h" /* * The server list in InspIRCd is maintained as two structures @@ -90,6 +91,7 @@ class TreeSocket : public BufferedSocket std::string OutboundPass; /* Outbound password */ bool sentcapab; /* Have sent CAPAB already */ public: + HandshakeTimer* hstimer; /* Handshake timer, needed to work around I/O hook buffering */ /** Because most of the I/O gubbins are encapsulated within * BufferedSocket, we just call the superclass constructor for diff --git a/src/modules/m_spanningtree/treesocket1.cpp b/src/modules/m_spanningtree/treesocket1.cpp index 12272d3b8..d0db01dec 100644 --- a/src/modules/m_spanningtree/treesocket1.cpp +++ b/src/modules/m_spanningtree/treesocket1.cpp @@ -46,6 +46,7 @@ TreeSocket::TreeSocket(SpanningTreeUtilities* Util, InspIRCd* SI, std::string sh Utils->timeoutlist[this] = std::pair(ServerName, maxtime); if (Hook) BufferedSocketHookRequest(this, (Module*)Utils->Creator, Hook).Send(); + hstimer = NULL; } /** When a listening socket gives us a new file descriptor, @@ -65,7 +66,8 @@ TreeSocket::TreeSocket(SpanningTreeUtilities* Util, InspIRCd* SI, int newfd, cha if (Hook) BufferedSocketHookRequest(this, (Module*)Utils->Creator, Hook).Send(); - ServerInstance->Timers->AddTimer(new HandshakeTimer(ServerInstance, this, &(Utils->LinkBlocks[0]), this->Utils, 1)); + hstimer = new HandshakeTimer(ServerInstance, this, &(Utils->LinkBlocks[0]), this->Utils, 1); + ServerInstance->Timers->AddTimer(hstimer); /* Fix by Brain - inbound sockets need a timeout, too. 30 secs should be pleanty */ Utils->timeoutlist[this] = std::pair("", 30); @@ -85,6 +87,8 @@ TreeSocket::~TreeSocket() { if (Hook) BufferedSocketUnhookRequest(this, (Module*)Utils->Creator, Hook).Send(); + if (hstimer) + ServerInstance->Timers->DelTimer(hstimer); Utils->timeoutlist.erase(this); } @@ -115,7 +119,10 @@ bool TreeSocket::OnConnected() /* found who we're supposed to be connecting to, send the neccessary gubbins. */ if (this->GetHook()) - ServerInstance->Timers->AddTimer(new HandshakeTimer(ServerInstance, this, &(*x), this->Utils, 1)); + { + hstimer = new HandshakeTimer(ServerInstance, this, &(*x), this->Utils, 1); + ServerInstance->Timers->AddTimer(hstimer); + } else this->SendCapabilities(); -- cgit v1.2.3