>From 8ebbddf2e11505f88a4ab132f8f321fd564a0f55 Mon Sep 17 00:00:00 2001
From: Simo Sorce <ssorce@redhat.com>
Date: Tue, 13 Apr 2010 12:19:25 -0400
Subject: [PATCH] Avoid freeing sdap_handle too early

Within sdap_release_handle() some callbacks can use sdap_mark_offline()
or try to restart calls. Make sure they do not end up freeing the
sdap_handle prematurely and cause access to freed memory later.

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()
---
 src/providers/data_provider_be.c  |    2 +-
 src/providers/dp_backend.h        |    4 ++
 src/providers/ldap/ldap_common.c  |    2 +-
 src/providers/ldap/ldap_id.c      |   49 ++++++++++++++----
 src/providers/ldap/ldap_id_enum.c |  105 ++++++++++++++++++++++++++++++++----
 5 files changed, 138 insertions(+), 24 deletions(-)

diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index 239b92a..591edb3 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 f1069d0..d657af1 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/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index b5765c2..90ec7e2 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 15e2f30..a2109f3 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -654,13 +654,42 @@ 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;
+    }
+
+    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 +703,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 +720,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 +734,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 +765,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 bc06e8b..6c5570d 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,86 @@ 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;
+    }
+
+    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;
+    }
+
+    tevent_schedule_immediate(im, state->ev,
+                              ldap_id_enum_groups_immediate, req);
+
+    return EOK;
+}
+
 
 /* ==User-Enumeration===================================================== */
 
-- 
1.6.6.1

