From 5d28670d574368e87dd5f9c309134b7d62055d0e Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 8 Aug 2011 18:24:32 +1000 Subject: [PATCH] talloc: ensure the sibling linked list remains valid during a free This ensures that the sibling list of a pointer doesn't become invalid during a free operation. It is an alternative fix to the fix in 6f51a1f45bf4de062cce7a562477e8140630a53d, and avoids the problem of trying to calculate the parent pointer early This should fix the subtle spoolss talloc bug that Simo found Autobuild-User: Andrew Tridgell Autobuild-Date: Tue Aug 9 01:53:17 CEST 2011 on sn-devel-104 (cherry picked from commit cf986f200804ce873b43c1ecf2d5e1bd08eb8a25) (cherry picked from commit 07554082cc9d286ca0628179c9e7f7a493016a57) --- lib/talloc/talloc.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c index 4700aa99e8c..a0217d14e93 100644 --- a/lib/talloc/talloc.c +++ b/lib/talloc/talloc.c @@ -838,6 +838,7 @@ static inline int _talloc_free_internal(void *ptr, const char *location) } else { if (tc->prev) tc->prev->next = tc->next; if (tc->next) tc->next->prev = tc->prev; + tc->prev = tc->next = NULL; } tc->flags |= TALLOC_FLAG_LOOP; @@ -925,6 +926,7 @@ static void *_talloc_steal_internal(const void *new_ctx, const void *ptr) } else { if (tc->prev) tc->prev->next = tc->next; if (tc->next) tc->next->prev = tc->prev; + tc->prev = tc->next = NULL; } tc->parent = new_tc; @@ -1251,23 +1253,9 @@ static inline void _talloc_free_children_internal(struct talloc_chunk *tc, 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 = old_parent; + struct talloc_chunk *p = talloc_parent_chunk(ptr); if (p) new_parent = TC_PTR_FROM_CHUNK(p); } _talloc_steal_internal(new_parent, child); -- 2.11.4.GIT