]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
dnsdist: Prevent a cleanup race between the DOHUnit and the request pool
authorRemi Gacogne <remi.gacogne@powerdns.com>
Fri, 26 Jun 2020 09:50:53 +0000 (11:50 +0200)
committerRemi Gacogne <remi.gacogne@powerdns.com>
Fri, 26 Jun 2020 16:01:52 +0000 (18:01 +0200)
- clean up the pointer in pool memory when releasing a DOHUnit so that we
  don't try to access it later when the memory pool is destroyed ;
- clean up the 'self' pointer when the memory pool is destroyed so we
  don't try to access it when the DOHUnit is released.

pdns/dnsdistdist/doh.cc
pdns/doh.hh

index 8d5d749d3c79d2211ea3e2e87d744aa5818618bf..cdea0e4300e37d2338e579eaa51deaa9e1f297a7 100644 (file)
@@ -652,9 +652,9 @@ static h2o_pathconf_t *register_handler(h2o_hostconf_t *hostconf, const char *pa
    We use this to signal to the 'du' that this req is no longer alive */
 static void on_generator_dispose(void *_self)
 {
-  DOHUnit** du = (DOHUnit**)_self;
-  if(*du) { // if 0, on_dnsdist cleaned up du already
-//    cout << "du "<<(void*)*du<<" containing req "<<(*du)->req<<" got killed"<<endl;
+  DOHUnit** du = reinterpret_cast<DOHUnit**>(_self);
+  if (*du) { // if 0, on_dnsdist cleaned up du already
+    (*du)->self = nullptr;
     (*du)->req = nullptr;
   }
 }
@@ -1039,6 +1039,13 @@ static void dnsdistclient(int qsock)
         continue;
       }
 
+      if (!du->req) {
+        // it got killed in flight already
+        du->self = nullptr;
+        du->release();
+        continue;
+      }
+
       // if there was no EDNS, we add it with a large buffer size
       // so we can use UDP to talk to the backend.
       auto dh = const_cast<struct dnsheader*>(reinterpret_cast<const struct dnsheader*>(du->query.c_str()));
@@ -1109,12 +1116,15 @@ static void on_dnsdist(h2o_socket_t *listener, const char *err)
   }
 
   if (!du->req) { // it got killed in flight
-//    cout << "du "<<(void*)du<<" came back from dnsdist, but it was killed"<<endl;
+    du->self = nullptr;
     du->release();
     return;
   }
 
-  *du->self = nullptr; // so we don't clean up again in on_generator_dispose
+  if (du->self) {
+    *du->self = nullptr; // so we don't clean up again in on_generator_dispose
+    du->self = nullptr;
+  }
 
   handleResponse(*dsc->df, du->req, du->status_code, du->response, dsc->df->d_customResponseHeaders, du->contentType, true);
 
index 53ee0e433f6ee7b91b25dda3ed2cdbc049834e50..40735b6211df1e8ce3fe6ee8619d9724c1b54998 100644 (file)
@@ -176,6 +176,10 @@ struct DOHUnit
   void release()
   {
     if (--d_refcnt == 0) {
+      if (self) {
+        *self = nullptr;
+      }
+
       delete this;
     }
   }