On 20 Jun 2014, at 10:56, Phil Mayers <p.mayers@imperial.ac.uk> wrote:
Hmm assuming nothing else is talloc'ed off the parent of handler that is, or freeing handler itself is racey...
That's now fixed too. I think. The memory associated with the handler is now freed within the destructor for the handler, which synchronises the frees using the same mutex. Hopefully that's OK. https://github.com/FreeRADIUS/freeradius-server/commit/f74b0fe85f171faf5c144... If it explodes I guess we'll need to figure out another solution.
I'm assuming talloc lets you alloc and free below a context in one thread whilst alloc and freeing off a sibling, ancestor or descendant context in another?
Looks like that's safe. There's nothing that modifies anything above the context of the chunk. /* Allocate a bit of memory as a child of an existing pointer */ static inline void *__talloc_with_prefix(const void *context, size_t size, size_t prefix_len) { struct talloc_chunk *tc = NULL; struct talloc_memlimit *limit = NULL; size_t total_len = TC_HDR_SIZE + size + prefix_len; if (unlikely(context == NULL)) { context = null_context; } if (unlikely(size >= MAX_TALLOC_SIZE)) { return NULL; } if (unlikely(total_len < TC_HDR_SIZE)) { return NULL; } if (context != NULL) { struct talloc_chunk *ptc = talloc_chunk_from_ptr(context); if (ptc->limit != NULL) { limit = ptc->limit; } tc = talloc_alloc_pool(ptc, TC_HDR_SIZE+size, prefix_len); } if (tc == NULL) { char *ptr; /* * Only do the memlimit check/update on actual allocation. */ if (!talloc_memlimit_check(limit, total_len)) { errno = ENOMEM; return NULL; } ptr = malloc(total_len); if (unlikely(ptr == NULL)) { return NULL; } tc = (struct talloc_chunk *)(ptr + prefix_len); tc->flags = TALLOC_MAGIC; tc->pool = NULL; talloc_memlimit_grow(limit, total_len); } tc->limit = limit; tc->size = size; tc->destructor = NULL; tc->child = NULL; tc->name = NULL; tc->refs = NULL; if (likely(context)) { struct talloc_chunk *parent = talloc_chunk_from_ptr(context); if (parent->child) { parent->child->parent = NULL; tc->next = parent->child; tc->next->prev = tc; } else { tc->next = NULL; } tc->parent = parent; tc->prev = NULL; parent->child = tc; } else { tc->next = tc->prev = tc->parent = NULL; } return TC_PTR_FROM_CHUNK(tc); } static inline void *__talloc(const void *context, size_t size) { return __talloc_with_prefix(context, size, 0); } Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS Development Team FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2