1 files changed, 48 insertions, 21 deletions

diff --git a/dnsdist-lua-network.cc b/dnsdist-lua-network.cc
index 6892888..56a58cd 100644
--- a/dnsdist-lua-network.cc
+++ b/dnsdist-lua-network.cc
@@ -28,11 +28,21 @@
 
 namespace dnsdist
 {
-NetworkListener::NetworkListener() :
+NetworkListener::ListenerData::ListenerData() :
   d_mplexer(std::unique_ptr<FDMultiplexer>(FDMultiplexer::getMultiplexerSilent(10)))
 {
 }
 
+NetworkListener::NetworkListener() :
+  d_data(std::make_shared<ListenerData>())
+{
+}
+
+NetworkListener::~NetworkListener()
+{
+  d_data->d_exiting = true;
+}
+
 void NetworkListener::readCB(int desc, FDMultiplexer::funcparam_t& param)
 {
   auto cbData = boost::any_cast<std::shared_ptr<NetworkListener::CBData>>(param);
@@ -40,24 +50,26 @@ void NetworkListener::readCB(int desc, FDMultiplexer::funcparam_t& param)
 
 #ifdef MSG_TRUNC
   /* first we peek to avoid allocating a very large buffer. "MSG_TRUNC [...] return the real length of the datagram, even when it was longer than the passed buffer" */
-  auto peeked = recvfrom(desc, nullptr, 0, MSG_PEEK | MSG_TRUNC, nullptr, 0);
+  auto peeked = recvfrom(desc, nullptr, 0, MSG_PEEK | MSG_TRUNC, nullptr, nullptr);
   if (peeked > 0) {
     packet.resize(static_cast<size_t>(peeked));
   }
 #endif
-  if (packet.size() == 0) {
+  if (packet.empty()) {
     packet.resize(65535);
   }
 
-  struct sockaddr_un from;
+  sockaddr_un from{};
   memset(&from, 0, sizeof(from));
 
   socklen_t fromLen = sizeof(from);
+  // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
   auto got = recvfrom(desc, &packet.at(0), packet.size(), 0, reinterpret_cast<sockaddr*>(&from), &fromLen);
   if (got > 0) {
     packet.resize(static_cast<size_t>(got));
     std::string fromAddr;
     if (fromLen <= sizeof(from)) {
+      // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
       fromAddr = std::string(from.sun_path, strlen(from.sun_path));
     }
     try {
@@ -72,13 +84,13 @@ void NetworkListener::readCB(int desc, FDMultiplexer::funcparam_t& param)
   }
 }
 
-bool NetworkListener::addUnixListeningEndpoint(const std::string& path, NetworkListener::EndpointID id, NetworkListener::NetworkDatagramCB cb)
+bool NetworkListener::addUnixListeningEndpoint(const std::string& path, NetworkListener::EndpointID endpointID, NetworkListener::NetworkDatagramCB callback)
 {
-  if (d_running == true) {
+  if (d_data->d_running) {
     throw std::runtime_error("NetworkListener should not be altered at runtime");
   }
 
-  struct sockaddr_un sun;
+  sockaddr_un sun{};
   if (makeUNsockaddr(path, &sun) != 0) {
     throw std::runtime_error("Invalid Unix socket path '" + path + "'");
   }
@@ -101,6 +113,7 @@ bool NetworkListener::addUnixListeningEndpoint(const std::string& path, NetworkL
     sunLength = sizeof(sa_family_t) + path.size();
   }
 
+  // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
   if (bind(sock.getHandle(), reinterpret_cast<const struct sockaddr*>(&sun), sunLength) != 0) {
     std::string sanitizedPath(path);
     if (abstractPath) {
@@ -112,38 +125,51 @@ bool NetworkListener::addUnixListeningEndpoint(const std::string& path, NetworkL
   sock.setNonBlocking();
 
   auto cbData = std::make_shared<CBData>();
-  cbData->d_endpoint = id;
-  cbData->d_cb = cb;
-  d_mplexer->addReadFD(sock.getHandle(), readCB, cbData);
+  cbData->d_endpoint = endpointID;
+  cbData->d_cb = std::move(callback);
+  d_data->d_mplexer->addReadFD(sock.getHandle(), readCB, cbData);
 
-  d_sockets.insert({path, std::move(sock)});
+  d_data->d_sockets.insert({path, std::move(sock)});
   return true;
 }
 
-void NetworkListener::runOnce(struct timeval& now, uint32_t timeout)
+void NetworkListener::runOnce(ListenerData& data, timeval& now, uint32_t timeout)
 {
-  d_running = true;
-  if (d_sockets.empty()) {
+  if (data.d_exiting) {
+    return;
+  }
+
+  data.d_running = true;
+  if (data.d_sockets.empty()) {
     throw runtime_error("NetworkListener started with no sockets");
   }
 
-  d_mplexer->run(&now, timeout);
+  data.d_mplexer->run(&now, static_cast<int>(timeout));
+}
+
+void NetworkListener::runOnce(timeval& now, uint32_t timeout)
+{
+  runOnce(*d_data, now, timeout);
 }
 
-void NetworkListener::mainThread()
+void NetworkListener::mainThread(std::shared_ptr<ListenerData>& dataArg)
 {
+  /* take our own copy of the shared_ptr so it's still alive if the NetworkListener object
+     gets destroyed while we are still running */
+  // NOLINTNEXTLINE(performance-unnecessary-copy-initialization): we really need a copy here, or we end up with use-after-free as explained above
+  auto data = dataArg;
   setThreadName("dnsdist/lua-net");
-  struct timeval now;
+  timeval now{};
 
-  while (true) {
-    runOnce(now, -1);
+  while (!data->d_exiting) {
+    runOnce(*data, now, -1);
   }
 }
 
 void NetworkListener::start()
 {
   std::thread main = std::thread([this] {
-    mainThread();
+    mainThread(d_data);
   });
   main.detach();
 }
@@ -151,7 +177,7 @@ void NetworkListener::start()
 NetworkEndpoint::NetworkEndpoint(const std::string& path) :
   d_socket(AF_UNIX, SOCK_DGRAM, 0)
 {
-  struct sockaddr_un sun;
+  sockaddr_un sun{};
   if (makeUNsockaddr(path, &sun) != 0) {
     throw std::runtime_error("Invalid Unix socket path '" + path + "'");
   }
@@ -163,6 +189,7 @@ NetworkEndpoint::NetworkEndpoint(const std::string& path) :
     /* abstract paths can contain null bytes so we need to set the actual size */
     sunLength = sizeof(sa_family_t) + path.size();
   }
+  // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
   if (connect(d_socket.getHandle(), reinterpret_cast<const struct sockaddr*>(&sun), sunLength) != 0) {
     std::string sanitizedPath(path);
     if (abstractPath) {