]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
auth: Gracefully handle uncaught exceptions in the UDP path 10770/head
authorRemi Gacogne <remi.gacogne@powerdns.com>
Fri, 16 Jul 2021 12:16:15 +0000 (14:16 +0200)
committerRemi Gacogne <remi.gacogne@powerdns.com>
Tue, 28 Sep 2021 12:35:32 +0000 (14:35 +0200)
These exceptions should never bubble up to this point, but if it
does it makes no sense to terminate the whole process because of it.
This commit logs a message at error level and moves on to the next
query, like we do in the TCP path.

pdns/common_startup.cc

index f591db569786e27bf2fa5bf305373932fe61bc1b..c80607b5418033afbdacde9accab614ebd120c90 100644 (file)
@@ -414,11 +414,16 @@ static void sendout(std::unique_ptr<DNSPacket>& a)
 {
   if(!a)
     return;
-  
-  N->send(*a);
 
-  int diff=a->d_dt.udiff();
-  avg_latency=0.999*avg_latency+0.001*diff;
+  try {
+    N->send(*a);
+
+    int diff=a->d_dt.udiff();
+    avg_latency=0.999*avg_latency+0.001*diff;
+  }
+  catch (const std::exception& e) {
+    g_log<<Logger::Error<<"Caught unhandled exception while sending a response: "<<e.what()<<endl;
+  }
 }
 
 //! The qthread receives questions over the internet via the Nameserver class, and hands them to the Distributor for further processing
