Additional patch for upstream #2829
This commit is contained in:
Lukas Slebodnik
parent
5df019d5aa
commit
a89ed4b83f
166
0104-FO-Use-tevent_req_defer_callback-when-notifying-call.patch
Normal file
166
0104-FO-Use-tevent_req_defer_callback-when-notifying-call.patch
Normal file
@ -0,0 +1,166 @@
|
||||
From bb102b392ff298ecd8a499f6ddc3904c59dcbba2 Mon Sep 17 00:00:00 2001
|
||||
From: Jakub Hrozek <jhrozek@redhat.com>
|
||||
Date: Wed, 18 Nov 2015 20:48:51 +0100
|
||||
Subject: [PATCH 104/104] FO: Use tevent_req_defer_callback() when notifying
|
||||
callers
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
If a fo_resolve_service callback would modify the server->common member
|
||||
in any way, for example by dereferencing the server and lowering the
|
||||
refcount to 0, which would free the common structure, then the next
|
||||
iteration of fo_resolve_service_done would access memory that was
|
||||
already gone.
|
||||
|
||||
Please see
|
||||
https://tevent.samba.org/group__tevent__request.html#ga09373077d0b39e321a196a86bfebf280
|
||||
for more details.
|
||||
|
||||
Reviewed-by: Pavel Březina <pbrezina@redhat.com>
|
||||
(cherry picked from commit a92f68763a57b211a1bf6b80b6dd80c4a1aa2738)
|
||||
---
|
||||
src/providers/fail_over.c | 15 +++++++++++--
|
||||
src/tests/cmocka/test_fo_srv.c | 49 +++++++++++++++++++++++++++++++++++++++---
|
||||
2 files changed, 59 insertions(+), 5 deletions(-)
|
||||
|
||||
diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c
|
||||
index c60310d..0b99098 100644
|
||||
--- a/src/providers/fail_over.c
|
||||
+++ b/src/providers/fail_over.c
|
||||
@@ -131,6 +131,7 @@ struct resolve_service_request {
|
||||
|
||||
struct server_common *server_common;
|
||||
struct tevent_req *req;
|
||||
+ struct tevent_context *ev;
|
||||
};
|
||||
|
||||
struct status {
|
||||
@@ -940,7 +941,9 @@ resolve_service_request_destructor(struct resolve_service_request *request)
|
||||
}
|
||||
|
||||
static int
|
||||
-set_lookup_hook(struct fo_server *server, struct tevent_req *req)
|
||||
+set_lookup_hook(struct tevent_context *ev,
|
||||
+ struct fo_server *server,
|
||||
+ struct tevent_req *req)
|
||||
{
|
||||
struct resolve_service_request *request;
|
||||
|
||||
@@ -956,6 +959,7 @@ set_lookup_hook(struct fo_server *server, struct tevent_req *req)
|
||||
talloc_free(request);
|
||||
return ENOMEM;
|
||||
}
|
||||
+ request->ev = ev;
|
||||
request->req = req;
|
||||
DLIST_ADD(server->common->request_list, request);
|
||||
talloc_set_destructor(request, resolve_service_request_destructor);
|
||||
@@ -1142,7 +1146,7 @@ fo_resolve_service_server(struct tevent_req *req)
|
||||
case SERVER_RESOLVING_NAME:
|
||||
/* Name resolution is already under way. Just add ourselves into the
|
||||
* waiting queue so we get notified after the operation is finished. */
|
||||
- ret = set_lookup_hook(state->server, req);
|
||||
+ ret = set_lookup_hook(state->ev, state->server, req);
|
||||
if (ret != EOK) {
|
||||
tevent_req_error(req, ret);
|
||||
return true;
|
||||
@@ -1194,6 +1198,13 @@ fo_resolve_service_done(struct tevent_req *subreq)
|
||||
/* Take care of all requests for this server. */
|
||||
while ((request = common->request_list) != NULL) {
|
||||
DLIST_REMOVE(common->request_list, request);
|
||||
+
|
||||
+ /* If the request callback decresed refcount on the returned
|
||||
+ * server, we would have crashed as common would not be valid
|
||||
+ * anymore. Rather schedule the notify for next tev iteration
|
||||
+ */
|
||||
+ tevent_req_defer_callback(request->req, request->ev);
|
||||
+
|
||||
if (ret) {
|
||||
tevent_req_error(request->req, ret);
|
||||
} else {
|
||||
diff --git a/src/tests/cmocka/test_fo_srv.c b/src/tests/cmocka/test_fo_srv.c
|
||||
index 67f86fb..a84ce43 100644
|
||||
--- a/src/tests/cmocka/test_fo_srv.c
|
||||
+++ b/src/tests/cmocka/test_fo_srv.c
|
||||
@@ -575,10 +575,10 @@ static void test_fo_srv_before(struct tevent_req *req)
|
||||
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);
|
||||
+ s1 = mock_ares_reply(test_ctx, "ldap1.sssd.com", 100, 2, 389);
|
||||
assert_non_null(s1);
|
||||
|
||||
- s2 = mock_ares_reply(test_ctx, "ldap3.sssd.com", 100, 1, 389);
|
||||
+ s2 = mock_ares_reply(test_ctx, "ldap2.sssd.com", 100, 1, 389);
|
||||
assert_non_null(s2);
|
||||
|
||||
s1->next = s2;
|
||||
@@ -596,12 +596,17 @@ static void test_fo_srv_before(struct tevent_req *req)
|
||||
tevent_req_set_callback(req, test_fo_srv_after, test_ctx);
|
||||
}
|
||||
|
||||
+static void test_fo_srv_after2(struct tevent_req *req);
|
||||
+
|
||||
static void test_fo_srv_after(struct tevent_req *req)
|
||||
{
|
||||
struct test_fo_ctx *test_ctx = \
|
||||
tevent_req_callback_data(req, struct test_fo_ctx);
|
||||
struct fo_server *srv;
|
||||
errno_t ret;
|
||||
+ struct ares_srv_reply *s1;
|
||||
+ struct ares_srv_reply *s2;
|
||||
+ char *dns_domain;
|
||||
|
||||
ret = fo_resolve_service_recv(req, req, &srv);
|
||||
talloc_zfree(req);
|
||||
@@ -612,8 +617,46 @@ static void test_fo_srv_after(struct tevent_req *req)
|
||||
*/
|
||||
fo_set_server_status(test_ctx->srv, SERVER_WORKING);
|
||||
|
||||
+ sleep(test_ctx->ttl + 1);
|
||||
+
|
||||
/* Must be a different server now */
|
||||
- check_server(test_ctx, srv, 389, "ldap3.sssd.com");
|
||||
+ check_server(test_ctx, srv, 389, "ldap2.sssd.com");
|
||||
+
|
||||
+ /* Simulate changing the DNS environment. Change the host names */
|
||||
+ s1 = mock_ares_reply(test_ctx, "ldap1.sssd.com", 100, 1, 389);
|
||||
+ assert_non_null(s1);
|
||||
+
|
||||
+ s2 = mock_ares_reply(test_ctx, "ldap2.sssd.com", 100, 2, 389);
|
||||
+ assert_non_null(s2);
|
||||
+
|
||||
+ 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);
|
||||
+ sleep(test_ctx->ttl + 1);
|
||||
+
|
||||
+ req = fo_resolve_service_send(test_ctx, test_ctx->ctx->ev,
|
||||
+ test_ctx->resolv, test_ctx->fo_ctx,
|
||||
+ test_ctx->fo_svc);
|
||||
+ assert_non_null(req);
|
||||
+ tevent_req_set_callback(req, test_fo_srv_after2, test_ctx);
|
||||
+}
|
||||
+
|
||||
+static void test_fo_srv_after2(struct tevent_req *req)
|
||||
+{
|
||||
+ struct test_fo_ctx *test_ctx = \
|
||||
+ tevent_req_callback_data(req, struct test_fo_ctx);
|
||||
+ struct fo_server *srv;
|
||||
+ errno_t ret;
|
||||
+
|
||||
+ ret = fo_resolve_service_recv(req, req, &srv);
|
||||
+ talloc_zfree(req);
|
||||
+ assert_int_equal(ret, ERR_OK);
|
||||
+
|
||||
+ /* Must be a different server now */
|
||||
+ check_server(test_ctx, srv, 389, "ldap1.sssd.com");
|
||||
|
||||
test_ctx->ctx->error = ERR_OK;
|
||||
test_ctx->ctx->done = true;
|
||||
--
|
||||
2.5.0
|
||||
|
||||
@ -37,6 +37,7 @@ 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
|
||||
Patch0104: 0104-FO-Use-tevent_req_defer_callback-when-notifying-call.patch
|
||||
|
||||
### Dependencies ###
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user