summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndrew Tridgell <tridge@samba.org>2011-01-05 16:33:13 +1100
committerAndrew Tridgell <tridge@samba.org>2011-01-05 07:22:27 +0100
commit6f51a1f45bf4de062cce7a562477e8140630a53d (patch)
tree5ae6fac7e4f91e129740661470110e2930b49c9a
parent66db49e35f87f0e0a9d82cfa661d2cc604758406 (diff)
downloadsamba-6f51a1f45bf4de062cce7a562477e8140630a53d.tar.gz
samba-6f51a1f45bf4de062cce7a562477e8140630a53d.tar.bz2
samba-6f51a1f45bf4de062cce7a562477e8140630a53d.zip
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@samba.org> Autobuild-Date: Wed Jan 5 07:22:27 CET 2011 on sn-devel-104
-rw-r--r--lib/talloc/talloc.c17
1 files changed, 16 insertions, 1 deletions
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index d6115af529..c616f3458c 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);