dnsdist: Fix a crash when TCP queries and responses keep coming

It happens when we keep finding queries waiting for us on the incoming
TCP socket from the client, and responses waiting for us on the TCP
socket to the backend after forwarding a new query. This is quite
unlikely but not impossible to happen, as reported by Renaud Allard
(many thanks for taking the time to investigate the issue!).

diff --git a/pdns/dnsdistdist/dnsdist-tcp-upstream.hh b/pdns/dnsdistdist/dnsdist-tcp-upstream.hh
index 45d989a7364697bf902e15a06e9a434dbd2adb0d..d592262fa2292cebc035251b344c526916d0ac12 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-tcp-upstream.hh
+++ b/pdns/dnsdistdist/dnsdist-tcp-upstream.hh
@@ -160,4 +160,5 @@ public:
   bool d_proxyProtocolPayloadHasTLV{false};
   bool d_lastIOBlocked{false};
   bool d_hadErrors{false};
+  bool d_handlingIO{false};
 };

diff --git a/pdns/dnsdistdist/dnsdist-tcp.cc b/pdns/dnsdistdist/dnsdist-tcp.cc
index 266b72477e8c21559969b575746482619b443d4f..96b5667c435b5c1f109cd67b35fbc0e4484676e4 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-tcp.cc
+++ b/pdns/dnsdistdist/dnsdist-tcp.cc
@@ -1192,8 +1192,39 @@ bool IncomingTCPConnectionState::readIncomingQuery(const timeval& now, IOState&
   return false;
 }
 
+class HandlingIOGuard
+{
+public:
+  HandlingIOGuard(bool& handlingIO) :
+    d_handlingIO(handlingIO)
+  {
+  }
+  HandlingIOGuard(const HandlingIOGuard&) = delete;
+  HandlingIOGuard(HandlingIOGuard&&) = delete;
+  HandlingIOGuard& operator=(const HandlingIOGuard& rhs) = delete;
+  HandlingIOGuard& operator=(HandlingIOGuard&&) = delete;
+  ~HandlingIOGuard()
+  {
+    d_handlingIO = false;
+  }
+
+private:
+  bool& d_handlingIO;
+};
+
 void IncomingTCPConnectionState::handleIO()
 {
+  // let's make sure we are not already in handleIO() below in the stack:
+  // this might happen when we have a response available on the backend socket
+  // right after forwarding the query, and then a query waiting for us on the
+  // client socket right after forwarding the response, and then a response available
+  // on the backend socket right after forwarding the query.. you get the idea.
+  if (d_handlingIO) {
+    return;
+  }
+  d_handlingIO = true;
+  HandlingIOGuard reentryGuard(d_handlingIO);
+
   // why do we loop? Because the TLS layer does buffering, and thus can have data ready to read
   // even though the underlying socket is not ready, so we need to actually ask for the data first
   IOState iostate = IOState::Done;