]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
dnsdist: Fix a crash when TCP queries and responses keep coming
authorRemi Gacogne <remi.gacogne@powerdns.com>
Tue, 13 May 2025 13:50:21 +0000 (15:50 +0200)
committerRemi Gacogne <remi.gacogne@powerdns.com>
Mon, 19 May 2025 08:57:21 +0000 (10:57 +0200)
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!).

(cherry picked from commit 368f409b0768c7085602ebf4233ce1e8e4ac0f8c)

pdns/dnsdist-tcp.cc
pdns/dnsdistdist/dnsdist-tcp-upstream.hh

index bfcb1b9dda3f1fc23044287bd5f82e6c9540d5c4..bbe703760111c45b251d0db862eb2b12ad164493 100644 (file)
@@ -1088,8 +1088,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;
index 93d7489bd27767c51d5bc736cab45649ccdaa00a..c0f05627b5ad6e41f641287f92f64212d9b8345a 100644 (file)
@@ -216,4 +216,5 @@ public:
   bool d_proxyProtocolPayloadHasTLV{false};
   bool d_lastIOBlocked{false};
   bool d_hadErrors{false};
+  bool d_handlingIO{false};
 };