dnsdist: Fix/silence Coverity "auto causes copy" warnings

I'm not very happy with what Coverity is doing with this rule: while
it does find some actual potential performance improvements, more
often than not what it suggests would actually lead to a use-after-free
issue.

diff --git a/pdns/dnsdistdist/dnsdist-carbon.cc b/pdns/dnsdistdist/dnsdist-carbon.cc
index ee396d0452c54da1b66ae8737e1ceb2fc73fefdb..6fa67e55d432b725cffead11e7f279958ac6d767 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-carbon.cc
+++ b/pdns/dnsdistdist/dnsdist-carbon.cc
@@ -349,7 +349,7 @@ Carbon::Endpoint Carbon::newEndpoint(const std::string& address, std::string our
   }
   return Carbon::Endpoint{ComboAddress(address, 2003),
                           !namespace_name.empty() ? namespace_name : "dnsdist",
-                          ourName,
+                          std::move(ourName),
                           !instance_name.empty() ? instance_name : "main",
                           interval < std::numeric_limits<unsigned int>::max() ? static_cast<unsigned int>(interval) : 30};
 }

diff --git a/pdns/dnsdistdist/dnsdist-console.cc b/pdns/dnsdistdist/dnsdist-console.cc
index f5ed5c705f202d0073b34687924fa56e0cba0ac8..8994c338af51936664658805a467066b9f1f2ce0 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-console.cc
+++ b/pdns/dnsdistdist/dnsdist-console.cc
@@ -225,7 +225,9 @@ namespace dnsdist::console
 {
 void doClient(const std::string& command)
 {
+  //coverity[auto_causes_copy]
   const auto consoleKey = dnsdist::configuration::getCurrentRuntimeConfiguration().d_consoleKey;
+  //coverity[auto_causes_copy]
   const auto server = dnsdist::configuration::getCurrentRuntimeConfiguration().d_consoleServerAddress;
   if (!dnsdist::crypto::authenticated::isValidKey(consoleKey)) {
     cerr << "The currently configured console key is not valid, please configure a valid key using the setKey() directive" << endl;
@@ -932,6 +934,7 @@ static void controlClientThread(ConsoleConnection&& conn)
 
     setTCPNoDelay(conn.getFD());
 
+    //coverity[auto_causes_copy]
     const auto consoleKey = dnsdist::configuration::getCurrentRuntimeConfiguration().d_consoleKey;
     dnsdist::crypto::authenticated::Nonce theirs;
     dnsdist::crypto::authenticated::Nonce ours;
@@ -957,6 +960,7 @@ static void controlClientThread(ConsoleConnection&& conn)
       }
 
       std::string line;
+      //coverity[tainted_data]
       line.resize(len);
       readn2(conn.getFD(), line.data(), len);
 

diff --git a/pdns/dnsdistdist/dnsdist-lua-rules.cc b/pdns/dnsdistdist/dnsdist-lua-rules.cc
index 6343dba706304754c4447fecf655fc35e9438aaa..10937c03fcdf29853298713d42c6d85623871214 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-lua-rules.cc
+++ b/pdns/dnsdistdist/dnsdist-lua-rules.cc
@@ -211,6 +211,7 @@ static void moveRuleToTop(IdentifierTypeT chainIdentifier)
     if (rules.empty()) {
       return;
     }
+    //coverity[auto_causes_copy]
     auto subject = *rules.rbegin();
     rules.erase(std::prev(rules.end()));
     rules.insert(rules.begin(), subject);
@@ -227,6 +228,7 @@ static void mvRule(IdentifierTypeT chainIdentifier, unsigned int from, unsigned
       g_outputBuffer = "Error: attempt to move rules from/to invalid index\n";
       return;
     }
+    //coverity[auto_causes_copy]
     auto subject = rules[from];
     rules.erase(rules.begin() + from);
     if (destination > rules.size()) {

diff --git a/pdns/dnsdistdist/dnsdist-lua.cc b/pdns/dnsdistdist/dnsdist-lua.cc
index e75227d01ab02bfc6169c332fd9dc7f3a59ac47a..bc3d3df0ec399a8b190f9a1c1c22bdd7e6d27959 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-lua.cc
+++ b/pdns/dnsdistdist/dnsdist-lua.cc
@@ -1244,6 +1244,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
   });
 
   luaCtx.writeFunction("getPoolServers", [](const string& pool) {
+    //coverity[auto_causes_copy]
     const auto poolServers = getDownstreamCandidates(pool);
     return *poolServers;
   });
@@ -1852,7 +1853,9 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
       //             1        2         3                4
       ret << (fmt % "Name" % "Cache" % "ServerPolicy" % "Servers") << endl;
 
+      //coverity[auto_causes_copy]
       const auto defaultPolicyName = dnsdist::configuration::getCurrentRuntimeConfiguration().d_lbPolicy->getName();
+      //coverity[auto_causes_copy]
       const auto pools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools;
       for (const auto& entry : pools) {
         const string& name = entry.first;
@@ -1890,7 +1893,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
     setLuaNoSideEffect();
     LuaArray<std::string> ret;
     int count = 1;
-    const auto pools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools;
+    const auto& pools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools;
     for (const auto& entry : pools) {
       const string& name = entry.first;
       ret.emplace_back(count++, name);

diff --git a/pdns/dnsdistdist/dnsdist-web.cc b/pdns/dnsdistdist/dnsdist-web.cc
index 7790e755f0d34a709209c45160de430ff7fab734..acf1d31f8953bccd53dfdb9d522eba076b439c45 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-web.cc
+++ b/pdns/dnsdistdist/dnsdist-web.cc
@@ -1238,7 +1238,7 @@ static void handleStats(const YaHTTP::Request& req, YaHTTP::Response& resp)
   Json::array pools;
   {
     num = 0;
-    const auto localPools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools;
+    const auto& localPools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools;
     pools.reserve(localPools.size());
     for (const auto& pool : localPools) {
       const auto& cache = pool.second->packetCache;

diff --git a/pdns/dnsdistdist/dnsdist.cc b/pdns/dnsdistdist/dnsdist.cc
index 41e8c1ca6d99dd604a6db9f9e9eaea95f67f9fe6..5c712f268cbe9daa0438789b25baa3e4418f817f 100644 (file)
--- a/pdns/dnsdistdist/dnsdist.cc
+++ b/pdns/dnsdistdist/dnsdist.cc
@@ -1497,6 +1497,7 @@ ProcessQueryResult processQueryAfterRules(DNSQuestion& dnsQuestion, std::shared_
 
       ++dnsdist::metrics::g_stats.cacheMisses;
 
+      //coverity[auto_causes_copy]
       const auto existingPool = dnsQuestion.ids.poolName;
       const auto& chains = dnsdist::configuration::getCurrentRuntimeConfiguration().d_ruleChains;
       const auto& cacheMissRuleActions = dnsdist::rules::getRuleChain(chains, dnsdist::rules::RuleChain::CacheMissRules);
@@ -2423,7 +2424,7 @@ static void checkFileDescriptorsLimits(size_t udpBindsCount, size_t tcpBindsCoun
   const auto& immutableConfig = dnsdist::configuration::getImmutableConfiguration();
   /* stdin, stdout, stderr */
   rlim_t requiredFDsCount = 3;
-  const auto backends = dnsdist::configuration::getCurrentRuntimeConfiguration().d_backends;
+  const auto& backends = dnsdist::configuration::getCurrentRuntimeConfiguration().d_backends;
   /* UDP sockets to backends */
   size_t backendUDPSocketsCount = 0;
   for (const auto& backend : backends) {
@@ -3484,6 +3485,7 @@ int main(int argc, char** argv)
     checkFileDescriptorsLimits(udpBindsCount, tcpBindsCount);
 
     {
+      //coverity[auto_causes_copy]
       const auto states = dnsdist::configuration::getCurrentRuntimeConfiguration().d_backends; // it is a copy, but the internal shared_ptrs are the real deal
       auto mplexer = std::unique_ptr<FDMultiplexer>(FDMultiplexer::getMultiplexerSilent(states.size()));
       for (auto& dss : states) {