From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Thu, 22 Dec 2022 09:25:00 +0000 (+0100)
Subject: channel: Transfer the object to a local pointer  before sending it
X-Git-Tag: rec-5.0.0-alpha1~161^2~4
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=465d1e15f1e511fe5eb80e426a18005297892bec;p=thirdparty%2Fpdns.git

channel: Transfer the object to a local pointer  before sending it

Even calling release() on the initial unique_ptr after sending the
object could cause a use-after-free, as the unique_ptr might have
been destroyed in the meantime.
---

diff --git a/pdns/channel.hh b/pdns/channel.hh
index d220040341..119c1a24fb 100644
--- a/pdns/channel.hh
+++ b/pdns/channel.hh
@@ -220,8 +220,10 @@ namespace channel
   template <typename T, typename D>
   bool Sender<T, D>::send(std::unique_ptr<T, D>&& object) const
   {
-    // we do not release right away because we might need the custom deleter later
-    auto ptr = object.get();
+    /* we cannot touch the initial unique pointer after writing to the pipe,
+       not even to release it, so let's transfer it to a local object */
+    auto localObj = std::move(object);
+    auto ptr = localObj.get();
     static_assert(sizeof(ptr) <= PIPE_BUF, "Writes up to PIPE_BUF are guaranted not to interleaved and to either fully succeed or fail");
     while (true) {
 #if __SANITIZE_THREAD__
@@ -230,11 +232,13 @@ namespace channel
       ssize_t sent = write(d_fd.getHandle(), &ptr, sizeof(ptr));
 
       if (sent == sizeof(ptr)) {
-        // we cannot touch it anymore
-        object.release();
+        localObj.release();
         return true;
       }
       else if (sent == 0) {
+#if __SANITIZE_THREAD__
+        __tsan_acquire(ptr);
+#endif /* __SANITIZE_THREAD__ */
         throw std::runtime_error("Unable to write to channel: remote end has been closed");
       }
       else {
@@ -245,6 +249,7 @@ namespace channel
           continue;
         }
         if (errno == EAGAIN || errno == EWOULDBLOCK) {
+          object = std::move(localObj);
           return false;
         }
         else {