rpms/sssd
7
0
Fork 0
Code Issues Pull Requests Releases Activity
e37379577b
sssd/0091-FO-Use-refcount-to-keep-track-of-servers-returned-to.patch
Lukas Slebodnik e37379577b Resolves: rhbz#1276868 - Sudo PAM Login should support multiple password 
prompts (e.g. Password + Token)
- Resolves: rhbz#1313041 - ssh with sssd proxy fails with "Connection closed
                           by remote host" if locale not available
2016-03-22 09:06:29 +01:00

490 lines
18 KiB
Diff
Raw Blame History

From 54d7922d287368fe419cafaa754aaf43b3ff1c93 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Sun, 11 Oct 2015 15:34:44 +0200
Subject: [PATCH 091/108] 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 <pbrezina@redhat.com>
(cherry picked from commit 10c07e188323a2f9824b5e34379f3b1a9b37759e)
(cherry picked from commit c532290fb1a85a63c3bc9ef95ba7d6248f0e94c1)
---
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 d1d8c4c5c94811ce73b550dc99166d3c913b95aa..39f57255af220b608d333da7c0bd1697788ea8f6 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 ffeeca4a6bad976ae8922bc4964b839242290259..bc5f716ffb8b85244604e3830141d390b7979a7c 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 3e6f1c2a24aaf713288146cc25d9cc462f243160..65d4fb690d270fa04a0e22f7db37bd8521ad3ba1 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 b8272a0a16015ff6b5d287b775c33a77e23eba67..f24b5715f13931965400c20562a1578aaf756908 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 2fab92e5d22a4dae870c5e9dde7ef162fc36cbe2..8d6a37b2ceb3347cb8092858889d07e5615e5c77 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 40256de99006815c97ee9390dfd2e997cf6fc072..f9074afb0c1340c7c2a50d4df0021eee4ae0d076 100644
--- a/src/providers/ldap/sdap_async_connection.c
+++ b/src/providers/ldap/sdap_async_connection.c
@@ -1160,7 +1160,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
@@ -1520,7 +1520,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 c9bac68711cfcf624064b5881f5226d4f8449e39..020f9d474b6bc8cee4fe80098f9631c4e343cec6 100644
--- a/src/tests/fail_over-tests.c
+++ b/src/tests/fail_over-tests.c
@@ -156,7 +156,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.7.3
Reference in New Issue View Git Blame Copy Permalink