From 12205c3cbee4e71ded2b5710a57342b510e9d6df Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 6 Mar 2019 10:35:02 -0500 Subject: Make map_anon expose the result of a noinherit attempt Previously we did this for tests only, but it's valuable for getting proper fork behavior in rand_fast. --- src/test/test_util.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'src/test/test_util.c') diff --git a/src/test/test_util.c b/src/test/test_util.c index 4990aa709a..039fc435ce 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -6130,10 +6130,12 @@ test_util_map_anon(void *arg) (void)arg; char *ptr = NULL; size_t sz = 16384; + unsigned inherit=0; /* Basic checks. */ - ptr = tor_mmap_anonymous(sz, 0); + ptr = tor_mmap_anonymous(sz, 0, &inherit); tt_ptr_op(ptr, OP_NE, 0); + tt_int_op(inherit, OP_EQ, INHERIT_KEEP); ptr[sz-1] = 3; tt_int_op(ptr[0], OP_EQ, 0); tt_int_op(ptr[sz-2], OP_EQ, 0); @@ -6141,8 +6143,9 @@ test_util_map_anon(void *arg) /* Try again, with a private (non-swappable) mapping. */ tor_munmap_anonymous(ptr, sz); - ptr = tor_mmap_anonymous(sz, ANONMAP_PRIVATE); + ptr = tor_mmap_anonymous(sz, ANONMAP_PRIVATE, &inherit); tt_ptr_op(ptr, OP_NE, 0); + tt_int_op(inherit, OP_EQ, INHERIT_KEEP); ptr[sz-1] = 10; tt_int_op(ptr[0], OP_EQ, 0); tt_int_op(ptr[sz/2], OP_EQ, 0); @@ -6150,7 +6153,7 @@ test_util_map_anon(void *arg) /* Now let's test a drop-on-fork mapping. */ tor_munmap_anonymous(ptr, sz); - ptr = tor_mmap_anonymous(sz, ANONMAP_NOINHERIT); + ptr = tor_mmap_anonymous(sz, ANONMAP_NOINHERIT, &inherit); tt_ptr_op(ptr, OP_NE, 0); ptr[sz-1] = 10; tt_int_op(ptr[0], OP_EQ, 0); @@ -6179,10 +6182,10 @@ test_util_map_anon_nofork(void *arg) char *ptr = NULL; size_t sz = 16384; int pipefd[2] = {-1, -1}; + unsigned inherit=0; tor_munmap_anonymous(ptr, sz); - ptr = tor_mmap_anonymous(sz, ANONMAP_NOINHERIT); - int outcome = get_last_anon_map_noinherit(); + ptr = tor_mmap_anonymous(sz, ANONMAP_NOINHERIT, &inherit); tt_ptr_op(ptr, OP_NE, 0); memset(ptr, 0xd0, sz); @@ -6204,16 +6207,16 @@ test_util_map_anon_nofork(void *arg) char buf[1]; ssize_t r = read(pipefd[0], buf, 1); - if (outcome == 2) { + if (inherit == INHERIT_ZERO) { // 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) { + } else if (inherit == INHERIT_DROP) { // 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(inherit, OP_EQ, INHERIT_KEEP); 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. } @@ -6221,7 +6224,7 @@ test_util_map_anon_nofork(void *arg) int ws; waitpid(child, &ws, 0); - if (outcome == 0) { + if (inherit == INHERIT_KEEP) { /* Call this test "skipped", not "passed", since noinherit wasn't * implemented. */ tt_skip(); -- cgit v1.2.3-54-g00ecf From 361e955cf3f852caebb63f618fbae883237bf28b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 6 Mar 2019 11:03:26 -0500 Subject: map_anon: define a macro if it is possible for noinherit to fail. --- src/lib/malloc/map_anon.h | 21 +++++++++++++++++++++ src/test/test_util.c | 6 ++++++ 2 files changed, 27 insertions(+) (limited to 'src/test/test_util.c') diff --git a/src/lib/malloc/map_anon.h b/src/lib/malloc/map_anon.h index edd7500821..89fb9da0f0 100644 --- a/src/lib/malloc/map_anon.h +++ b/src/lib/malloc/map_anon.h @@ -41,6 +41,27 @@ * the child process. */ #define INHERIT_ZERO 2 +/* Here we define the NOINHERIT_CAN_FAIL macro if and only if + * it's possible that ANONMAP_NOINHERIT might yield inheritable memory. + */ +#ifdef _WIN32 +/* Windows can't fork, so NOINHERIT is never needed. */ +#elif defined(HAVE_MINHERIT) +/* minherit() will always have a working MAP_INHERIT_NONE or MAP_INHERIT_ZERO. + * NOINHERIT should always work. + */ +#elif defined(HAVE_MADVISE) +/* madvise() sometimes has neither MADV_DONTFORK and MADV_WIPEONFORK. + * We need to be ready for the possibility it failed. + * + * (Linux added DONTFORK in 2.6.16 and WIPEONFORK in 4.14. If we someday + * require 2.6.16 or later, we can assume that DONTFORK will work.) + */ +#define NOINHERIT_CAN_FAIL +#else +#define NOINHERIT_CAN_FAIL +#endif + void *tor_mmap_anonymous(size_t sz, unsigned flags, unsigned *inherit_result_out); void tor_munmap_anonymous(void *mapping, size_t sz); diff --git a/src/test/test_util.c b/src/test/test_util.c index 039fc435ce..f6085fdb90 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -6224,11 +6224,17 @@ test_util_map_anon_nofork(void *arg) int ws; waitpid(child, &ws, 0); +#ifndef NOINHERIT_CAN_FAIL + /* Only if NOINHERIT_CAN_FAIL should it be possible for us to get + * INHERIT_KEEP behavior in this case. */ + tt_assert(inherit, OP_NE, INHERIT_KEEP); +#else if (inherit == INHERIT_KEEP) { /* Call this test "skipped", not "passed", since noinherit wasn't * implemented. */ tt_skip(); } +#endif done: tor_munmap_anonymous(ptr, sz); -- cgit v1.2.3-54-g00ecf From 027c536598f336014d7ef406610207ec4559d490 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 6 Mar 2019 12:08:25 -0500 Subject: rename inherit values to avoid conflict with system defines --- src/lib/crypt_ops/crypto_rand_fast.c | 12 ++++++------ src/lib/malloc/map_anon.c | 12 ++++++------ src/lib/malloc/map_anon.h | 6 +++--- src/test/test_util.c | 14 +++++++------- 4 files changed, 22 insertions(+), 22 deletions(-) (limited to 'src/test/test_util.c') diff --git a/src/lib/crypt_ops/crypto_rand_fast.c b/src/lib/crypt_ops/crypto_rand_fast.c index 384c172c32..01817c618f 100644 --- a/src/lib/crypt_ops/crypto_rand_fast.c +++ b/src/lib/crypt_ops/crypto_rand_fast.c @@ -152,7 +152,7 @@ crypto_fast_rng_new(void) crypto_fast_rng_t * crypto_fast_rng_new_from_seed(const uint8_t *seed) { - unsigned inherit = INHERIT_KEEP; + unsigned inherit = INHERIT_RES_KEEP; /* We try to allocate this object as securely as we can, to avoid * having it get dumped, swapped, or shared after fork. */ @@ -164,7 +164,7 @@ crypto_fast_rng_new_from_seed(const uint8_t *seed) result->bytes_left = 0; result->n_till_reseed = RESEED_AFTER; #ifdef CHECK_PID - if (inherit == INHERIT_KEEP) { + if (inherit == INHERIT_RES_KEEP) { /* This value will neither be dropped nor zeroed after fork, so we need to * check our pid to make sure we are not sharing it across a fork. This * can be expensive if the pid value isn't cached, sadly. @@ -176,7 +176,7 @@ crypto_fast_rng_new_from_seed(const uint8_t *seed) #else /* We decided above that noinherit would always do _something_. Assert here * that we were correct. */ - tor_assert(inherit != INHERIT_KEEP); + tor_assert(inherit != INHERIT_RES_KEEP); #endif return result; } @@ -253,12 +253,12 @@ crypto_fast_rng_getbytes_impl(crypto_fast_rng_t *rng, uint8_t *out, if (rng->owner) { /* Note that we only need to do this check when we have owner set: that * is, when our attempt to block inheriting failed, and the result was - * INHERIT_KEEP. + * INHERIT_RES_KEEP. * - * If the result was INHERIT_DROP, then any attempt to access the rng + * If the result was INHERIT_RES_DROP, then any attempt to access the rng * memory after forking will crash. * - * If the result was INHERIT_ZERO, then forking will set the bytes_left + * If the result was INHERIT_RES_ZERO, then forking will set the bytes_left * and n_till_reseed fields to zero. This function will call * crypto_fast_rng_refill(), which will in turn reseed the PRNG. * diff --git a/src/lib/malloc/map_anon.c b/src/lib/malloc/map_anon.c index bb0a3d5741..3e1c22648d 100644 --- a/src/lib/malloc/map_anon.c +++ b/src/lib/malloc/map_anon.c @@ -123,14 +123,14 @@ noinherit_mem(void *mem, size_t sz, unsigned *inherit_result_out) #ifdef FLAG_ZERO int r = MINHERIT(mem, sz, FLAG_ZERO); if (r == 0) { - *inherit_result_out = INHERIT_ZERO; + *inherit_result_out = INHERIT_RES_ZERO; return 0; } #endif #ifdef FLAG_NOINHERIT int r2 = MINHERIT(mem, sz, FLAG_NOINHERIT); if (r2 == 0) { - *inherit_result_out = INHERIT_DROP; + *inherit_result_out = INHERIT_RES_DROP; } return r2; #else @@ -154,9 +154,9 @@ noinherit_mem(void *mem, size_t sz, unsigned *inherit_result_out) * Memory returned from this function must be released with * tor_munmap_anonymous(). * - * If inherit_result_out is non-NULL, set it to one of INHERIT_KEEP, - * INHERIT_DROP, and INHERIT_ZERO, depending on the properties of the returned - * memory. + * If inherit_result_out is non-NULL, set it to one of + * INHERIT_RES_KEEP, INHERIT_RES_DROP, or INHERIT_RES_ZERO, depending on the + * properties of the returned memory. * * [Note: OS people use the word "anonymous" here to mean that the memory * isn't associated with any file. This has *nothing* to do with the kind of @@ -170,7 +170,7 @@ tor_mmap_anonymous(size_t sz, unsigned flags, unsigned *inherit_result_out) if (inherit_result_out == NULL) { inherit_result_out = &itmp; } - *inherit_result_out = INHERIT_KEEP; + *inherit_result_out = INHERIT_RES_KEEP; #if defined(_WIN32) HANDLE mapping = CreateFileMapping(INVALID_HANDLE_VALUE, diff --git a/src/lib/malloc/map_anon.h b/src/lib/malloc/map_anon.h index 89fb9da0f0..f101654eb8 100644 --- a/src/lib/malloc/map_anon.h +++ b/src/lib/malloc/map_anon.h @@ -33,13 +33,13 @@ /** Possible value for inherit_result_out: the memory will be kept * by any child process. */ -#define INHERIT_KEEP 0 +#define INHERIT_RES_KEEP 0 /** Possible value for inherit_result_out: the memory will be dropped in * the child process. Attempting to access it will likely cause a segfault. */ -#define INHERIT_DROP 1 +#define INHERIT_RES_DROP 1 /** Possible value for inherit_result_out: the memory will be cleared in * the child process. */ -#define INHERIT_ZERO 2 +#define INHERIT_RES_ZERO 2 /* Here we define the NOINHERIT_CAN_FAIL macro if and only if * it's possible that ANONMAP_NOINHERIT might yield inheritable memory. diff --git a/src/test/test_util.c b/src/test/test_util.c index f6085fdb90..4f81e54d2b 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -6135,7 +6135,7 @@ test_util_map_anon(void *arg) /* Basic checks. */ ptr = tor_mmap_anonymous(sz, 0, &inherit); tt_ptr_op(ptr, OP_NE, 0); - tt_int_op(inherit, OP_EQ, INHERIT_KEEP); + tt_int_op(inherit, OP_EQ, INHERIT_RES_KEEP); ptr[sz-1] = 3; tt_int_op(ptr[0], OP_EQ, 0); tt_int_op(ptr[sz-2], OP_EQ, 0); @@ -6145,7 +6145,7 @@ test_util_map_anon(void *arg) tor_munmap_anonymous(ptr, sz); ptr = tor_mmap_anonymous(sz, ANONMAP_PRIVATE, &inherit); tt_ptr_op(ptr, OP_NE, 0); - tt_int_op(inherit, OP_EQ, INHERIT_KEEP); + tt_int_op(inherit, OP_EQ, INHERIT_RES_KEEP); ptr[sz-1] = 10; tt_int_op(ptr[0], OP_EQ, 0); tt_int_op(ptr[sz/2], OP_EQ, 0); @@ -6207,16 +6207,16 @@ test_util_map_anon_nofork(void *arg) char buf[1]; ssize_t r = read(pipefd[0], buf, 1); - if (inherit == INHERIT_ZERO) { + if (inherit == INHERIT_RES_ZERO) { // 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 (inherit == INHERIT_DROP) { + } else if (inherit == INHERIT_RES_DROP) { // 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(inherit, OP_EQ, INHERIT_KEEP); + tt_int_op(inherit, OP_EQ, INHERIT_RES_KEEP); 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. } @@ -6227,9 +6227,9 @@ test_util_map_anon_nofork(void *arg) #ifndef NOINHERIT_CAN_FAIL /* Only if NOINHERIT_CAN_FAIL should it be possible for us to get * INHERIT_KEEP behavior in this case. */ - tt_assert(inherit, OP_NE, INHERIT_KEEP); + tt_assert(inherit, OP_NE, INHERIT_RES_KEEP); #else - if (inherit == INHERIT_KEEP) { + if (inherit == INHERIT_RES_KEEP) { /* Call this test "skipped", not "passed", since noinherit wasn't * implemented. */ tt_skip(); -- cgit v1.2.3-54-g00ecf From 8c06f02c94067d7ad1bf39982e7238b7fafae0e0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 6 Mar 2019 12:09:46 -0500 Subject: Syntax fix in test. --- src/test/test_util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/test/test_util.c') diff --git a/src/test/test_util.c b/src/test/test_util.c index 4f81e54d2b..c683c9475b 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -6227,7 +6227,7 @@ test_util_map_anon_nofork(void *arg) #ifndef NOINHERIT_CAN_FAIL /* Only if NOINHERIT_CAN_FAIL should it be possible for us to get * INHERIT_KEEP behavior in this case. */ - tt_assert(inherit, OP_NE, INHERIT_RES_KEEP); + tt_int_op(inherit, OP_NE, INHERIT_RES_KEEP); #else if (inherit == INHERIT_RES_KEEP) { /* Call this test "skipped", not "passed", since noinherit wasn't -- cgit v1.2.3-54-g00ecf