commit 065e7da8e6fdbd9331de8c13344275a8e0fbf32d Author: Nick Mathewson nickm@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 };