Bug#36081753 use-after-free in TcpPortPool

When built with ASAN, a use-after-free is reported for the TcpPortPool.

AddressSanitizer: heap-use-after-free on address 0x60200019f190 at pc
0x00000076a18d bp 0x7fff51e7d1d0 sp 0x7fff51e7d1c0

    #4 0x770b73 in UniqueId::ProcessUniqueIds::erase(unsigned int)
       ../router/tests/helpers/tcp_port_pool.h:112
    #5 0x770c48 in UniqueId::~UniqueId()
       ../router/tests/helpers/tcp_port_pool.cc:234
    ...
    #12 0x82faa3 in testing::UnitTest::~UnitTest()
	../extra/googletest/googletest-release-1.12.0/googletest/src/gtest.cc:5496
    #13 0x7f5fe085ace8 in __run_exit_handlers (/lib64/libc.so.6+0x39ce8)

0x60200019f190 is located 0 bytes inside of 16-byte region
[0x60200019f190,0x60200019f1a0)
freed by thread T0 here:
    #0 0x7f5fe3cbd10f in operator delete(void*, unsigned long)
       (/lib64/libasan.so.6+0xb710f)
    #1 0x7f5fe085ace8 in __run_exit_handlers (/lib64/libc.so.6+0x39ce8)

Background
==========

__run_exit_handlers destroys "static" and "global" variables in reverse
order of their creation.

googletest's unit-tests are a static, and the TcpPortPool also has
ProcessUniqueId's which contains the process-wide unique-ids.

At construct: unittest -> tcp-port-pool -> proces-unique-ids
At destruct : process-unique-ids -> tcp-port-pool -> 💥

The use-after-free happens as the process-unique-ids static is
destructed before the tcp-port-pool which tries to its Ids from the
process-unique-ids.

Change
======

- extend the lifetime of the process-unique-ids to after the last use of
  the tcp-port-pool via a std::shared_ptr<>

Change-Id: I75b8b781e1d240f18ca72f2c86182639a7699f06

Author: weigon

router/tests/helpers/tcp_port_pool.cc

@@ -34,8 +34,9 @@
 #endif
 
 #include <cstring>
+#include <memory>  // shared_ptr
 #include <system_error>
-#include <utility>
+#include <utility>  // move
 
 #include "mysql/harness/filesystem.h"  // Path
 #include "mysql/harness/stdx/expected.h"
@@ -199,7 +200,8 @@ std::string UniqueId::get_lock_file_dir() {
 #endif
 }
 
-UniqueId::UniqueId(value_type start_from, value_type range) {
+UniqueId::UniqueId(value_type start_from, value_type range)
+    : proc_ids_(process_unique_ids()) {
   const std::string lock_file_dir = get_lock_file_dir();
   mysql_harness::mkdir(lock_file_dir, 0777);
 #ifndef _WIN32
@@ -209,7 +211,7 @@ UniqueId::UniqueId(value_type start_from, value_type range) {
 #endif
 
   for (auto i = start_from; i < start_from + range; i++) {
-    if (process_unique_ids().contains(i)) continue;
+    if (proc_ids_->contains(i)) continue;
 
     Path lock_file_path(lock_file_dir);
     lock_file_path.append(std::to_string(i));
@@ -218,7 +220,7 @@ UniqueId::UniqueId(value_type start_from, value_type range) {
     if (lock_res) {
       id_ = i;
 
-      process_unique_ids().insert(i);
+      proc_ids_->insert(i);
 
       // obtained the lock, we are good to go
       return;
@@ -231,12 +233,29 @@ UniqueId::UniqueId(value_type start_from, value_type range) {
 UniqueId::~UniqueId() {
   // release the process unique-id if we own one.
   if (id_) {
-    process_unique_ids().erase(*id_);
+    proc_ids_->erase(*id_);
   }
 }
 
-UniqueId::ProcessUniqueIds &UniqueId::process_unique_ids() {
-  static ProcessUniqueIds ids;
+// process-wide unique-ids
+//
+// is a "static shared_ptr<>" instead of a "static" as the TcpPort may be part
+// of a "static" too.
+//
+// It would create:
+// 1. (static) TcpPortPool
+// 2. static ProcessUniqueIds
+//
+// ... and then at destruct in reverse order:
+// * ProcessUniqueIds
+// * TcpPortPool ... but the TcpPortPool would try to remove itself from the
+//   ProcessUniqueIds
+//
+//
+//
+std::shared_ptr<UniqueId::ProcessUniqueIds> UniqueId::process_unique_ids() {
+  static std::shared_ptr<UniqueId::ProcessUniqueIds> ids =
+      std::make_shared<UniqueId::ProcessUniqueIds>();
 
   return ids;
 }

router/tests/helpers/tcp_port_pool.h

@@ -26,6 +26,7 @@
 #define _TCP_PORT_POOL_H_
 
 #include <cstdint>
+#include <memory>
 #include <optional>
 #include <string>
 #include <system_error>
@@ -120,10 +121,12 @@ class UniqueId {
   UniqueId(const UniqueId &) = delete;
   UniqueId &operator=(const UniqueId &) = delete;
   UniqueId(UniqueId &&other) noexcept
-      : id_(std::exchange(other.id_, {})),
+      : proc_ids_(std::move(other.proc_ids_)),
+        id_(std::exchange(other.id_, {})),
         lock_file_fd_(std::exchange(other.lock_file_fd_, {})) {}
 
   UniqueId &operator=(UniqueId &&other) noexcept {
+    proc_ids_ = std::move(other.proc_ids_);
     id_ = std::exchange(other.id_, {});
     lock_file_fd_ = std::exchange(other.lock_file_fd_, {});
 
@@ -137,7 +140,9 @@ class UniqueId {
  private:
   stdx::expected<void, std::error_code> lock_file(const std::string &file_name);
 
-  static ProcessUniqueIds &process_unique_ids();
+  static std::shared_ptr<UniqueId::ProcessUniqueIds> process_unique_ids();
+
+  std::shared_ptr<UniqueId::ProcessUniqueIds> proc_ids_;
 
   static std::string get_lock_file_dir();