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

diff --git a/src/modules/m_spanningtree/handshaketimer.cpp b/src/modules/m_spanningtree/handshaketimer.cpp
index ae41bad0ade4f23c6b9df2e88393c77d383cdc87..14b27472c1724f59222955d7e65f03ff6e156a4a 100644 (file)
--- 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 26379c8a60d8a06b2be7aea31830c2cbd8534558..0a60c78fc6c3551cc5c05176a2d983c8a21bc049 100644 (file)
--- 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 31a3d8cbc4be708a377397649b4ed3e173944701..d4def42bcc3d862299351e695579710815f19593 100644 (file)
--- 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 12272d3b8e449ac5b4ad3087483591f5806b9c9b..d0db01dec87292e3e2c1e2a7f6f8bcd30ac39218 100644 (file)
--- 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<std::string, int>(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<std::string, int>("<unknown>", 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();