diff options
author | Andrew Tridgell <tridge@samba.org> | 2011-01-05 16:33:13 +1100 |
---|---|---|
committer | Andrew Tridgell <tridge@samba.org> | 2011-01-05 07:22:27 +0100 |
commit | 6f51a1f45bf4de062cce7a562477e8140630a53d (patch) | |
tree | 5ae6fac7e4f91e129740661470110e2930b49c9a /lib/talloc | |
parent | 66db49e35f87f0e0a9d82cfa661d2cc604758406 (diff) | |
download | samba-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
Diffstat (limited to 'lib/talloc')
-rw-r--r-- | lib/talloc/talloc.c | 17 |
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); |