[SCM] Samba Shared Repository - branch v3-6-test updated

[Stefan Metzmacher]

The branch, v3-6-test has been updated
       via  e47ced6 talloc: fixed a use after free error
       via  1d08998 talloc: added a test for the use after free Rusty found (cherry picked from commit 66db49e35f87f0e0a9d82cfa661d2cc604758406)
       via  59f1fbc talloc: Clarify error message on access after free. (cherry picked from commit 733bc1c1ca31df2b61d86f5ee8783ee9c3867faa)
      from  2bd7650 s3-rpcecho: Only register rpcecho in the developer build.

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-6-test


- Log -----------------------------------------------------------------
commit e47ced633c905756d2651ad41d682e217765f3fc
Author: Andrew Tridgell <tridge at samba.org>
Date:   Wed Jan 5 16:33:13 2011 +1100

    talloc: fixed a use after free error
    
    this is the minimal fix for the problem Rusty found. I previously
    thought that the best fix would be to change tc->parent to be valid
    for all pointers, but that is expensive for realloc with large numbers
    of child pointers, which is much more commmon than I expected it to
    be.
    
    Autobuild-User: Andrew Tridgell <tridge at samba.org>
    Autobuild-Date: Wed Jan  5 07:22:27 CET 2011 on sn-devel-104
    (cherry picked from commit 6f51a1f45bf4de062cce7a562477e8140630a53d)
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>

commit 1d08998db859422e5995ed3e7b74ca5402057775
Author: Andrew Tridgell <tridge at samba.org>
Date:   Wed Dec 22 15:29:37 2010 +1100

    talloc: added a test for the use after free Rusty found
    (cherry picked from commit 66db49e35f87f0e0a9d82cfa661d2cc604758406)
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>

commit 59f1fbc6b99bf3e3a9ea9d61aa35a35c82d93f96
Author: Jelmer Vernooij <jelmer at samba.org>
Date:   Wed Jan 5 01:01:28 2011 +0100

    talloc: Clarify error message on access after free.
    (cherry picked from commit 733bc1c1ca31df2b61d86f5ee8783ee9c3867faa)
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 lib/talloc/talloc.c    |   25 ++++++++++++++++++++-----
 lib/talloc/testsuite.c |   17 +++++++++++++++++
 2 files changed, 37 insertions(+), 5 deletions(-)


Changeset truncated at 500 lines:

diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index ec67a46..c616f34 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -224,9 +224,9 @@ static void talloc_abort_magic(unsigned magic)
 	talloc_abort("Bad talloc magic value - wrong talloc version used/mixed");
 }
 
-static void talloc_abort_double_free(void)
+static void talloc_abort_access_after_free(void)
 {
-	talloc_abort("Bad talloc magic value - double free");
+	talloc_abort("Bad talloc magic value - access after free");
 }
 
 static void talloc_abort_unknown_value(void)
@@ -246,8 +246,8 @@ static inline struct talloc_chunk *talloc_chunk_from_ptr(const void *ptr)
 		}
 
 		if (tc->flags & TALLOC_FLAG_FREE) {
-			talloc_log("talloc: double free error - first free may be at %s\n", tc->name);
-			talloc_abort_double_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 {
 			talloc_abort_unknown_value();
@@ -645,13 +645,28 @@ static inline int _talloc_free_internal(void *ptr, const char *location)
 		   final choice is the null context. */
 		void *child = TC_PTR_FROM_CHUNK(tc->child);
 		const void *new_parent = null_context;
+		struct talloc_chunk *old_parent = NULL;
 		if (unlikely(tc->child->refs)) {
 			struct talloc_chunk *p = talloc_parent_chunk(tc->child->refs);
 			if (p) new_parent = TC_PTR_FROM_CHUNK(p);
 		}
+		/* finding the parent here is potentially quite
+		   expensive, but the alternative, which is to change
+		   talloc to always have a valid tc->parent pointer,
+		   makes realloc more expensive where there are a
+		   large number of children.
+
+		   The reason we need the parent pointer here is that
+		   if _talloc_free_internal() fails due to references
+		   or a failing destructor we need to re-parent, but
+		   the free call can invalidate the prev pointer.
+		*/
+		if (new_parent == null_context && (tc->child->refs || tc->child->destructor)) {
+			old_parent = talloc_parent_chunk(ptr);
+		}
 		if (unlikely(_talloc_free_internal(child, location) == -1)) {
 			if (new_parent == null_context) {
-				struct talloc_chunk *p = talloc_parent_chunk(ptr);
+				struct talloc_chunk *p = old_parent;
 				if (p) new_parent = TC_PTR_FROM_CHUNK(p);
 			}
 			_talloc_steal_internal(new_parent, child);
diff --git a/lib/talloc/testsuite.c b/lib/talloc/testsuite.c
index 0026931..ee6256b 100644
--- a/lib/talloc/testsuite.c
+++ b/lib/talloc/testsuite.c
@@ -1167,6 +1167,21 @@ static bool test_free_ref_null_context(void)
 	return true;
 }
 
+static bool test_rusty(void)
+{
+	void *root;
+	const char *p1;
+
+	talloc_enable_null_tracking();
+	root = talloc_new(NULL);
+	p1 = talloc_strdup(root, "foo");
+	talloc_increase_ref_count(p1);
+	talloc_report_full(root, stdout);
+	talloc_free(root);
+	return true;
+}
+
+
 static void test_reset(void)
 {
 	talloc_set_log_fn(test_log_stdout);
@@ -1221,6 +1236,8 @@ bool torture_local_talloc(struct torture_context *tctx)
 	ret &= test_pool();
 	test_reset();
 	ret &= test_free_ref_null_context();
+	test_reset();
+	ret &= test_rusty();
 
 	if (ret) {
 		test_reset();


-- 
Samba Shared Repository