https://fedorahosted.org/sssd/ticket/1514
See the commit message and ticket comments for more information.
This patch introduces one of the possible solutions, but I suggest to
consider actually changing memory hierarchy in responders.
Currently, the hierarchy looks like this:
main_ctx->specific_ctx->rctx, where specific_ctx is one of the pam, nss,
sudo, etc. contexts.
Does anybody know why it is not main_ctx->rctx->specific_ctx? This would
solve the issue as well and I personally think that it is more logical.
From 7780bf2561bb77830bd9451f3a2f008ec0623700 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Tue, 2 Oct 2012 15:22:32 +0200
Subject: [PATCH] do not call dp callbacks when responder is shutting down
https://fedorahosted.org/sssd/ticket/1514
We were experiencing crash duting responder shut down. This happened
when there were some unresolved dp request during the shut down.
The memory hierarchy is main_ctx->specific_ctx->rctx, where
specific_ctx may be one of the pam, nss, sudo, etc. contexts.
If we try to call dp request callback as a result of responder
termination, the specific context is already semi freed, which may
cause crash.
---
src/responder/common/responder.h | 2 ++
src/responder/common/responder_common.c | 15 +++++++++++++++
src/responder/common/responder_dp.c | 8 ++++++++
3 files changed, 25 insertions(+)
diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h
index 5bab0d3c3aee67ad9a094a7a001a3df218e7fa16..2903aac0011eceb35dd2d26249b855750d7a1d34 100644
--- a/src/responder/common/responder.h
+++ b/src/responder/common/responder.h
@@ -104,6 +104,8 @@ struct resp_ctx {
char *default_domain;
void *pvt_ctx;
+
+ bool shutting_down;
};
struct cli_ctx {
diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index 4fa8190977e22dd6211fb5b2ec37cbfe64cac5e9..d9f73fe2698cd7eacfb1a589c159e5cb037dab64 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -716,6 +716,18 @@ failed:
return EIO;
}
+static int sss_responder_ctx_destructor(void *ptr)
+{
+ struct resp_ctx *rctx = talloc_get_type(ptr, struct resp_ctx);
+
+ /* mark that we are shutting down the responder, so it is propagated
+ * into underlying contexts that are freed right before rctx */
+ DEBUG(SSSDBG_TRACE_FUNC, ("Responder is being shut down\n"));
+ rctx->shutting_down = true;
+
+ return 0;
+}
+
int sss_process_init(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
struct confdb_ctx *cdb,
@@ -745,6 +757,9 @@ int sss_process_init(TALLOC_CTX *mem_ctx,
rctx->sock_name = sss_pipe_name;
rctx->priv_sock_name = sss_priv_pipe_name;
rctx->confdb_service_path = confdb_service_path;
+ rctx->shutting_down = false;
+
+ talloc_set_destructor((TALLOC_CTX*)rctx, sss_responder_ctx_destructor);
ret = confdb_get_int(rctx->cdb, rctx->confdb_service_path,
CONFDB_RESPONDER_CLI_IDLE_TIMEOUT,
diff --git a/src/responder/common/responder_dp.c b/src/responder/common/responder_dp.c
index ca9cb834de7a6c0c3ce89ecc8eb20f61fe6c0428..34fc9f3499f9b9f9f182e2a5a3e90419c6a0cad3 100644
--- a/src/responder/common/responder_dp.c
+++ b/src/responder/common/responder_dp.c
@@ -76,6 +76,14 @@ static int sss_dp_req_destructor(void *ptr)
sdp_req->pending_reply = NULL;
}
+ /* Do not call callbacks if the responder is shutting down, because
+ * the top level responder context (pam_ctx, sudo_ctx, ...) may be
+ * already semi-freed and we may end up accessing freed memory.
+ */
+ if (sdp_req->rctx->shutting_down) {
+ return 0;
+ }
+
/* If there are callbacks that haven't been invoked, return
* an error now.
*/
--
1.7.11.4
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
