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.

7 files changed, 144 insertions, 42 deletions

diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index 69aba6ceb..e5a5ff7b0 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -405,7 +405,7 @@ static int be_get_account_info(DBusMessage *message, struct sbus_connection *con
     }
 
     /* process request */
-    be_req = talloc(becli, struct be_req);
+    be_req = talloc_zero(becli, struct be_req);
     if (!be_req) {
         err_maj = DP_ERR_FATAL;
         err_min = ENOMEM;
diff --git a/src/providers/dp_backend.h b/src/providers/dp_backend.h
index f1069d0db..d657af18b 100644
--- a/src/providers/dp_backend.h
+++ b/src/providers/dp_backend.h
@@ -103,6 +103,8 @@ struct bet_ops {
     be_req_fn_t finalize;
 };
 
+#define MAX_BE_REQ_RESTARTS 2
+
 struct be_req {
     struct be_client *becli;
     struct be_ctx *be_ctx;
@@ -110,6 +112,8 @@ struct be_req {
 
     be_async_callback_t fn;
     void *pvt;
+
+    int restarts;
 };
 
 struct be_acct_req {
diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c
index a7058bad1..d88e5c931 100644
--- a/src/providers/ipa/ipa_access.c
+++ b/src/providers/ipa/ipa_access.c
@@ -325,10 +325,6 @@ static struct tevent_req *hbac_get_host_info_send(TALLOC_CTX *memctx,
     }
 
     if (sdap_ctx->gsh == NULL || ! sdap_ctx->gsh->connected) {
-        if (sdap_ctx->gsh != NULL) {
-            talloc_zfree(sdap_ctx->gsh);
-        }
-
         subreq = sdap_cli_connect_send(state, ev, sdap_ctx->opts,
                                        sdap_ctx->be, sdap_ctx->service, NULL);
         if (!subreq) {
@@ -780,10 +776,6 @@ static struct tevent_req *hbac_get_rules_send(TALLOC_CTX *memctx,
     }
 
     if (sdap_ctx->gsh == NULL || ! sdap_ctx->gsh->connected) {
-        if (sdap_ctx->gsh != NULL) {
-            talloc_zfree(sdap_ctx->gsh);
-        }
-
         subreq = sdap_cli_connect_send(state, ev, sdap_ctx->opts,
                                        sdap_ctx->be, sdap_ctx->service, NULL);
         if (!subreq) {
diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index b5765c276..90ec7e2e7 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 15e2f30ad..a2217da66 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 bc06e8bdc..d86b38d0f 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 9871dc210..ec36a6018 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;