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

diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index a820ebf0ac75637dd67d86ef4e67ee9a3d5ad78b..19e6a37f2c3bf7882f64e7589eb893c0bf5cea1d 100644 (file)
--- 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);