rec: Fix a potential null deref in MTasker::schedule()

The bug is located in a part of the code that we never actually
use since we always pass the current time to the function, so
I decided to reduce the complexity by making this parameter mandatory.

Reported by Coverity as CID 1533199.

diff --git a/pdns/recursordist/mtasker.hh b/pdns/recursordist/mtasker.hh
index beb883003f0243bcb26602c7a28997642fe8b3fb..e9d936676bd947fe88e2d8fb14f295906e9e0b98 100644 (file)
--- a/pdns/recursordist/mtasker.hh
+++ b/pdns/recursordist/mtasker.hh
@@ -85,7 +85,7 @@ public:
   void yield();
   int sendEvent(const EventKey& key, const EventVal* val = nullptr);
   void makeThread(tfunc_t* start, void* val);
-  bool schedule(const struct timeval* now = nullptr);
+  bool schedule(const struct timeval& now);
 
   const waiters_t& getWaiters() const
   {
@@ -395,7 +395,7 @@ void MTasker<Key, Val, Cmp>::makeThread(tfunc_t* start, void* val)
 
 */
 template <class Key, class Val, class Cmp>
-bool MTasker<Key, Val, Cmp>::schedule(const struct timeval* now)
+bool MTasker<Key, Val, Cmp>::schedule(const struct timeval& now)
 {
   if (!d_runQueue.empty()) {
     d_tid = d_runQueue.front();
@@ -433,21 +433,12 @@ bool MTasker<Key, Val, Cmp>::schedule(const struct timeval* now)
     return true;
   }
   if (!d_waiters.empty()) {
-    struct timeval rnow
-    {
-    };
-    if (now != nullptr) {
-      gettimeofday(&rnow, nullptr);
-    }
-    else {
-      rnow = *now;
-    }
     typedef typename waiters_t::template index<KeyTag>::type waiters_by_ttd_index_t;
     //    waiters_by_ttd_index_t& ttdindex=d_waiters.template get<KeyTag>();
     waiters_by_ttd_index_t& ttdindex = boost::multi_index::get<KeyTag>(d_waiters);
 
     for (typename waiters_by_ttd_index_t::iterator i = ttdindex.begin(); i != ttdindex.end();) {
-      if (i->ttd.tv_sec && i->ttd < rnow) {
+      if (i->ttd.tv_sec && i->ttd < now) {
         d_waitstatus = TimeOut;
         d_eventkey = i->key; // pass waitEvent the exact key it was woken for
         auto ucontext = i->context;

diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc
index 6a1ad5a9f91053ac15de506127b1cb3717752547..54e6bc199f2cb2d21f9e6bcbd6d215eaffea7099 100644 (file)
--- a/pdns/recursordist/rec-main.cc
+++ b/pdns/recursordist/rec-main.cc
@@ -2630,7 +2630,7 @@ static void recLoop()
   auto& threadInfo = RecThreadInfo::self();
 
   while (!RecursorControlChannel::stop) {
-    while (g_multiTasker->schedule(&g_now)) {
+    while (g_multiTasker->schedule(g_now)) {
       ; // MTasker letting the mthreads do their thing
     }
 

diff --git a/pdns/recursordist/test-mtasker.cc b/pdns/recursordist/test-mtasker.cc
index c7f2a21905213f8efe25e7422f957b4b322edf17..435f28d1ba6039216f7a889bd2a6b432e4e1c4fc 100644 (file)
--- a/pdns/recursordist/test-mtasker.cc
+++ b/pdns/recursordist/test-mtasker.cc
@@ -32,7 +32,7 @@ BOOST_AUTO_TEST_CASE(test_Simple)
   bool first = true;
   int o = 24;
   for (;;) {
-    while (mt.schedule(&now))
+    while (mt.schedule(now))
       ;
     if (first) {
       mt.sendEvent(12, &o);
@@ -70,7 +70,7 @@ BOOST_AUTO_TEST_CASE(test_AlmostStackOverflow)
   bool first = true;
   int o = 25;
   for (;;) {
-    while (mt.schedule(&now)) {
+    while (mt.schedule(now)) {
       ;
     }
     if (first) {
@@ -98,7 +98,7 @@ BOOST_AUTO_TEST_CASE(test_MtaskerException)
     now.tv_sec = now.tv_usec = 0;
 
     for (;;) {
-      mt.schedule(&now);
+      mt.schedule(now);
     }
   },
                     std::exception);