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 <tri...@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 <tri...@samba.org>
Autobuild-Date: Wed Jan 5 07:22:27 CET 2011 on sn-devel-104
commit 66db49e35f87f0e0a9d82cfa661d2cc604758406
Author: Andrew Tridgell <tri...@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
