[SSSD] [PATCH] Use refcount to keep track of server structures returned from failover

Jakub Hrozek jhrozek at redhat.com
Sun Oct 11 20:00:52 UTC 2015


Hi,

the attached patches are my proposal to fix
https://fedorahosted.org/sssd/ticket/2829

I haven't tested them past make check yet, because I'm not sure I like
them myself :) but at the same time I can't see a better way to keep
track of the servers and let callers set state of servers.

The most ugly thing so far IMO is the fo_internal_owner member. I would
prefer to instead have a fo_server_wrap structure that would be used for
the server_list, but I didn't want to do a large change before we agree
the refcount is a good idea at all.

The other ugly side-effect is that we need to be sure that nobody calls
talloc_free on the fo_server structure. Instead, only the parent context
can be freed (that's also what the first patch is about).
-------------- next part --------------
>From 2f71ee096701e869ed3e4646cbf8a4bf7a380f00 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Sun, 11 Oct 2015 18:08:46 +0200
Subject: [PATCH 1/4] FO: Don't free rc-allocated structure

---
 src/providers/fail_over.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c
index b076687ac6e571f7e27402fd11ac60183ea46951..b309f1c68d0f4219d4b97eb0c01416e53ea856d0 100644
--- a/src/providers/fail_over.c
+++ b/src/providers/fail_over.c
@@ -507,7 +507,6 @@ create_server_common(TALLOC_CTX *mem_ctx, struct fo_ctx *ctx, const char *name)
 
     common->name = talloc_strdup(common, name);
     if (common->name == NULL) {
-        talloc_free(common);
         return NULL;
     }
 
-- 
2.4.3

-------------- next part --------------
>From 3dd31148b43d0b466c055cf637c00ea8391b694b Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Sun, 11 Oct 2015 15:31:44 +0200
Subject: [PATCH 2/4] tests: Reduce failover code duplication

---
 src/tests/cmocka/test_fo_srv.c | 88 +++++++++++++++++-------------------------
 1 file changed, 35 insertions(+), 53 deletions(-)

diff --git a/src/tests/cmocka/test_fo_srv.c b/src/tests/cmocka/test_fo_srv.c
index e892bab0a261779363ea78a10038b15acefc49b7..109f664c84238cf9c1055a1cbc1a8c8870f2dc39 100644
--- a/src/tests/cmocka/test_fo_srv.c
+++ b/src/tests/cmocka/test_fo_srv.c
@@ -319,34 +319,50 @@ static void test_fo_srv_done3(struct tevent_req *req);
 static void test_fo_srv_done4(struct tevent_req *req);
 static void test_fo_srv_done5(struct tevent_req *req);
 
-static void test_fo_srv_mock_dns(struct test_fo_ctx *test_ctx)
+
+struct ares_srv_reply *
+mock_ares_reply(TALLOC_CTX *mem_ctx, const char *hostname,
+                int weight, int priority, int port)
+{
+    struct ares_srv_reply *s;
+
+    s = talloc_zero(mem_ctx, struct ares_srv_reply);
+    if (s == NULL) {
+        return NULL;
+    }
+
+    s->host = talloc_strdup(s, hostname);
+    if (s->host == NULL) {
+        talloc_free(s);
+        return NULL;
+    }
+
+    s->weight = weight;
+    s->priority = priority;
+    s->port = port;
+
+    return s;
+}
+
+static void test_fo_srv_mock_dns(struct test_fo_ctx *test_ctx,
+                                 int ttl)
 {
     struct ares_srv_reply *s1;
     struct ares_srv_reply *s2;
     char *dns_domain;
 
-    s1 = talloc_zero(test_ctx, struct ares_srv_reply);
+    s1 = mock_ares_reply(test_ctx, "ldap1.sssd.com", 100, 1, 389);
     assert_non_null(s1);
-    s1->host = talloc_strdup(s1, "ldap1.sssd.com");
-    assert_non_null(s1->host);
-    s1->weight = 100;
-    s1->priority = 1;
-    s1->port = 389;
 
-    s2 = talloc_zero(test_ctx, struct ares_srv_reply);
+    s2 = mock_ares_reply(test_ctx, "ldap2.sssd.com", 100, 2, 389);
     assert_non_null(s2);
-    s2->host = talloc_strdup(s2, "ldap2.sssd.com");
-    assert_non_null(s2->host);
-    s2->weight = 100;
-    s2->priority = 2;
-    s2->port = 389;
 
     s1->next = s2;
 
     dns_domain = talloc_strdup(test_ctx, "sssd.com");
     assert_non_null(dns_domain);
 
-    mock_srv_results(s1, TEST_SRV_TTL, dns_domain);
+    mock_srv_results(s1, ttl, dns_domain);
 }
 
 static void test_fo_srv(void **state)
@@ -355,7 +371,7 @@ static void test_fo_srv(void **state)
     struct test_fo_ctx *test_ctx =
         talloc_get_type(*state, struct test_fo_ctx);
 
-    test_fo_srv_mock_dns(test_ctx);
+    test_fo_srv_mock_dns(test_ctx, TEST_SRV_TTL);
 
     ret = fo_add_srv_server(test_ctx->fo_svc, "_ldap", "sssd.com",
                             "sssd.local", "tcp", test_ctx);
@@ -466,7 +482,7 @@ static void test_fo_srv_done4(struct tevent_req *req)
     /* reset the server status and try again.. */
     fo_reset_servers(test_ctx->fo_svc);
     if (test_ctx->srv_ctx) {
-        test_fo_srv_mock_dns(test_ctx);
+        test_fo_srv_mock_dns(test_ctx, TEST_SRV_TTL);
     }
 
     req = fo_resolve_service_send(test_ctx, test_ctx->ctx->ev,
@@ -516,32 +532,8 @@ static void test_fo_srv_ttl_change_step(struct test_fo_ctx *test_ctx)
 {
     errno_t ret;
     struct tevent_req *req;
-    struct ares_srv_reply *s1;
-    struct ares_srv_reply *s2;
-    char *dns_domain;
 
-    s1 = talloc_zero(test_ctx, struct ares_srv_reply);
-    assert_non_null(s1);
-    s1->host = talloc_strdup(s1, "ldap1.sssd.com");
-    assert_non_null(s1->host);
-    s1->weight = 100;
-    s1->priority = 1;
-    s1->port = 389;
-
-    s2 = talloc_zero(test_ctx, struct ares_srv_reply);
-    assert_non_null(s2);
-    s2->host = talloc_strdup(s2, "ldap2.sssd.com");
-    assert_non_null(s2->host);
-    s2->weight = 100;
-    s2->priority = 2;
-    s2->port = 389;
-
-    s1->next = s2;
-
-    dns_domain = talloc_strdup(test_ctx, "sssd.com");
-    assert_non_null(dns_domain);
-
-    mock_srv_results(s1, test_ctx->ttl, dns_domain);
+    test_fo_srv_mock_dns(test_ctx, test_ctx->ttl);
 
     ret = fo_add_srv_server(test_ctx->fo_svc, "_ldap", "sssd.com",
                             "sssd.local", "tcp", test_ctx);
@@ -582,21 +574,11 @@ static void test_fo_srv_before(struct tevent_req *req)
     fo_set_server_status(srv, SERVER_WORKING);
 
     /* Simulate changing the DNS environment. Change the host names */
-    s1 = talloc_zero(test_ctx, struct ares_srv_reply);
+    s1 = mock_ares_reply(test_ctx, "ldap2.sssd.com", 100, 2, 389);
     assert_non_null(s1);
-    s1->host = talloc_strdup(s1, "ldap2.sssd.com");
-    assert_non_null(s1->host);
-    s1->weight = 100;
-    s1->priority = 2;
-    s1->port = 389;
 
-    s2 = talloc_zero(test_ctx, struct ares_srv_reply);
+    s2 = mock_ares_reply(test_ctx, "ldap3.sssd.com", 100, 1, 389);
     assert_non_null(s2);
-    s2->host = talloc_strdup(s2, "ldap3.sssd.com");
-    assert_non_null(s2->host);
-    s2->weight = 100;
-    s2->priority = 1;
-    s2->port = 389;
 
     s1->next = s2;
 
-- 
2.4.3

-------------- next part --------------
>From a511ab3b9794b7e81e698aa53666d7dca50103ff Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Sun, 11 Oct 2015 15:34:44 +0200
Subject: [PATCH 3/4] tests: Reproduce use-after free when SRV result list
 expires

---
 src/tests/cmocka/test_fo_srv.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/tests/cmocka/test_fo_srv.c b/src/tests/cmocka/test_fo_srv.c
index 109f664c84238cf9c1055a1cbc1a8c8870f2dc39..ffdc2d0fc2615e13dd472aa219bbc7cd79611451 100644
--- a/src/tests/cmocka/test_fo_srv.c
+++ b/src/tests/cmocka/test_fo_srv.c
@@ -201,6 +201,8 @@ struct test_fo_ctx {
     struct fo_service *fo_svc;
     struct sss_test_ctx *ctx;
     int ttl;
+
+    struct fo_server *srv;
 };
 
 int test_fo_srv_data_cmp(void *ud1, void *ud2)
@@ -588,6 +590,8 @@ static void test_fo_srv_before(struct tevent_req *req)
     mock_srv_results(s1, test_ctx->ttl, dns_domain);
     sleep(test_ctx->ttl + 1);
 
+    test_ctx->srv = srv;
+
     req = fo_resolve_service_send(test_ctx, test_ctx->ctx->ev,
                                   test_ctx->resolv, test_ctx->fo_ctx,
                                   test_ctx->fo_svc);
@@ -606,6 +610,11 @@ static void test_fo_srv_after(struct tevent_req *req)
     talloc_zfree(req);
     assert_int_equal(ret, ERR_OK);
 
+    /* Try accessing server from a previous iteration. The
+     * server should be collapsed, but at least we shouldn't crash
+     */
+    fo_set_server_status(test_ctx->srv, SERVER_WORKING);
+
     /* Must be a different server now */
     check_server(test_ctx, srv, 389, "ldap3.sssd.com");
 
-- 
2.4.3

-------------- next part --------------
>From 2a5f9d97e8b7ac512a5252a3f7e7bd760bd66b67 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Sun, 11 Oct 2015 16:10:00 +0200
Subject: [PATCH 4/4] FO: Use refcount to keep track of servers returned to
 callers

---
 src/providers/data_provider_fo.c           |  7 ++-
 src/providers/dp_backend.h                 |  4 +-
 src/providers/fail_over.c                  | 95 ++++++++++++++++++++++++------
 src/providers/fail_over.h                  | 10 +++-
 src/providers/krb5/krb5_auth.c             |  4 +-
 src/providers/ldap/ldap_auth.c             |  2 +-
 src/providers/ldap/sdap_async_connection.c |  4 +-
 src/tests/cmocka/test_fo_srv.c             | 21 +++----
 src/tests/fail_over-tests.c                |  2 +-
 9 files changed, 108 insertions(+), 41 deletions(-)

diff --git a/src/providers/data_provider_fo.c b/src/providers/data_provider_fo.c
index cd57340a0ba0ac7e474dc502bf1f1b4de0e1f778..4c4d7b233bb4bd22aa7c7dcd1fec955c92fb08e4 100644
--- a/src/providers/data_provider_fo.c
+++ b/src/providers/data_provider_fo.c
@@ -606,7 +606,7 @@ errno_t be_resolve_server_process(struct tevent_req *subreq,
     time_t srv_status_change;
     struct be_svc_callback *callback;
 
-    ret = fo_resolve_service_recv(subreq, &state->srv);
+    ret = fo_resolve_service_recv(subreq, state, &state->srv);
     switch (ret) {
     case EOK:
         if (!state->srv) {
@@ -699,7 +699,9 @@ errno_t be_resolve_server_process(struct tevent_req *subreq,
     return EOK;
 }
 
-int be_resolve_server_recv(struct tevent_req *req, struct fo_server **srv)
+int be_resolve_server_recv(struct tevent_req *req,
+                           TALLOC_CTX *ref_ctx,
+                           struct fo_server **srv)
 {
     struct be_resolve_server_state *state = tevent_req_data(req,
                                              struct be_resolve_server_state);
@@ -707,6 +709,7 @@ int be_resolve_server_recv(struct tevent_req *req, struct fo_server **srv)
     TEVENT_REQ_RETURN_ON_ERROR(req);
 
     if (srv) {
+        fo_ref_server(ref_ctx, state->srv);
         *srv = state->srv;
     }
 
diff --git a/src/providers/dp_backend.h b/src/providers/dp_backend.h
index 0ced851be8468ce21a9d283e26461fc47194557e..f90d0b9c5fe69b1b14caa090bb515c60746de154 100644
--- a/src/providers/dp_backend.h
+++ b/src/providers/dp_backend.h
@@ -258,7 +258,9 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
                                           struct be_ctx *ctx,
                                           const char *service_name,
                                           bool first_try);
-int be_resolve_server_recv(struct tevent_req *req, struct fo_server **srv);
+int be_resolve_server_recv(struct tevent_req *req,
+                           TALLOC_CTX *ref_ctx,
+                           struct fo_server **srv);
 
 #define be_fo_set_port_status(ctx, service_name, server, status) \
     _be_fo_set_port_status(ctx, service_name, server, status, \
diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c
index b309f1c68d0f4219d4b97eb0c01416e53ea856d0..24aed9dfa469ad730d176244eb329522a43c6fd8 100644
--- a/src/providers/fail_over.c
+++ b/src/providers/fail_over.c
@@ -79,6 +79,8 @@ struct fo_service {
 };
 
 struct fo_server {
+    REFCOUNT_COMMON;
+
     struct fo_server *prev;
     struct fo_server *next;
 
@@ -90,6 +92,8 @@ struct fo_server {
     struct fo_service *service;
     struct timeval last_status_change;
     struct server_common *common;
+
+    TALLOC_CTX *fo_internal_owner;
 };
 
 struct server_common {
@@ -217,6 +221,15 @@ int fo_is_srv_lookup(struct fo_server *s)
     return s && s->srv_data;
 }
 
+static void fo_server_free(struct fo_server *server)
+{
+    if (server == NULL) {
+        return;
+    }
+
+    talloc_free(server->fo_internal_owner);
+}
+
 static struct fo_server *
 collapse_srv_lookup(struct fo_server **_server)
 {
@@ -231,12 +244,12 @@ collapse_srv_lookup(struct fo_server **_server)
         while (server->prev && server->prev->srv_data == meta->srv_data) {
             tmp = server->prev;
             DLIST_REMOVE(server->service->server_list, tmp);
-            talloc_zfree(tmp);
+            fo_server_free(tmp);
         }
         while (server->next && server->next->srv_data == meta->srv_data) {
             tmp = server->next;
             DLIST_REMOVE(server->service->server_list, tmp);
-            talloc_zfree(tmp);
+            fo_server_free(tmp);
         }
 
         if (server == server->service->active_server) {
@@ -249,7 +262,7 @@ collapse_srv_lookup(struct fo_server **_server)
         /* add back the meta server to denote SRV lookup */
         DLIST_ADD_AFTER(server->service->server_list, meta, server);
         DLIST_REMOVE(server->service->server_list, server);
-        talloc_zfree(server);
+        fo_server_free(server);
     }
 
     meta->srv_data->srv_lookup_status = SRV_NEUTRAL;
@@ -502,8 +515,9 @@ create_server_common(TALLOC_CTX *mem_ctx, struct fo_ctx *ctx, const char *name)
     struct server_common *common;
 
     common = rc_alloc(mem_ctx, struct server_common);
-    if (common == NULL)
+    if (common == NULL) {
         return NULL;
+    }
 
     common->name = talloc_strdup(common, name);
     if (common->name == NULL) {
@@ -524,6 +538,41 @@ create_server_common(TALLOC_CTX *mem_ctx, struct fo_ctx *ctx, const char *name)
     return common;
 }
 
+static struct fo_server *
+fo_server_alloc(struct fo_service *service, int port,
+                void *user_data, bool primary)
+{
+    static struct fo_server *server;
+    TALLOC_CTX *server_owner;
+
+    server_owner = talloc_new(service);
+    if (server_owner == NULL) {
+        return NULL;
+    }
+
+    server = rc_alloc(server_owner, struct fo_server);
+    if (server == NULL) {
+        return NULL;
+    }
+
+    server->fo_internal_owner = server_owner;
+
+    server->common = NULL;
+    server->next = NULL;
+    server->prev = NULL;
+    server->srv_data = NULL;
+    server->last_status_change.tv_sec = 0;
+    server->last_status_change.tv_usec = 0;
+
+    server->port = port;
+    server->user_data = user_data;
+    server->service = service;
+    server->port_status = DEFAULT_PORT_STATUS;
+    server->primary = primary;
+
+    return server;
+}
+
 int
 fo_add_srv_server(struct fo_service *service, const char *srv,
                   const char *discovery_domain, const char *sssd_domain,
@@ -557,14 +606,11 @@ fo_add_srv_server(struct fo_service *service, const char *srv,
         }
     }
 
-    server = talloc_zero(service, struct fo_server);
-    if (server == NULL)
+    /* SRV servers are always primary */
+    server = fo_server_alloc(service, 0, user_data, true);
+    if (server == NULL) {
         return ENOMEM;
-
-    server->user_data = user_data;
-    server->service = service;
-    server->port_status = DEFAULT_PORT_STATUS;
-    server->primary = true; /* SRV servers are never back up */
+    }
 
     /* add the SRV-specific data */
     server->srv_data = talloc_zero(service, struct srv_data);
@@ -608,7 +654,7 @@ create_fo_server(struct fo_service *service, const char *name,
     struct fo_server *server;
     int ret;
 
-    server = talloc_zero(service, struct fo_server);
+    server = fo_server_alloc(service, port, user_data, primary);
     if (server == NULL)
         return NULL;
 
@@ -623,11 +669,11 @@ create_fo_server(struct fo_service *service, const char *name,
         if (ret == ENOENT) {
             server->common = create_server_common(server, service->ctx, name);
             if (server->common == NULL) {
-                talloc_free(server);
+                fo_server_free(server);
                 return NULL;
             }
         } else if (ret != EOK) {
-            talloc_free(server);
+            fo_server_free(server);
             return NULL;
         }
     }
@@ -760,7 +806,6 @@ static errno_t fo_add_server_list(struct fo_service *service,
         server = create_fo_server(service, servers[i].host, servers[i].port,
                                   user_data, primary);
         if (server == NULL) {
-            talloc_free(srv_list);
             return ENOMEM;
         }
 
@@ -769,7 +814,7 @@ static errno_t fo_add_server_list(struct fo_service *service,
         ret = fo_add_server_to_list(&srv_list, service->server_list,
                                     server, service->name);
         if (ret != EOK) {
-            talloc_zfree(server);
+            fo_server_free(server);
             continue;
         }
 
@@ -803,12 +848,20 @@ fo_add_server(struct fo_service *service, const char *name, int port,
     ret = fo_add_server_to_list(&service->server_list, service->server_list,
                                 server, service->name);
     if (ret != EOK) {
-        talloc_free(server);
+        fo_server_free(server);
     }
 
     return ret;
 }
 
+void fo_ref_server(TALLOC_CTX *ref_ctx,
+                   struct fo_server *server)
+{
+    if (server) {
+        rc_reference(ref_ctx, struct fo_server, server);
+    }
+}
+
 static int
 get_first_server_entity(struct fo_service *service, struct fo_server **_server)
 {
@@ -1150,7 +1203,9 @@ fo_resolve_service_done(struct tevent_req *subreq)
 }
 
 int
-fo_resolve_service_recv(struct tevent_req *req, struct fo_server **server)
+fo_resolve_service_recv(struct tevent_req *req,
+                        TALLOC_CTX *ref_ctx,
+                        struct fo_server **server)
 {
     struct resolve_service_state *state;
 
@@ -1158,8 +1213,10 @@ fo_resolve_service_recv(struct tevent_req *req, struct fo_server **server)
 
     /* always return the server if asked for, otherwise the caller
      * cannot mark it as faulty in case we return an error */
-    if (server)
+    if (server != NULL) {
+        fo_ref_server(ref_ctx, state->server);
         *server = state->server;
+    }
 
     TEVENT_REQ_RETURN_ON_ERROR(req);
 
diff --git a/src/providers/fail_over.h b/src/providers/fail_over.h
index e49c6414a14eb6ca2cad333f8efbb58576811345..75bff8da1cc29dd6184b6ec0d3fad545eb7204c2 100644
--- a/src/providers/fail_over.h
+++ b/src/providers/fail_over.h
@@ -128,7 +128,6 @@ int fo_add_server(struct fo_service *service,
                   const char *name, int port,
                   void *user_data, bool primary);
 
-
 int fo_add_srv_server(struct fo_service *service,
                       const char *srv,
                       const char *discovery_domain,
@@ -148,8 +147,17 @@ struct tevent_req *fo_resolve_service_send(TALLOC_CTX *mem_ctx,
                                            struct fo_service *service);
 
 int fo_resolve_service_recv(struct tevent_req *req,
+                            TALLOC_CTX *ref_ctx,
                             struct fo_server **server);
 
+
+/* To be used by async consumers of fo_resolve_service. If a server should be returned
+ * to an outer request, it should be referenced by a memory from that outer request,
+ * because the failover's server list might change with a subsequent call (see upstream
+ * bug #2829)
+ */
+void fo_ref_server(TALLOC_CTX *ref_ctx, struct fo_server *server);
+
 /*
  * Set feedback about 'server'. Caller should use this to indicate a problem
  * with the server itself, not only with the service on that server. This
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index e3e9601b356efd72e50ab86e8b7cdd048e4e70d4..7b7a16a612332639aa474a7ebea6b966df18f08f 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -695,9 +695,9 @@ static void krb5_auth_resolve_done(struct tevent_req *subreq)
     int ret;
 
     if (!state->search_kpasswd) {
-        ret = be_resolve_server_recv(subreq, &kr->srv);
+        ret = be_resolve_server_recv(subreq, kr, &kr->srv);
     } else {
-        ret = be_resolve_server_recv(subreq, &kr->kpasswd_srv);
+        ret = be_resolve_server_recv(subreq, kr, &kr->kpasswd_srv);
     }
     talloc_zfree(subreq);
 
diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c
index 217e80fd07abc41f2594d19397783683d44600cd..c94ba15bb17aa1641eb36781cc59ce158d48ca66 100644
--- a/src/providers/ldap/ldap_auth.c
+++ b/src/providers/ldap/ldap_auth.c
@@ -695,7 +695,7 @@ static void auth_resolve_done(struct tevent_req *subreq)
     int ret;
     bool use_tls;
 
-    ret = be_resolve_server_recv(subreq, &state->srv);
+    ret = be_resolve_server_recv(subreq, state, &state->srv);
     talloc_zfree(subreq);
     if (ret) {
         /* all servers have been tried and none
diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c
index 8f5227d263f995693f6e65bd238171538aa52af7..ef7a1594954b4cb11f35d2cd56b0c4806ead797a 100644
--- a/src/providers/ldap/sdap_async_connection.c
+++ b/src/providers/ldap/sdap_async_connection.c
@@ -1148,7 +1148,7 @@ static void sdap_kinit_kdc_resolved(struct tevent_req *subreq)
     struct tevent_req *tgtreq;
     int ret;
 
-    ret = be_resolve_server_recv(subreq, &state->kdc_srv);
+    ret = be_resolve_server_recv(subreq, state, &state->kdc_srv);
     talloc_zfree(subreq);
     if (ret != EOK) {
         /* all servers have been tried and none
@@ -1508,7 +1508,7 @@ static void sdap_cli_resolve_done(struct tevent_req *subreq)
                                              struct sdap_cli_connect_state);
     int ret;
 
-    ret = be_resolve_server_recv(subreq, &state->srv);
+    ret = be_resolve_server_recv(subreq, state, &state->srv);
     talloc_zfree(subreq);
     if (ret) {
         state->srv = NULL;
diff --git a/src/tests/cmocka/test_fo_srv.c b/src/tests/cmocka/test_fo_srv.c
index ffdc2d0fc2615e13dd472aa219bbc7cd79611451..67f86fb17753bf90b88d007a6a1b309df830c152 100644
--- a/src/tests/cmocka/test_fo_srv.c
+++ b/src/tests/cmocka/test_fo_srv.c
@@ -403,7 +403,7 @@ static void test_fo_srv_done1(struct tevent_req *req)
     struct fo_server *srv;
     errno_t ret;
 
-    ret = fo_resolve_service_recv(req, &srv);
+    ret = fo_resolve_service_recv(req, req, &srv);
     talloc_zfree(req);
     assert_int_equal(ret, ERR_OK);
 
@@ -428,7 +428,7 @@ static void test_fo_srv_done2(struct tevent_req *req)
     struct fo_server *srv;
     errno_t ret;
 
-    ret = fo_resolve_service_recv(req, &srv);
+    ret = fo_resolve_service_recv(req, req, &srv);
     talloc_zfree(req);
     assert_int_equal(ret, ERR_OK);
 
@@ -452,7 +452,7 @@ static void test_fo_srv_done3(struct tevent_req *req)
     struct fo_server *srv;
     errno_t ret;
 
-    ret = fo_resolve_service_recv(req, &srv);
+    ret = fo_resolve_service_recv(req, req, &srv);
     talloc_zfree(req);
     assert_int_equal(ret, ERR_OK);
 
@@ -476,7 +476,7 @@ static void test_fo_srv_done4(struct tevent_req *req)
     struct fo_server *srv;
     errno_t ret;
 
-    ret = fo_resolve_service_recv(req, &srv);
+    ret = fo_resolve_service_recv(req, req, &srv);
     talloc_zfree(req);
     /* No servers are left..*/
     assert_int_equal(ret, ENOENT);
@@ -501,7 +501,7 @@ static void test_fo_srv_done5(struct tevent_req *req)
     struct fo_server *srv;
     errno_t ret;
 
-    ret = fo_resolve_service_recv(req, &srv);
+    ret = fo_resolve_service_recv(req, req, &srv);
     talloc_zfree(req);
 
     assert_int_equal(ret, ERR_OK);
@@ -560,20 +560,19 @@ static void test_fo_srv_before(struct tevent_req *req)
 {
     struct test_fo_ctx *test_ctx = \
         tevent_req_callback_data(req, struct test_fo_ctx);
-    struct fo_server *srv;
     struct ares_srv_reply *s1;
     struct ares_srv_reply *s2;
     char *dns_domain;
     errno_t ret;
 
-    ret = fo_resolve_service_recv(req, &srv);
+    ret = fo_resolve_service_recv(req, test_ctx, &test_ctx->srv);
     talloc_zfree(req);
     assert_int_equal(ret, ERR_OK);
 
     DEBUG(SSSDBG_TRACE_FUNC, "Before TTL change\n");
 
-    check_server(test_ctx, srv, 389, "ldap1.sssd.com");
-    fo_set_server_status(srv, SERVER_WORKING);
+    check_server(test_ctx, test_ctx->srv, 389, "ldap1.sssd.com");
+    fo_set_server_status(test_ctx->srv, SERVER_WORKING);
 
     /* Simulate changing the DNS environment. Change the host names */
     s1 = mock_ares_reply(test_ctx, "ldap2.sssd.com", 100, 2, 389);
@@ -590,8 +589,6 @@ static void test_fo_srv_before(struct tevent_req *req)
     mock_srv_results(s1, test_ctx->ttl, dns_domain);
     sleep(test_ctx->ttl + 1);
 
-    test_ctx->srv = srv;
-
     req = fo_resolve_service_send(test_ctx, test_ctx->ctx->ev,
                                   test_ctx->resolv, test_ctx->fo_ctx,
                                   test_ctx->fo_svc);
@@ -606,7 +603,7 @@ static void test_fo_srv_after(struct tevent_req *req)
     struct fo_server *srv;
     errno_t ret;
 
-    ret = fo_resolve_service_recv(req, &srv);
+    ret = fo_resolve_service_recv(req, req, &srv);
     talloc_zfree(req);
     assert_int_equal(ret, ERR_OK);
 
diff --git a/src/tests/fail_over-tests.c b/src/tests/fail_over-tests.c
index b21ead38229be5d55df2de10bec3dd00a8566d71..7c296d116968ae059c75920c91059f6e83ea0508 100644
--- a/src/tests/fail_over-tests.c
+++ b/src/tests/fail_over-tests.c
@@ -154,7 +154,7 @@ test_resolve_service_callback(struct tevent_req *req)
 
     task->test_ctx->tasks--;
 
-    recv_status = fo_resolve_service_recv(req, &server);
+    recv_status = fo_resolve_service_recv(req, req, &server);
     talloc_free(req);
     fail_if(recv_status != task->recv, "%s: Expected return of %d, got %d",
             task->location, task->recv, recv_status);
-- 
2.4.3



More information about the sssd-devel mailing list