From 465d1e15f1e511fe5eb80e426a18005297892bec Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Thu, 22 Dec 2022 10:25:00 +0100 Subject: [PATCH] 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. --- pdns/channel.hh | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) 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 bool Sender::send(std::unique_ptr&& 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 { -- 2.47.2