talloc & threads in rlm_eap

[Arran Cudbard-Bell]

On 20 Jun 2014, at 10:56, Phil Mayers <p.mayers at 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/f74b0fe85f171faf5c1445240d23a28eb73f20c1

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 at freeradius.org>
FreeRADIUS Development Team

FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 881 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.freeradius.org/pipermail/freeradius-devel/attachments/20140620/8e9f502f/attachment.pgp>