From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Tue, 13 May 2025 13:50:21 +0000 (+0200)
Subject: dnsdist: Fix a crash when TCP queries and responses keep coming
X-Git-Tag: dnsdist-2.0.0-alpha2~5^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=bb9aaec283b97c1ef2081cc66a2cf33e6edc40ea;p=thirdparty%2Fpdns.git

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 45d989a736..d592262fa2 100644
--- 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 266b72477e..96b5667c43 100644
--- 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;