From 817901ff3cee433ef5febdfc19ff29487b94fdd8 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Tue, 27 Dec 2022 17:01:55 +0100 Subject: [PATCH] dnsdist: Prevent an underflow of the TCP d_queued counter By incrementing it _before_ writing to the pipe, and decrementing it in case of an error, we prevent a very possible underflow from occurring if the reader manages to decrement before we can return from write and increment it. --- pdns/dnsdistdist/dnsdist-tcp.hh | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-tcp.hh b/pdns/dnsdistdist/dnsdist-tcp.hh index 05b2db4e46..43d615dea2 100644 --- a/pdns/dnsdistdist/dnsdist-tcp.hh +++ b/pdns/dnsdistdist/dnsdist-tcp.hh @@ -182,17 +182,6 @@ class TCPClientCollection public: TCPClientCollection(size_t maxThreads, std::vector tcpStates); - int getThread() - { - if (d_numthreads == 0) { - throw std::runtime_error("No TCP worker thread yet"); - } - - uint64_t pos = d_pos++; - ++d_queued; - return d_tcpclientthreads.at(pos % d_numthreads).d_newConnectionPipe.getHandle(); - } - bool passConnectionToThread(std::unique_ptr&& conn) { if (d_numthreads == 0) { @@ -203,13 +192,17 @@ public: auto pipe = d_tcpclientthreads.at(pos % d_numthreads).d_newConnectionPipe.getHandle(); auto tmp = conn.release(); + /* we need to increment this counter _before_ writing to the pipe, + otherwise there is a very real possiblity that the other end + decrement the counter before we can increment it, leading to an underflow */ + ++d_queued; if (write(pipe, &tmp, sizeof(tmp)) != sizeof(tmp)) { + --d_queued; ++g_stats.tcpQueryPipeFull; delete tmp; tmp = nullptr; return false; } - ++d_queued; return true; } -- 2.47.2