[SCM] Samba Shared Repository - branch master updated

[Andrew Tridgell]

The branch, master has been updated
       via  6f51a1f talloc: fixed a use after free error
       via  66db49e talloc: added a test for the use after free Rusty found
      from  26c8a52 upgradeprovision: Fix use of dict.get().

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 6f51a1f45bf4de062cce7a562477e8140630a53d
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

commit 66db49e35f87f0e0a9d82cfa661d2cc604758406
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

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

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


Changeset truncated at 500 lines:

diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index d6115af..c616f34 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -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