@@ -458,81 +463,86 @@ try
   }
 
   for(;;) {
-    if (g_proxyProtocolACL.empty()) {
-      buffer.resize(DNSPacket::s_udpTruncationThreshold);
-    }
-    else {
-      buffer.resize(DNSPacket::s_udpTruncationThreshold + g_proxyProtocolMaximumSize);
-    }
+    try {
+      if (g_proxyProtocolACL.empty()) {
+        buffer.resize(DNSPacket::s_udpTruncationThreshold);
+      }
+      else {
+        buffer.resize(DNSPacket::s_udpTruncationThreshold + g_proxyProtocolMaximumSize);
+      }
 
-    if(!NS->receive(question, buffer)) { // receive a packet         inline
-      continue;                    // packet was broken, try again
-    }
+      if(!NS->receive(question, buffer)) { // receive a packet         inline
+        continue;                    // packet was broken, try again
+      }
 
-    numreceived++;
+      numreceived++;
 
-    if(question.d_remote.getSocklen()==sizeof(sockaddr_in))
-      numreceived4++;
-    else
-      numreceived6++;
+      if(question.d_remote.getSocklen()==sizeof(sockaddr_in))
+        numreceived4++;
+      else
+        numreceived6++;
 
-    if(question.d_dnssecOk)
-      numreceiveddo++;
+      if(question.d_dnssecOk)
+        numreceiveddo++;
 
-    if(question.hasEDNSCookie())
-      numreceivedcookie++;
+      if(question.hasEDNSCookie())
+        numreceivedcookie++;
 
-     if(question.d.qr)
-       continue;
+      if(question.d.qr)
+        continue;
 
-    S.ringAccount("queries", question.qdomain, question.qtype);
-    S.ringAccount("remotes", question.d_remote);
-    if(logDNSQueries) {
-      g_log << Logger::Notice<<"Remote "<< question.getRemoteString() <<" wants '" << question.qdomain<<"|"<<question.qtype <<
-        "', do = " <<question.d_dnssecOk <<", bufsize = "<< question.getMaxReplyLen();
-      if(question.d_ednsRawPacketSizeLimit > 0 && question.getMaxReplyLen() != (unsigned int)question.d_ednsRawPacketSizeLimit)
-        g_log<<" ("<<question.d_ednsRawPacketSizeLimit<<")";
-    }
+      S.ringAccount("queries", question.qdomain, question.qtype);
+      S.ringAccount("remotes", question.d_remote);
+      if(logDNSQueries) {
+        g_log << Logger::Notice<<"Remote "<< question.getRemoteString() <<" wants '" << question.qdomain<<"|"<<question.qtype <<
+          "', do = " <<question.d_dnssecOk <<", bufsize = "<< question.getMaxReplyLen();
+        if(question.d_ednsRawPacketSizeLimit > 0 && question.getMaxReplyLen() != (unsigned int)question.d_ednsRawPacketSizeLimit)
+          g_log<<" ("<<question.d_ednsRawPacketSizeLimit<<")";
+      }
+
+      if(PC.enabled() && (question.d.opcode != Opcode::Notify && question.d.opcode != Opcode::Update) && question.couldBeCached()) {
+        bool haveSomething=PC.get(question, cached); // does the PacketCache recognize this question?
+        if (haveSomething) {
+          if(logDNSQueries)
+            g_log<<": packetcache HIT"<<endl;
+          cached.setRemote(&question.d_remote);  // inlined
+          cached.setSocket(question.getSocket());                               // inlined
+          cached.d_anyLocal = question.d_anyLocal;
+          cached.setMaxReplyLen(question.getMaxReplyLen());
+          cached.d.rd=question.d.rd; // copy in recursion desired bit
+          cached.d.id=question.d.id;
+          cached.commitD(); // commit d to the packet                        inlined
+          NS->send(cached); // answer it then                              inlined
+          diff=question.d_dt.udiff();
+          avg_latency=0.999*avg_latency+0.001*diff; // 'EWMA'
+          continue;
+        }
+      }
 
-    if(PC.enabled() && (question.d.opcode != Opcode::Notify && question.d.opcode != Opcode::Update) && question.couldBeCached()) {
-      bool haveSomething=PC.get(question, cached); // does the PacketCache recognize this question?
-      if (haveSomething) {
+      if(distributor->isOverloaded()) {
         if(logDNSQueries)
-          g_log<<": packetcache HIT"<<endl;
-        cached.setRemote(&question.d_remote);  // inlined
-        cached.setSocket(question.getSocket());                               // inlined
-        cached.d_anyLocal = question.d_anyLocal;
-        cached.setMaxReplyLen(question.getMaxReplyLen());
-        cached.d.rd=question.d.rd; // copy in recursion desired bit
-        cached.d.id=question.d.id;
-        cached.commitD(); // commit d to the packet                        inlined
-        NS->send(cached); // answer it then                              inlined
-        diff=question.d_dt.udiff();
-        avg_latency=0.999*avg_latency+0.001*diff; // 'EWMA'
+          g_log<<": Dropped query, backends are overloaded"<<endl;
+        overloadDrops++;
         continue;
       }
-    }
-
-    if(distributor->isOverloaded()) {
-      if(logDNSQueries)
-        g_log<<": Dropped query, backends are overloaded"<<endl;
-      overloadDrops++;
-      continue;
-    }
 
-    if (logDNSQueries) {
-      if (PC.enabled()) {
-        g_log<<": packetcache MISS"<<endl;
-      } else {
-        g_log<<endl;
+      if (logDNSQueries) {
+        if (PC.enabled()) {
+          g_log<<": packetcache MISS"<<endl;
+        } else {
+          g_log<<endl;
+        }
       }
-    }
 
-    try {
-      distributor->question(question, &sendout); // otherwise, give to the distributor
+      try {
+        distributor->question(question, &sendout); // otherwise, give to the distributor
+      }
+      catch(DistributorFatal& df) { // when this happens, we have leaked loads of memory. Bailing out time.
+        _exit(1);
+      }
     }
-    catch(DistributorFatal& df) { // when this happens, we have leaked loads of memory. Bailing out time.
-      _exit(1);
+    catch (const std::exception& e) {
+      g_log<<Logger::Error<<"Caught unhandled exception in question thread: "<<e.what()<<endl;
     }
   }
 }