[tor-commits] [tor/master] Re-enable and fix unit test for nofork mappings

[asn at torproject.org]

commit 065e7da8e6fdbd9331de8c13344275a8e0fbf32d
Author: Nick Mathewson <nickm at torproject.org>
Date:   Mon Feb 25 08:55:25 2019 -0500

    Re-enable and fix unit test for nofork mappings
    
    This test was previously written to use the contents of the system
    headers to decide whether INHERIT_NONE or INHERIT_ZERO was going to
    work.  But that won't work across different environments, such as
    (for example) when the kernel doesn't match the headers.  Instead,
    we add a testing-only feature to the code to track which of these
    options actually worked, and verify that it behaved as we expected.
    
    Closes ticket 29541; bugfix not on any released version of Tor.
---
 src/lib/malloc/map_anon.c | 34 ++++++++++++++++++++++++++++++++--
 src/lib/malloc/map_anon.h |  4 ++++
 src/test/test_util.c      | 34 +++++++++++++++++++++++++---------
 3 files changed, 61 insertions(+), 11 deletions(-)

diff --git a/src/lib/malloc/map_anon.c b/src/lib/malloc/map_anon.c
index 2fc6e89ea..5dac5256a 100644
--- a/src/lib/malloc/map_anon.c
+++ b/src/lib/malloc/map_anon.c
@@ -107,6 +107,29 @@ nodump_mem(void *mem, size_t sz)
 #endif
 }
 
+#ifdef TOR_UNIT_TESTS
+static unsigned last_anon_map_noinherit = ~0;
+/* Testing helper: return the outcome of the last call to noinherit_mem():
+ * 0 if it did no good; 1 if it caused the memory not to be inherited, and
+ * 2 if it caused the memory to be cleared on fork */
+unsigned
+get_last_anon_map_noinherit(void)
+{
+  return last_anon_map_noinherit;
+}
+static void
+set_last_anon_map_noinherit(unsigned f)
+{
+  last_anon_map_noinherit = f;
+}
+#else
+static void
+set_last_anon_map_noinherit(unsigned f)
+{
+  (void)f;
+}
+#endif
+
 /**
  * Helper: try to prevent the <b>sz</b> bytes at <b>mem</b> from being
  * accessible in child processes -- ideally by having them set to 0 after a
@@ -117,13 +140,20 @@ nodump_mem(void *mem, size_t sz)
 static int
 noinherit_mem(void *mem, size_t sz)
 {
+  set_last_anon_map_noinherit(0);
 #ifdef FLAG_ZERO
   int r = MINHERIT(mem, sz, FLAG_ZERO);
-  if (r == 0)
+  if (r == 0) {
+    set_last_anon_map_noinherit(2);
     return 0;
+  }
 #endif
 #ifdef FLAG_NOINHERIT
-  return MINHERIT(mem, sz, FLAG_NOINHERIT);
+  int r2 = MINHERIT(mem, sz, FLAG_NOINHERIT);
+  if (r2 == 0) {
+    set_last_anon_map_noinherit(1);
+  }
+  return r2;
 #else
   (void)mem;
   (void)sz;
diff --git a/src/lib/malloc/map_anon.h b/src/lib/malloc/map_anon.h
index cc5797e4e..395145bd7 100644
--- a/src/lib/malloc/map_anon.h
+++ b/src/lib/malloc/map_anon.h
@@ -34,4 +34,8 @@
 void *tor_mmap_anonymous(size_t sz, unsigned flags);
 void tor_munmap_anonymous(void *mapping, size_t sz);
 
+#ifdef TOR_UNIT_TESTS
+unsigned get_last_anon_map_noinherit(void);
+#endif
+
 #endif /* !defined(TOR_MAP_ANON_H) */
diff --git a/src/test/test_util.c b/src/test/test_util.c
index 7a2708c54..4990aa709 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -6165,8 +6165,8 @@ static void
 test_util_map_anon_nofork(void *arg)
 {
   (void)arg;
-#if !defined(HAVE_MADVISE) && !defined(HAVE_MINHERIT)
-  /* The operating system doesn't support this. */
+#ifdef _WIN32
+  /* The operating system doesn't support forking. */
   tt_skip();
  done:
   ;
@@ -6182,6 +6182,7 @@ test_util_map_anon_nofork(void *arg)
 
   tor_munmap_anonymous(ptr, sz);
   ptr = tor_mmap_anonymous(sz, ANONMAP_NOINHERIT);
+  int outcome = get_last_anon_map_noinherit();
   tt_ptr_op(ptr, OP_NE, 0);
   memset(ptr, 0xd0, sz);
 
@@ -6202,15 +6203,30 @@ test_util_map_anon_nofork(void *arg)
   pipefd[1] = -1;
   char buf[1];
   ssize_t r = read(pipefd[0], buf, 1);
-#if defined(INHERIT_ZERO) || defined(MADV_WIPEONFORK)
-  tt_int_op((int)r, OP_EQ, 1); // child should send us a byte.
-  tt_int_op(buf[0], OP_EQ, 0);
-#else
-  tt_int_op(r, OP_LE, 0); // child said nothing; it should have crashed.
-#endif
+
+  if (outcome == 2) {
+    // We should be seeing clear-on-fork behavior.
+    tt_int_op((int)r, OP_EQ, 1); // child should send us a byte.
+    tt_int_op(buf[0], OP_EQ, 0); // that byte should be zero.
+  } else if (outcome == 1) {
+    // We should be seeing noinherit behavior.
+    tt_int_op(r, OP_LE, 0); // child said nothing; it should have crashed.
+  } else {
+    // noinherit isn't implemented.
+    tt_int_op(outcome, OP_EQ, 0);
+    tt_int_op((int)r, OP_EQ, 1); // child should send us a byte.
+    tt_int_op(buf[0], OP_EQ, 0xd0); // that byte should what we set it to.
+  }
+
   int ws;
   waitpid(child, &ws, 0);
 
+  if (outcome == 0) {
+    /* Call this test "skipped", not "passed", since noinherit wasn't
+     * implemented. */
+    tt_skip();
+  }
+
  done:
   tor_munmap_anonymous(ptr, sz);
   if (pipefd[0] >= 0) {
@@ -6360,6 +6376,6 @@ struct testcase_t util_tests[] = {
   UTIL_TEST(get_unquoted_path, 0),
   UTIL_TEST(log_mallinfo, 0),
   UTIL_TEST(map_anon, 0),
-  UTIL_TEST(map_anon_nofork, TT_SKIP /* See bug #29535 */),
+  UTIL_TEST(map_anon_nofork, 0),
   END_OF_TESTCASES
 };