]> granicus.if.org Git - pdns/commitdiff
dnsdist: Fix the oustanding counter when an exception is raised
authorRemi Gacogne <remi.gacogne@powerdns.com>
Thu, 14 Dec 2017 11:28:34 +0000 (12:28 +0100)
committerRemi Gacogne <remi.gacogne@powerdns.com>
Tue, 13 Feb 2018 13:58:52 +0000 (14:58 +0100)
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)

pdns/dnsdist.cc

index 53c25c865dc0deae8fe37a25e9c43a9c5d4b295d..3fd8c0aa65a4a6c07fd0b7f453fed43287ba9b2f 100644 (file)
@@ -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;