Attachment 13901 Details for Bug 13210 – patch for master

[patch] patch for master

talloc-magic-abort-wrong.patch.txt (text/plain), 4.65 KB, created by Andrew Bartlett on 2018-01-11 22:21:55 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2018-01-11 22:21:55 UTC

Size: 4.65 KB

patch

obsolete

>From 689b11e15840ad2e36048cbc3026ca1fd7c4b19c Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>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 <abartlet@samba.org>
>---
> 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 <abartlet@samba.org>
>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 <abartlet@samba.org>
>---
> 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
>

Actions: View

Attachments on bug 13210: 13901