>From e27117d3646efaa95c5d1714c19f86262544a802 Mon Sep 17 00:00:00 2001 From: eindenbom Date: Mon, 19 Apr 2010 15:05:06 +0400 Subject: [PATCH 1/4] Avoid freeing sdap_handle too early When disconnecting LDAP and calling op callbacks protect sdap_handle from talloc_free by returning -1 from destructor. Free orphan sdap_handle as soon as we are done with calling op callbacks. --- src/providers/ldap/sdap.h | 11 +++ src/providers/ldap/sdap_async.c | 98 ++++++++++++++++++++-------- src/providers/ldap/sdap_async_connection.c | 3 + 3 files changed, 84 insertions(+), 28 deletions(-) diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index f0e345e..a34848f 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h @@ -78,6 +78,14 @@ struct sdap_handle { #endif struct sdap_op *ops; + + /* indicates that handle is being disconnected and + * operation callbacks are being called */ + bool being_disconnected; + /* indicates that handle destruction was started by talloc_free + * but deferred by sdap_handle_destructor as handle was busy + * calling operation callbacks */ + bool orphan; }; struct sdap_service { @@ -230,6 +238,9 @@ struct sdap_options { struct ldb_dn *groups_base; }; +/* Disconnect sdap_handle and abort all active operations with EIO error code */ +void sdap_handle_disconnect(struct sdap_handle* sh); + int sdap_get_map(TALLOC_CTX *memctx, struct confdb_ctx *cdb, const char *conf_path, diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c index 15f3048..49920a8 100644 --- a/src/providers/ldap/sdap_async.c +++ b/src/providers/ldap/sdap_async.c @@ -63,10 +63,11 @@ static int sdap_msg_attach(TALLOC_CTX *memctx, LDAPMessage *msg) return EOK; } -/* ==sdap-hanlde-utility-functions======================================== */ +/* ==sdap-handle-utility-functions======================================== */ static inline void sdap_handle_release(struct sdap_handle *sh); static int sdap_handle_destructor(void *mem); +static int sdap_op_destructor(struct sdap_op *op); struct sdap_handle *sdap_handle_create(TALLOC_CTX *memctx) { @@ -84,44 +85,82 @@ static int sdap_handle_destructor(void *mem) { struct sdap_handle *sh = talloc_get_type(mem, struct sdap_handle); + /* if the handle is currently being disconnected, + * then mark it to be released and prevent talloc from freeing the memory */ + if (sh->being_disconnected) { + DEBUG(8, ("Trace: sh[%p], connected[%d], ops[%p], ldap[%p], " + "in disconnect[%d], orphan[%d]\n", + sh, (int)sh->connected, sh->ops, sh->ldap, + (int)sh->being_disconnected, (int)sh->orphan)); + + sh->orphan = true; + return -1; + } + sdap_handle_release(sh); - return 0; } static void sdap_handle_release(struct sdap_handle *sh) { - DEBUG(8, ("Trace: sh[%p], connected[%d], ops[%p], ldap[%p]\n", - sh, (int)sh->connected, sh->ops, sh->ldap)); + struct sdap_op *op; - if (sh->connected) { - struct sdap_op *op; + DEBUG(8, ("Trace: sh[%p], connected[%d], ops[%p], ldap[%p], " + "in disconnect[%d], orphan[%d]\n", + sh, (int)sh->connected, sh->ops, sh->ldap, + (int)sh->being_disconnected, (int)sh->orphan)); + + if (sh->being_disconnected) { + return; + } + sh->being_disconnected = true; + + /* make sure nobody tries to reuse this connection from now on */ + sh->connected = false; #ifdef HAVE_LDAP_CONNCB + if (sh->conncb) { + if (sh->ldap != NULL) { + ldap_get_option(sh->ldap, LDAP_OPT_CONNECT_CB, sh->conncb); + } /* remove all related fd events from the event loop */ - talloc_zfree(sh->conncb->lc_arg); -#else - talloc_zfree(sh->fde); -#endif - - while (sh->ops) { - op = sh->ops; - op->callback(op, NULL, EIO, op->data); - /* calling the callback may result in freeing the op */ - /* check if it is still the same or avoid freeing */ - if (op == sh->ops) talloc_free(op); - } - - if (sh->ldap) { - ldap_unbind_ext(sh->ldap, NULL, NULL); - } -#ifdef HAVE_LDAP_CONNCB talloc_zfree(sh->conncb); + } +#else + talloc_zfree(sh->fde); #endif - sh->connected = false; + + while (sh->ops) { + op = sh->ops; + /* early call op destructor to release LDAP memory and detach operation + * from sdap_handle */ + sdap_op_destructor(op); + op->sh = NULL; + + op->callback(op, NULL, EIO, op->data); + } + + if (sh->ldap) { + ldap_unbind_ext(sh->ldap, NULL, NULL); sh->ldap = NULL; - sh->ops = NULL; } + + /* ok, we have done the job, unlock now */ + sh->being_disconnected = false; + + /* finally if a destructor was ever called, free sh before + * exiting */ + if (sh->orphan) { + /* neutralize the destructor as we already handled + * all was needed to be released */ + talloc_set_destructor(sh, NULL); + talloc_free(sh); + } +} + +void sdap_handle_disconnect(struct sdap_handle* sh) +{ + sdap_handle_release(sh); } /* ==Parse-Results-And-Handle-Disconnections============================== */ @@ -450,9 +489,12 @@ int sdap_install_ldap_callbacks(struct sdap_handle *sh, /* ==LDAP-Operations-Helpers============================================== */ -static int sdap_op_destructor(void *mem) +static int sdap_op_destructor(struct sdap_op *op) { - struct sdap_op *op = (struct sdap_op *)mem; + if( op->sh == NULL) { + /* already detached from handle, nothing to do */ + return 0; + } DLIST_REMOVE(op->sh->ops, op); @@ -516,7 +558,7 @@ int sdap_op_add(TALLOC_CTX *memctx, struct tevent_context *ev, DLIST_ADD(sh->ops, op); - talloc_set_destructor((TALLOC_CTX *)op, sdap_op_destructor); + talloc_set_destructor(op, sdap_op_destructor); *_op = op; return EOK; diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c index 2acbbb8..f202b70 100644 --- a/src/providers/ldap/sdap_async_connection.c +++ b/src/providers/ldap/sdap_async_connection.c @@ -1157,6 +1157,9 @@ int sdap_cli_connect_recv(struct tevent_req *req, } if (gsh) { + if (*gsh) { + talloc_zfree(*gsh); + } *gsh = talloc_steal(memctx, state->sh); if (!*gsh) { return ENOMEM; -- 1.6.6.1