From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Thu, 14 Dec 2017 11:28:34 +0000 (+0100)
Subject: dnsdist: Fix the oustanding counter when an exception is raised
X-Git-Tag: dnsdist-1.2.1^2~11^2~1
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8ae7f5082ac86f9b1d0257866b93e98aef278bc6;p=pdns

dnsdist: Fix the oustanding counter when an exception is raised

If an exception is raised in the responder thread after the outstanding
queries counter has been decremented, but before we could mark the
state as processed, the same state would be processed again and the
counter decremented a second time, either because of a duplicate
answer or more likely by the timeout handler.
This commit simply increase back the outstanding counter when such
an exception occurs.

(cherry picked from commit df560083763da68488f09ed385c479c10eedf750)
---

diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc
index 53c25c865..3fd8c0aa6 100644
--- a/pdns/dnsdist.cc
+++ b/pdns/dnsdist.cc
@@ -396,6 +396,7 @@ try {
   uint16_t queryId = 0;
   for(;;) {
     dnsheader* dh = reinterpret_cast<struct dnsheader*>(packet);
+    bool outstandingDecreased = false;
     try {
       ssize_t got = recv(state->fd, packet, sizeof(packet), 0);
       char * response = packet;
@@ -428,6 +429,7 @@ try {
       }
 
       --state->outstanding;  // you'd think an attacker could game this, but we're using connected socket
+      outstandingDecreased = true;
 
       if(dh->tc && g_truncateTC) {
         truncateTC(response, &responseLen);
@@ -501,12 +503,21 @@ try {
         ids->dnsCryptQuery = 0;
 #endif
         ids->origFD = -1;
+        outstandingDecreased = false;
       }
 
       rewrittenResponse.clear();
     }
-    catch(std::exception& e){
+    catch(const std::exception& e){
       vinfolog("Got an error in UDP responder thread while parsing a response from %s, id %d: %s", state->remote.toStringWithPort(), queryId, e.what());
+      if (outstandingDecreased) {
+        /* so an exception was raised after we decreased the outstanding queries counter,
+           but before we could set ids->origFD to -1 (because we also set outstandingDecreased
+           to false then), meaning the IDS still considered active and we will decrease the
+           counter again on a duplicate, or simply while reaping downstream timeouts, so let's
+           increase it back. */
+        state->outstanding++;
+      }
     }
   }
   return 0;