BUG: talloc reads freed memory.
Rusty Russell
rusty at rustcorp.com.au
Mon Dec 20 20:26:35 MST 2010
Not sure how to fix this one; valgrind with my new failtest module
caught it in CCAN:
talloc_enable_null_tracking();
root = talloc_new(NULL);
p1 = talloc_strdup(root, "foo");
talloc_increase_ref_count(p1);
talloc_free(root);
The problem is here in _talloc_free_internal:
if (tc->parent) {
_TLIST_REMOVE(tc->parent->child, tc);
if (tc->parent->child) {
tc->parent->child->parent = tc->parent;
}
} else {
if (tc->prev) tc->prev->next = tc->next;
if (tc->next) tc->next->prev = tc->prev;
}
Note that we leave tc->prev set, even though we've removed ourselves
from the list.
tc->flags |= TALLOC_FLAG_LOOP;
while (tc->child) {
/* we need to work out who will own an abandoned child
if it cannot be freed. In priority order, the first
choice is owner of any remaining reference to this
pointer, the second choice is our parent, and the
final choice is the null context. */
void *child = TC_PTR_FROM_CHUNK(tc->child);
const void *new_parent = null_context;
if (unlikely(tc->child->refs)) {
struct talloc_chunk *p = talloc_parent_chunk(tc->child->refs);
if (p) new_parent = TC_PTR_FROM_CHUNK(p);
}
if (unlikely(_talloc_free_internal(child, location) == -1)) {
The child frees the reference, which is our sibling (ie. tc->prev).
if (new_parent == null_context) {
struct talloc_chunk *p = talloc_parent_chunk(ptr);
talloc_parent_chunk follows tc->prev through the (freed) reference.
if (p) new_parent = TC_PTR_FROM_CHUNK(p);
}
_talloc_steal_internal(new_parent, child);
}
}
Now, simply clearing tc->prev (and tc->next) just breaks this reparenting
logic. Getting the parent earlier in case the destructor fails is
O(siblings) and we'd be better off just keeping all the parent pointers
uptodate (ie. make talloc_steal() O(siblings)).
Thoughts?
Rusty.
PS. I haven't actually verified this with the current SAMBA talloc, but
the code is basically unchanged.
More information about the samba-technical
mailing list