Better handle sdap_handle memory from callers.

Always just mark the sdap_handle as not connected and let later _send()
functions to take care of freeing the handle before reconnecting.

Introduce restart functions to avoid calling _send() functions in _done()
functions error paths as this would have the same effect as directly freeing
the sdap_handle and cause access to freed memory in sdap_handle_release()

By freeing sdap_handle only in the connection _recv() function we
guarantee it can never be done within sdap_handle_release() but only
in a following event.

4 files changed, 139 insertions, 33 deletions

diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index b5765c27..90ec7e2e 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -359,7 +359,7 @@ void sdap_mark_offline(struct sdap_id_ctx *ctx)
     if (ctx->gsh) {
         /* make sure we mark the connection as gone when we go offline so that
          * we do not try to reuse a bad connection by mistale later */
-        talloc_zfree(ctx->gsh);
+        ctx->gsh->connected = false;
     }
 
     be_mark_offline(ctx->be);
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index 15e2f30a..a2217da6 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -99,8 +99,6 @@ struct tevent_req *users_get_send(TALLOC_CTX *memctx,
 
     if (!sdap_connected(ctx)) {
 
-        if (ctx->gsh) talloc_zfree(ctx->gsh);
-
         /* FIXME: add option to decide if tls should be used
          * or SASL/GSSAPI, etc ... */
         subreq = sdap_cli_connect_send(state, ev, ctx->opts,
@@ -299,8 +297,6 @@ struct tevent_req *groups_get_send(TALLOC_CTX *memctx,
 
     if (!sdap_connected(ctx)) {
 
-        if (ctx->gsh) talloc_zfree(ctx->gsh);
-
         /* FIXME: add option to decide if tls should be used
          * or SASL/GSSAPI, etc ... */
         subreq = sdap_cli_connect_send(state, ev, ctx->opts,
@@ -465,8 +461,6 @@ static struct tevent_req *groups_by_user_send(TALLOC_CTX *memctx,
 
     if (!sdap_connected(ctx)) {
 
-        if (ctx->gsh) talloc_zfree(ctx->gsh);
-
         /* FIXME: add option to decide if tls should be used
          * or SASL/GSSAPI, etc ... */
         subreq = sdap_cli_connect_send(state, ev, ctx->opts,
@@ -654,13 +648,43 @@ void sdap_account_info_handler(struct be_req *breq)
     if (ret != EOK) return sdap_handler_done(breq, DP_ERR_FATAL, ret, err);
 }
 
+static void sdap_account_info_immediate(struct tevent_context *ctx,
+                                        struct tevent_immediate *im,
+                                        void *private_data)
+{
+    struct be_req *breq = talloc_get_type(private_data, struct be_req);
+
+    sdap_account_info_handler(breq);
+}
+
+static int sdap_account_info_restart(struct be_req *breq)
+{
+    struct tevent_immediate *im;
+
+    breq->restarts++;
+    if (breq->restarts > MAX_BE_REQ_RESTARTS) {
+        return ELOOP;
+    }
+
+    im = tevent_create_immediate(breq);
+    if (!im) {
+        return ENOMEM;
+    }
+
+    /* schedule a completely new event to avoid deep recursions */
+    tevent_schedule_immediate(im, breq->be_ctx->ev,
+                              sdap_account_info_immediate, breq);
+
+    return EOK;
+}
+
 static void sdap_account_info_users_done(struct tevent_req *req)
 {
     struct be_req *breq = tevent_req_callback_data(req, struct be_req);
     struct sdap_id_ctx *ctx;
     int dp_err = DP_ERR_OK;
     const char *error = NULL;
-    int ret;
+    int ret, err;
 
     ret = users_get_recv(req);
     talloc_zfree(req);
@@ -674,9 +698,9 @@ static void sdap_account_info_users_done(struct tevent_req *req)
             ctx = talloc_get_type(breq->be_ctx->bet_info[BET_ID].pvt_bet_data,
                                   struct sdap_id_ctx);
             if (sdap_check_gssapi_reconnect(ctx)) {
-                talloc_zfree(ctx->gsh);
-                sdap_account_info_handler(breq);
-                return;
+                ctx->gsh->connected = false;
+                err = sdap_account_info_restart(breq);
+                if (err == EOK) return;
             }
             sdap_mark_offline(ctx);
         }
@@ -691,7 +715,7 @@ static void sdap_account_info_groups_done(struct tevent_req *req)
     struct sdap_id_ctx *ctx;
     int dp_err = DP_ERR_OK;
     const char *error = NULL;
-    int ret;
+    int ret, err;
 
     ret = groups_get_recv(req);
     talloc_zfree(req);
@@ -705,9 +729,9 @@ static void sdap_account_info_groups_done(struct tevent_req *req)
             ctx = talloc_get_type(breq->be_ctx->bet_info[BET_ID].pvt_bet_data,
                                   struct sdap_id_ctx);
             if (sdap_check_gssapi_reconnect(ctx)) {
-                talloc_zfree(ctx->gsh);
-                sdap_account_info_handler(breq);
-                return;
+                ctx->gsh->connected = false;
+                err = sdap_account_info_restart(breq);
+                if (err == EOK) return;
             }
             sdap_mark_offline(ctx);
         }
@@ -736,8 +760,8 @@ static void sdap_account_info_initgr_done(struct tevent_req *req)
             ctx = talloc_get_type(breq->be_ctx->bet_info[BET_ID].pvt_bet_data,
                                   struct sdap_id_ctx);
             if (sdap_check_gssapi_reconnect(ctx)) {
-                talloc_zfree(ctx->gsh);
-                sdap_account_info_handler(breq);
+                ctx->gsh->connected = false;
+                sdap_account_info_restart(breq);
                 return;
             }
             sdap_mark_offline(ctx);
diff --git a/src/providers/ldap/ldap_id_enum.c b/src/providers/ldap/ldap_id_enum.c
index bc06e8bd..d86b38d0 100644
--- a/src/providers/ldap/ldap_id_enum.c
+++ b/src/providers/ldap/ldap_id_enum.c
@@ -143,12 +143,14 @@ int ldap_id_enumerate_set_timer(struct sdap_id_ctx *ctx, struct timeval tv)
     return EOK;
 }
 
+#define MAX_ENUM_RESTARTS 3
 
 struct global_enum_state {
     struct tevent_context *ev;
     struct sdap_id_ctx *ctx;
 
     bool purge;
+    int restarts;
 };
 
 static struct tevent_req *enum_users_send(TALLOC_CTX *memctx,
@@ -162,6 +164,8 @@ static struct tevent_req *enum_groups_send(TALLOC_CTX *memctx,
                                           bool purge);
 static void ldap_id_enum_groups_done(struct tevent_req *subreq);
 static void ldap_id_enum_cleanup_done(struct tevent_req *subreq);
+static int ldap_id_enum_users_restart(struct tevent_req *req);
+static int ldap_id_enum_groups_restart(struct tevent_req *req);
 
 static struct tevent_req *ldap_id_enumerate_send(struct tevent_context *ev,
                                                  struct sdap_id_ctx *ctx)
@@ -175,6 +179,7 @@ static struct tevent_req *ldap_id_enumerate_send(struct tevent_context *ev,
 
     state->ev = ev;
     state->ctx = ctx;
+    state->restarts = 0;
 
     ctx->last_enum = tevent_timeval_current();
 
@@ -203,6 +208,7 @@ static void ldap_id_enum_users_done(struct tevent_req *subreq)
                                                  struct global_enum_state);
     enum tevent_req_state tstate;
     uint64_t err = 0;
+    int ret;
 
     if (tevent_req_is_error(subreq, &tstate, &err)) {
         if (tstate != TEVENT_REQ_USER_ERROR) {
@@ -228,12 +234,9 @@ fail:
                   (int)err, strerror(err)));
 
         if (sdap_check_gssapi_reconnect(state->ctx)) {
-            talloc_zfree(state->ctx->gsh);
-            subreq = enum_users_send(state, state->ev, state->ctx, state->purge);
-            if (subreq != NULL) {
-                tevent_req_set_callback(subreq, ldap_id_enum_users_done, req);
-                return;
-            }
+            state->ctx->gsh->connected = false;
+            ret = ldap_id_enum_users_restart(req);
+            if (ret == EOK) return;
         }
         sdap_mark_offline(state->ctx);
     }
@@ -250,6 +253,7 @@ static void ldap_id_enum_groups_done(struct tevent_req *subreq)
                                                  struct global_enum_state);
     enum tevent_req_state tstate;
     uint64_t err = 0;
+    int ret;
 
     if (tevent_req_is_error(subreq, &tstate, &err)) {
         if (tstate != TEVENT_REQ_USER_ERROR) {
@@ -278,12 +282,9 @@ static void ldap_id_enum_groups_done(struct tevent_req *subreq)
 fail:
     /* check if credentials are expired otherwise go offline on failures */
     if (sdap_check_gssapi_reconnect(state->ctx)) {
-        talloc_zfree(state->ctx->gsh);
-        subreq = enum_groups_send(state, state->ev, state->ctx, state->purge);
-        if (subreq != NULL) {
-            tevent_req_set_callback(subreq, ldap_id_enum_groups_done, req);
-            return;
-        }
+        state->ctx->gsh->connected = false;
+        ret = ldap_id_enum_groups_restart(req);
+        if (ret == EOK) return;
     }
     sdap_mark_offline(state->ctx);
     DEBUG(1, ("Failed to enumerate groups (%d [%s]), retrying later!\n",
@@ -299,6 +300,88 @@ static void ldap_id_enum_cleanup_done(struct tevent_req *subreq)
     tevent_req_done(req);
 }
 
+static void ldap_id_enum_users_immediate(struct tevent_context *ctx,
+                                         struct tevent_immediate *im,
+                                         void *private_data)
+{
+    struct tevent_req *req = talloc_get_type(private_data,
+                                             struct tevent_req);
+    struct global_enum_state *state = tevent_req_data(req,
+                                                 struct global_enum_state);
+    struct tevent_req *subreq;
+
+    subreq = enum_users_send(state, state->ev, state->ctx, state->purge);
+    if (subreq == NULL) {
+        tevent_req_error(req, ENOMEM);
+        return;
+    }
+    tevent_req_set_callback(subreq, ldap_id_enum_users_done, req);
+}
+
+static int ldap_id_enum_users_restart(struct tevent_req *req)
+{
+    struct global_enum_state *state = tevent_req_data(req,
+                                                 struct global_enum_state);
+    struct tevent_immediate *im;
+
+    state->restarts++;
+    if (state->restarts < MAX_ENUM_RESTARTS) {
+        return ELOOP;
+    }
+
+    im = tevent_create_immediate(req);
+    if (!im) {
+        return ENOMEM;
+    }
+
+    /* schedule a completely new event to avoid deep recursions */
+    tevent_schedule_immediate(im, state->ev,
+                              ldap_id_enum_users_immediate, req);
+
+    return EOK;
+}
+
+static void ldap_id_enum_groups_immediate(struct tevent_context *ctx,
+                                          struct tevent_immediate *im,
+                                          void *private_data)
+{
+    struct tevent_req *req = talloc_get_type(private_data,
+                                             struct tevent_req);
+    struct global_enum_state *state = tevent_req_data(req,
+                                                 struct global_enum_state);
+    struct tevent_req *subreq;
+
+    subreq = enum_groups_send(state, state->ev, state->ctx, state->purge);
+    if (subreq == NULL) {
+        tevent_req_error(req, ENOMEM);
+        return;
+    }
+    tevent_req_set_callback(subreq, ldap_id_enum_groups_done, req);
+}
+
+static int ldap_id_enum_groups_restart(struct tevent_req *req)
+{
+    struct global_enum_state *state = tevent_req_data(req,
+                                                 struct global_enum_state);
+    struct tevent_immediate *im;
+
+    state->restarts++;
+    if (state->restarts < MAX_ENUM_RESTARTS) {
+        return ELOOP;
+    }
+
+    im = tevent_create_immediate(req);
+    if (!im) {
+        return ENOMEM;
+    }
+
+    /* schedule a completely new event to avoid deep recursions */
+    tevent_schedule_immediate(im, state->ev,
+                              ldap_id_enum_groups_immediate, req);
+
+    return EOK;
+}
+
 
 /* ==User-Enumeration===================================================== */
 
@@ -357,8 +440,6 @@ static struct tevent_req *enum_users_send(TALLOC_CTX *memctx,
 
     if (!sdap_connected(ctx)) {
 
-        if (ctx->gsh) talloc_zfree(ctx->gsh);
-
         /* FIXME: add option to decide if tls should be used
          * or SASL/GSSAPI, etc ... */
         subreq = sdap_cli_connect_send(state, ev, ctx->opts,
@@ -512,8 +593,6 @@ static struct tevent_req *enum_groups_send(TALLOC_CTX *memctx,
 
     if (!sdap_connected(ctx)) {
 
-        if (ctx->gsh) talloc_zfree(ctx->gsh);
-
         /* FIXME: add option to decide if tls should be used
          * or SASL/GSSAPI, etc ... */
         subreq = sdap_cli_connect_send(state, ev, ctx->opts,
diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c
index 9871dc21..ec36a601 100644
--- a/src/providers/ldap/sdap_async_connection.c
+++ b/src/providers/ldap/sdap_async_connection.c
@@ -1119,6 +1119,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;