aws · torben-hansen · May 8, 2021 · Apr 30, 2021 · Apr 30, 2021 · Apr 30, 2021
@@ -42,37 +42,108 @@ DEFINE_STATIC_MUTEX(g_fork_detect_lock)
 DEFINE_BSS_GET(volatile char *, g_fork_detect_addr)
 DEFINE_BSS_GET(uint64_t, g_fork_generation)
 DEFINE_BSS_GET(int, g_ignore_madv_wipeonfork)
+DEFINE_BSS_GET(int, g_ignore_pthread_atfork)
 
+static int init_fork_detect_madv_wipeonfork(void *addr, long page_size) {
+
+  // Some versions of qemu (up to at least 5.0.0-rc4, see linux-user/syscall.c)
+  // ignore |madvise| calls and just return zero (i.e. success). But we need to
+  // know whether MADV_WIPEONFORK actually took effect. Therefore try an invalid
+  // call to check that the implementation of |madvise| is actually rejecting
+  // unknown |advice| values.
+  if (madvise(addr, (size_t)page_size, -1) == 0 ||
+      madvise(addr, (size_t)page_size, MADV_WIPEONFORK) != 0) {
+    munmap(addr, (size_t)page_size);
+    return 0;
+  }
+
+  return 1;
+}
+
+static void pthread_atfork_on_fork(void) {
+
+  struct CRYPTO_STATIC_MUTEX *const lock = g_fork_detect_lock_bss_get();
+
+  // This zeroises the first byte of the memory page pointed to by
+  // |*g_fork_detect_addr_bss_get|. This is the same byte used as fork
+  // detection sentinel in |CRYPTO_get_fork_generation|. The same memory page,
+  // and in turn, the byte, is also the memory zeroised by the |MADV_WIPEONFORK|
+  // fork detection mechanism.
+  //
+  // Aquire locks to be on the safe side. We want to avoid the checks in
+  // |CRYPTO_get_fork_generation| getting executed before setting the sentinel
+  // flag. The write lock prevents any other thread from owning any other type
+  // of lock.
+  CRYPTO_STATIC_MUTEX_lock_write(lock);
+  volatile char *const flag_ptr = *g_fork_detect_addr_bss_get();
+  *flag_ptr = 0;
+  CRYPTO_STATIC_MUTEX_unlock_write(lock);
+}
+
+static int init_fork_detect_pthread_atfork(void) {
+
+  // Register the fork handler |pthread_atfork_on_fork| that is excuted in the
+  // child process after |fork| processing completes.
+  if (pthread_atfork(NULL, NULL, pthread_atfork_on_fork) != 0) {
+    // Returns 0 on success:
+    // https://man7.org/linux/man-pages/man3/pthread_atfork.3.html#RETURN_VALUE
+    return 0;
+  }
+  return 1;
+}
+
+// We employ a layered approach to fork detection using two different
+// mechanisms:
+//  1) |MADV_WIPE_ON_FORK| a memory page through |madvise|.
+//  2) Register a fork handler through |pthread_atfork|.
 static void init_fork_detect(void) {
-  if (*g_ignore_madv_wipeonfork_bss_get()) {
-    return;
+
+  int res = 0;
+  void *addr = MAP_FAILED;
+  long page_size = 0;
+
+  /*
+   * Check whether we are completely ignoring fork detection. This is only
+   * done during testing.
+   */
+  if (*g_ignore_madv_wipeonfork_bss_get() == 1 &&
+      *g_ignore_pthread_atfork_bss_get() == 1) {
+    goto cleanup;
   }
 
-  long page_size = sysconf(_SC_PAGESIZE);
+  page_size = sysconf(_SC_PAGESIZE);
   if (page_size <= 0) {
-    return;
+    goto cleanup;
   }
 
-  void *addr = mmap(NULL, (size_t)page_size, PROT_READ | PROT_WRITE,
+  addr = mmap(NULL, (size_t)page_size, PROT_READ | PROT_WRITE,
                     MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
   if (addr == MAP_FAILED) {
-    return;
+    goto cleanup;
   }
 
-  // Some versions of qemu (up to at least 5.0.0-rc4, see linux-user/syscall.c)
-  // ignore |madvise| calls and just return zero (i.e. success). But we need to
-  // know whether MADV_WIPEONFORK actually took effect. Therefore try an invalid
-  // call to check that the implementation of |madvise| is actually rejecting
-  // unknown |advice| values.
-  if (madvise(addr, (size_t)page_size, -1) == 0 ||
-      madvise(addr, (size_t)page_size, MADV_WIPEONFORK) != 0) {
-    munmap(addr, (size_t)page_size);
-    return;
+  if (*g_ignore_madv_wipeonfork_bss_get() != 1) {
+    if (init_fork_detect_madv_wipeonfork(addr, page_size) == 0) {
+      goto cleanup;
+    }
+  }
+
+  if (*g_ignore_pthread_atfork_bss_get() != 1) {
+    if (init_fork_detect_pthread_atfork() == 0) {
+      goto cleanup;
+    }
   }
 
   *((volatile char *) addr) = 1;
   *g_fork_detect_addr_bss_get() = addr;
   *g_fork_generation_bss_get() = 1;
+
+  res = 1;
+
+cleanup:
+  if (res == 0 && addr != MAP_FAILED) {
+    munmap(addr, (size_t)page_size);
+  }
 }
 
 uint64_t CRYPTO_get_fork_generation(void) {
@@ -130,6 +201,10 @@ void CRYPTO_fork_detect_ignore_madv_wipeonfork_for_testing(void) {
   *g_ignore_madv_wipeonfork_bss_get() = 1;
 }
 
+void CRYPTO_fork_detect_ignore_pthread_atfork_for_testing(void) {
+  *g_ignore_pthread_atfork_bss_get() = 1;
+}
+
 #else   // !OPENSSL_LINUX
 
 uint64_t CRYPTO_get_fork_generation(void) { return 0; }

@@ -42,6 +42,10 @@ OPENSSL_EXPORT uint64_t CRYPTO_get_fork_generation(void);
 // used for testing purposes.
 OPENSSL_EXPORT void CRYPTO_fork_detect_ignore_madv_wipeonfork_for_testing(void);
 
+// CRYPTO_fork_detect_ignore_pthread_atfork_for_testing is an internal detail
+// used for testing purposes.
+OPENSSL_EXPORT void CRYPTO_fork_detect_ignore_pthread_atfork_for_testing(void);
+
 #if defined(__cplusplus)
 }  // extern C
 #endif

@@ -100,6 +100,15 @@ static void ForkInChild(std::function<void()> f) {
 }
 
 TEST(ForkDetect, Test) {
+
+  if (getenv("BORINGSSL_IGNORE_MADV_WIPEONFORK")) {
+    CRYPTO_fork_detect_ignore_madv_wipeonfork_for_testing();
+  }
+
+  if (getenv("BORINGSSL_IGNORE_PTHREAD_ATFORK")) {
+    CRYPTO_fork_detect_ignore_pthread_atfork_for_testing();
+  }
+
   const uint64_t start = CRYPTO_get_fork_generation();
   if (start == 0) {
     fprintf(stderr, "Fork detection not supported. Skipping test.\n");

@@ -512,6 +512,10 @@ int main(int argc, char **argv) {
     CRYPTO_fork_detect_ignore_madv_wipeonfork_for_testing();
   }
 
+  if (getenv("BORINGSSL_IGNORE_PTHREAD_ATFORK")) {
+    CRYPTO_fork_detect_ignore_pthread_atfork_for_testing();
+  }
+
   return RUN_ALL_TESTS();
 }
 

@@ -39,13 +39,27 @@
 #include <unistd.h>
 #endif
 
+static void maybe_disable_some_fork_detect_mechanisms(void) {
+#if defined(OPENSSL_LINUX)
+  if (getenv("BORINGSSL_IGNORE_MADV_WIPEONFORK")) {
+    CRYPTO_fork_detect_ignore_madv_wipeonfork_for_testing();
+  }
+
+  if (getenv("BORINGSSL_IGNORE_PTHREAD_ATFORK")) {
+    CRYPTO_fork_detect_ignore_pthread_atfork_for_testing();
+  }
+#endif
+}
+
 
 // These tests are, strictly speaking, flaky, but we use large enough buffers
 // that the probability of failing when we should pass is negligible.
 
 TEST(RandTest, NotObviouslyBroken) {
   static const uint8_t kZeros[256] = {0};
 
+  maybe_disable_some_fork_detect_mechanisms();
+
   uint8_t buf1[256], buf2[256];
   RAND_bytes(buf1, sizeof(buf1));
   RAND_bytes(buf2, sizeof(buf2));
@@ -129,6 +143,8 @@ static bool ForkAndRand(bssl::Span<uint8_t> out) {
 TEST(RandTest, Fork) {
   static const uint8_t kZeros[16] = {0};
 
+  maybe_disable_some_fork_detect_mechanisms();
+
   // Draw a little entropy to initialize any internal PRNG buffering.
   uint8_t byte;
   RAND_bytes(&byte, 1);
@@ -181,6 +197,8 @@ TEST(RandTest, Threads) {
   constexpr size_t kFewerThreads = 10;
   constexpr size_t kMoreThreads = 20;
 
+  maybe_disable_some_fork_detect_mechanisms();
+
   // Draw entropy in parallel.
   RunConcurrentRands(kFewerThreads);
   // Draw entropy in parallel with higher concurrency than the previous maximum.
@@ -197,6 +215,8 @@ TEST(RandTest, RdrandABI) {
     return;
   }
 
+  maybe_disable_some_fork_detect_mechanisms();
+
   uint8_t buf[32];
   CHECK_ABI_SEH(CRYPTO_rdrand, buf);
   CHECK_ABI_SEH(CRYPTO_rdrand_multiple8_buf, nullptr, 0);

@@ -35,21 +35,46 @@
     "skip_valgrind": true
   },
   {
-    "comment": "No RDRAND and without WIPEONFORK",
+    "comment": "No RDRAND and without fork detection",
     "cmd": ["crypto/urandom_test"],
-    "env": ["OPENSSL_ia32cap=~0x4000000000000000", "BORINGSSL_IGNORE_MADV_WIPEONFORK=1"],
+    "env": ["OPENSSL_ia32cap=~0x4000000000000000", "BORINGSSL_IGNORE_MADV_WIPEONFORK=1", "BORINGSSL_IGNORE_PTHREAD_ATFORK=1"],
     "skip_valgrind": true
   },
   {
-    "comment": "Potentially with RDRAND, but not Intel, and no WIPEONFORK",
+    "comment": "Potentially with RDRAND, but not Intel, and without fork detection",
     "cmd": ["crypto/urandom_test"],
-    "env": ["OPENSSL_ia32cap=~0x0000000040000000", "BORINGSSL_IGNORE_MADV_WIPEONFORK=1"],
+    "env": ["OPENSSL_ia32cap=~0x0000000040000000", "BORINGSSL_IGNORE_MADV_WIPEONFORK=1", "BORINGSSL_IGNORE_PTHREAD_ATFORK=1"],
     "skip_valgrind": true
   },
   {
+    "comment": "Run RAND test suite without fork detection",
     "cmd": ["crypto/crypto_test", "--fork_unsafe_buffering", "--gtest_filter=RandTest.*:-RandTest.Fork"],
     "skip_valgrind": true
   },
+  {
+    "comment": "Run RAND test suite with only madv WIPEONFORK enabled",
+    "cmd": ["crypto/crypto_test", "--gtest_filter=RandTest.*"],
+    "env": ["BORINGSSL_IGNORE_PTHREAD_ATFORK=1"],
+    "skip_valgrind": true
+  },
+  {
+    "comment": "Run RAND test suite with only pthread_atfork enabled",
+    "cmd": ["crypto/crypto_test", "--gtest_filter=RandTest.*"],
+    "env": ["BORINGSSL_IGNORE_MADV_WIPEONFORK=1"],
+    "skip_valgrind": true
+  },
+  {
+    "comment": "Run fork detection test suite with only madv WIPEONFORK enabled",
+    "cmd": ["crypto/crypto_test", "--gtest_filter=ForkDetect.*"],
+    "env": ["BORINGSSL_IGNORE_PTHREAD_ATFORK=1"],
+    "skip_valgrind": true
+  },
+  {
+    "comment": "Run fork detection test suite with only pthread_atfork enabled",
+    "cmd": ["crypto/crypto_test", "--gtest_filter=ForkDetect.*"],
+    "env": ["BORINGSSL_IGNORE_MADV_WIPEONFORK=1"],
+    "skip_valgrind": true
+  },
   {
     "cmd": ["decrepit/decrepit_test"],
     "skip_valgrind": true

@@ -1 +1,2 @@
 This file is ignored. It exists to make no-op commits to trigger new builds.
+
Original file line number	Diff line number	Diff line change
		@@ -1 +1,2 @@
		This file is ignored. It exists to make no-op commits to trigger new builds.