From 31ed44fa55ae2553e474cf438f274b58d0262dbc Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Fri, 20 Nov 2015 09:19:25 +0100 Subject: [PATCH] Backport patches from sssd master #2829 Use after free in failover --- ...FO-Don-t-free-rc-allocated-structure.patch | 28 + ...sts-Reduce-failover-code-duplication.patch | 162 ++++++ ...to-keep-track-of-servers-returned-to.patch | 488 ++++++++++++++++++ ...x-warning-value-computed-is-not-used.patch | 33 ++ sssd.spec | 4 + 5 files changed, 715 insertions(+) create mode 100644 0100-FO-Don-t-free-rc-allocated-structure.patch create mode 100644 0101-tests-Reduce-failover-code-duplication.patch create mode 100644 0102-FO-Use-refcount-to-keep-track-of-servers-returned-to.patch create mode 100644 0103-FAIL_OVER-Fix-warning-value-computed-is-not-used.patch diff --git a/0100-FO-Don-t-free-rc-allocated-structure.patch b/0100-FO-Don-t-free-rc-allocated-structure.patch new file mode 100644 index 0000000..ad2eaf7 --- /dev/null +++ b/0100-FO-Don-t-free-rc-allocated-structure.patch @@ -0,0 +1,28 @@ +From a9e5c97001638eca5d5b4d88463edfb0e8b01cc0 Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Sun, 11 Oct 2015 18:08:46 +0200 +Subject: [PATCH 100/103] FO: Don't free rc-allocated structure +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Reviewed-by: Pavel Březina +--- + 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.5.0 + diff --git a/0101-tests-Reduce-failover-code-duplication.patch b/0101-tests-Reduce-failover-code-duplication.patch new file mode 100644 index 0000000..2e2d272 --- /dev/null +++ b/0101-tests-Reduce-failover-code-duplication.patch @@ -0,0 +1,162 @@ +From 9b095121329b2cc5f929af61222ad94705e5d38e Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Sun, 11 Oct 2015 15:31:44 +0200 +Subject: [PATCH 101/103] tests: Reduce failover code duplication +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Reviewed-by: Pavel Březina +(cherry picked from commit 4a4af8e1b6a9bab7c7a34d86055a400376e3829e) +--- + 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.5.0 + diff --git a/0102-FO-Use-refcount-to-keep-track-of-servers-returned-to.patch b/0102-FO-Use-refcount-to-keep-track-of-servers-returned-to.patch new file mode 100644 index 0000000..f683974 --- /dev/null +++ b/0102-FO-Use-refcount-to-keep-track-of-servers-returned-to.patch @@ -0,0 +1,488 @@ +From 24ef73fe8bbea12ceedddad1953c7bd94ce36f31 Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Sun, 11 Oct 2015 15:34:44 +0200 +Subject: [PATCH 102/103] FO: Use refcount to keep track of servers returned to + callers +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Resolves: + https://fedorahosted.org/sssd/ticket/2829 + +Reviewed-by: Pavel Březina +(cherry picked from commit 10c07e188323a2f9824b5e34379f3b1a9b37759e) +--- + 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 | 26 ++++---- + src/tests/fail_over-tests.c | 2 +- + 9 files changed, 115 insertions(+), 39 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 109f664c84238cf9c1055a1cbc1a8c8870f2dc39..67f86fb17753bf90b88d007a6a1b309df830c152 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) +@@ -401,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); + +@@ -426,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); + +@@ -450,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); + +@@ -474,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); +@@ -499,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); +@@ -558,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); +@@ -602,10 +603,15 @@ 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); + ++ /* 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"); + +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.5.0 + diff --git a/0103-FAIL_OVER-Fix-warning-value-computed-is-not-used.patch b/0103-FAIL_OVER-Fix-warning-value-computed-is-not-used.patch new file mode 100644 index 0000000..9d15eab --- /dev/null +++ b/0103-FAIL_OVER-Fix-warning-value-computed-is-not-used.patch @@ -0,0 +1,33 @@ +From 9d620a935122683e0e6d19953484084a9feb21f5 Mon Sep 17 00:00:00 2001 +From: Lukas Slebodnik +Date: Tue, 10 Nov 2015 07:41:10 +0000 +Subject: [PATCH 103/103] FAIL_OVER: Fix warning value computed is not used +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +src/providers/fail_over.c: In function ‘fo_ref_server’: +src/providers/fail_over.c:861: warning: value computed is not used + +Reviewed-by: Jakub Hrozek +(cherry picked from commit acd615cffd144b69e2558a0fc45c6966423f2d02) +--- + src/providers/fail_over.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c +index 24aed9dfa469ad730d176244eb329522a43c6fd8..c60310d8f17b129be115a4d3724ac7e54549820e 100644 +--- a/src/providers/fail_over.c ++++ b/src/providers/fail_over.c +@@ -858,7 +858,7 @@ void fo_ref_server(TALLOC_CTX *ref_ctx, + struct fo_server *server) + { + if (server) { +- rc_reference(ref_ctx, struct fo_server, server); ++ server = rc_reference(ref_ctx, struct fo_server, server); + } + } + +-- +2.5.0 + diff --git a/sssd.spec b/sssd.spec index f0a819a..a8da9ff 100644 --- a/sssd.spec +++ b/sssd.spec @@ -32,6 +32,10 @@ Source0: https://fedorahosted.org/released/sssd/%{name}-%{version}.tar.gz BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) ### Patches ### +Patch0100: 0100-FO-Don-t-free-rc-allocated-structure.patch +Patch0101: 0101-tests-Reduce-failover-code-duplication.patch +Patch0102: 0102-FO-Use-refcount-to-keep-track-of-servers-returned-to.patch +Patch0103: 0103-FAIL_OVER-Fix-warning-value-computed-is-not-used.patch ### Dependencies ###