From 689b11e15840ad2e36048cbc3026ca1fd7c4b19c Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 8 Jan 2018 17:29:19 +1300 Subject: [PATCH 1/2] talloc: Remove talloc_abort_magic() The check required for talloc_abort_magic() prevents the 'access after free error' from being printed. It is also no longer possible to determine the difference between invalid memory and a talloc version mismatch as the magic is now random on many platforms. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13210 Signed-off-by: Andrew Bartlett --- lib/talloc/talloc.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c index 7721fa4a9c6..3569ba75929 100644 --- a/lib/talloc/talloc.c +++ b/lib/talloc/talloc.c @@ -429,11 +429,6 @@ static void talloc_abort(const char *reason) talloc_abort_fn(reason); } -static void talloc_abort_magic(unsigned magic) -{ - talloc_abort("Bad talloc magic value - wrong talloc version used/mixed"); -} - static void talloc_abort_access_after_free(void) { talloc_abort("Bad talloc magic value - access after free"); @@ -450,19 +445,14 @@ static inline struct talloc_chunk *talloc_chunk_from_ptr(const void *ptr) const char *pp = (const char *)ptr; struct talloc_chunk *tc = discard_const_p(struct talloc_chunk, pp - TC_HDR_SIZE); if (unlikely((tc->flags & (TALLOC_FLAG_FREE | ~TALLOC_FLAG_MASK)) != talloc_magic)) { - if ((tc->flags & (~TALLOC_FLAG_MASK)) == talloc_magic) { - talloc_abort_magic(tc->flags & (~TALLOC_FLAG_MASK)); - return NULL; - } - - if (tc->flags & TALLOC_FLAG_FREE) { - talloc_log("talloc: access after free error - first free may be at %s\n", tc->name); - talloc_abort_access_after_free(); - return NULL; - } else { + if ((tc->flags & (~TALLOC_FLAG_MASK)) != talloc_magic) { talloc_abort_unknown_value(); return NULL; } + + talloc_log("talloc: access after free error - first free may be at %s\n", tc->name); + talloc_abort_access_after_free(); + return NULL; } return tc; } -- 2.11.0 From dafbed2f7a01818c5aeb796fd2e251745c5f1fb6 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 12 Jan 2018 11:17:09 +1300 Subject: [PATCH 2/2] talloc: Add tests to require use-after-free to give the correct talloc_abort() string BUG: https://bugzilla.samba.org/show_bug.cgi?id=13210 Signed-off-by: Andrew Bartlett --- lib/talloc/testsuite.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/lib/talloc/testsuite.c b/lib/talloc/testsuite.c index dfaeec1d1d9..92081b5fdd8 100644 --- a/lib/talloc/testsuite.c +++ b/lib/talloc/testsuite.c @@ -2006,6 +2006,72 @@ static bool test_magic_protection(void) return true; } +static void test_magic_free_protection_abort(const char *reason) +{ + /* exit with errcode 42 to communicate successful test to the parent process */ + if (strcmp(reason, "Bad talloc magic value - access after free") == 0) { + _exit(42); + } + /* not 42 */ + _exit(404); +} + +static bool test_magic_free_protection(void) +{ + void *pool = talloc_pool(NULL, 1024); + int *p1, *p2, *p3; + pid_t pid; + int exit_status; + + printf("test: magic_protection\n"); + p1 = talloc(pool, int); + p2 = talloc(pool, int); + + /* To avoid complaints from the compiler assign values to the p1 & p2. */ + *p1 = 6; + *p2 = 9; + + p3 = talloc_realloc(pool, p2, int, 2048); + torture_assert("pool realloc 2048", + p3 != p2, + "failed: pointer not changed"); + + /* + * Now access the memory in the pool after the realloc(). It + * should be marked as free, so use of the old pointer should + * trigger the abort function + */ + pid = fork(); + if (pid == 0) { + talloc_set_abort_fn(test_magic_free_protection_abort); + + talloc_get_name(p2); + + /* Never reached. Make compilers happy */ + return true; + } + + while (wait(&exit_status) != pid); + + if (!WIFEXITED(exit_status)) { + printf("Child exited through unexpected abnormal means\n"); + return false; + } + if (WEXITSTATUS(exit_status) != 42) { + printf("Child exited with wrong exit status\n"); + return false; + } + if (WIFSIGNALED(exit_status)) { + printf("Child recieved unexpected signal\n"); + return false; + } + + talloc_free(pool); + + printf("success: magic_free_protection\n"); + return true; +} + static void test_reset(void) { talloc_set_log_fn(test_log_stdout); @@ -2092,6 +2158,8 @@ bool torture_local_talloc(struct torture_context *tctx) ret &= test_autofree(); test_reset(); ret &= test_magic_protection(); + test_reset(); + ret &= test_magic_free_protection(); test_reset(); talloc_disable_null_tracking(); -- 2.11.0