URL: https://github.com/SSSD/sssd/pull/821
Author: thalman
 Title: #821: SERVER: Receving SIGSEGV process on shutdown
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/821/head:pr821
git checkout pr821
From 8ddf3c062903fe19b7d8e1f15571351c9040db08 Mon Sep 17 00:00:00 2001
From: Tomas Halman <[email protected]>
Date: Thu, 22 Aug 2019 13:44:10 +0200
Subject: [PATCH] SERVER: Receving SIGSEGV process on shutdown

There is race condition when dynamic libraries are unloaded. Talloc
library calls our destructors but they still need openssl calls
which might be not available.

Solution is to free explicitely memory context and trigger
destructors before calling exit(). In this PR the SIGTERM
handler is moved from individual providers to generel
backend code.

Also generic server code is changed to explicitely free
memory context when signal is received.

Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1672584
---
 src/providers/data_provider_be.c      | 30 ++++++++++++++++++
 src/providers/krb5/krb5_common.c      | 34 --------------------
 src/providers/krb5/krb5_common.h      |  3 --
 src/providers/krb5/krb5_init_shared.c |  6 ----
 src/providers/ldap/ldap_common.c      | 45 ---------------------------
 src/providers/ldap/ldap_common.h      |  4 ---
 src/util/server.c                     | 24 +++++++-------
 7 files changed, 43 insertions(+), 103 deletions(-)

diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index ce00231ff5..e79a171042 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -445,6 +445,36 @@ be_register_monitor_iface(struct sbus_connection *conn, struct be_ctx *be_ctx)
     return sbus_connection_add_path_map(be_ctx->mon_conn, paths);
 }
 
+static void be_process_finalize(struct tevent_context *ev,
+                                struct tevent_signal *se,
+                                int signum,
+                                int count,
+                                void *siginfo,
+                                void *private_data)
+{
+    struct be_ctx *be_ctx;
+
+    be_ctx = talloc_get_type(private_data, struct be_ctx);
+    talloc_free(be_ctx);
+    orderly_shutdown(0);
+}
+
+static errno_t be_process_install_sigterm_handler(struct be_ctx *be_ctx)
+{
+    struct tevent_signal *sige;
+
+    BlockSignals(false, SIGTERM);
+
+    sige = tevent_add_signal(be_ctx->ev, be_ctx, SIGTERM, SA_SIGINFO,
+                             be_process_finalize, be_ctx);
+    if (sige == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "tevent_add_signal failed.\n");
+        return ENOMEM;
+    }
+
+    return EOK;
+}
+
 static void dp_initialized(struct tevent_req *req);
 
 errno_t be_process_init(TALLOC_CTX *mem_ctx,
diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c
index f188dc8415..bfda561c12 100644
--- a/src/providers/krb5/krb5_common.c
+++ b/src/providers/krb5/krb5_common.c
@@ -25,7 +25,6 @@
 #include <sys/stat.h>
 #include <unistd.h>
 #include <netdb.h>
-#include <signal.h>
 #include <arpa/inet.h>
 #include <ctype.h>
 
@@ -1144,39 +1143,6 @@ void krb5_finalize(struct tevent_context *ev,
     orderly_shutdown(0);
 }
 
-errno_t krb5_install_sigterm_handler(struct tevent_context *ev,
-                                     struct krb5_ctx *krb5_ctx)
-{
-    const char *krb5_realm;
-    char *sig_realm;
-    struct tevent_signal *sige;
-
-    BlockSignals(false, SIGTERM);
-
-    krb5_realm = dp_opt_get_cstring(krb5_ctx->opts, KRB5_REALM);
-    if (krb5_realm == NULL) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "Missing krb5_realm option!\n");
-        return EINVAL;
-    }
-
-    sig_realm = talloc_strdup(krb5_ctx, krb5_realm);
-    if (sig_realm == NULL) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup failed!\n");
-        return ENOMEM;
-    }
-
-    sige = tevent_add_signal(ev, krb5_ctx, SIGTERM, SA_SIGINFO, krb5_finalize,
-                             sig_realm);
-    if (sige == NULL) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "tevent_add_signal failed.\n");
-        talloc_free(sig_realm);
-        return ENOMEM;
-    }
-    talloc_steal(sige, sig_realm);
-
-    return EOK;
-}
-
 errno_t krb5_get_simple_upn(TALLOC_CTX *mem_ctx, struct krb5_ctx *krb5_ctx,
                             struct sss_domain_info *dom, const char *username,
                             const char *user_dom, char **_upn)
diff --git a/src/providers/krb5/krb5_common.h b/src/providers/krb5/krb5_common.h
index 31e452617a..cc93131153 100644
--- a/src/providers/krb5/krb5_common.h
+++ b/src/providers/krb5/krb5_common.h
@@ -203,9 +203,6 @@ void krb5_finalize(struct tevent_context *ev,
                    void *siginfo,
                    void *private_data);
 
-errno_t krb5_install_sigterm_handler(struct tevent_context *ev,
-                                     struct krb5_ctx *krb5_ctx);
-
 errno_t remove_krb5_info_files(TALLOC_CTX *mem_ctx, const char *realm);
 
 errno_t krb5_get_simple_upn(TALLOC_CTX *mem_ctx, struct krb5_ctx *krb5_ctx,
diff --git a/src/providers/krb5/krb5_init_shared.c b/src/providers/krb5/krb5_init_shared.c
index 368d6f7b0f..afe15b365f 100644
--- a/src/providers/krb5/krb5_init_shared.c
+++ b/src/providers/krb5/krb5_init_shared.c
@@ -71,12 +71,6 @@ errno_t krb5_child_init(struct krb5_ctx *krb5_auth_ctx,
         goto done;
     }
 
-    ret = krb5_install_sigterm_handler(bectx->ev, krb5_auth_ctx);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "krb5_install_sigterm_handler failed.\n");
-        goto done;
-    }
-
     krb5_auth_ctx->child_debug_fd = -1; /* -1 means not initialized */
     ret = child_debug_init(KRB5_CHILD_LOG_FILE,
                            &krb5_auth_ctx->child_debug_fd);
diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index cd8d2a10c7..5574fdc51d 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -22,8 +22,6 @@
     along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
-#include <signal.h>
-
 #include "providers/ldap/ldap_common.h"
 #include "providers/fail_over.h"
 #include "providers/ldap/sdap_async_private.h"
@@ -151,43 +149,6 @@ static void sdap_uri_callback(void *private_data, struct fo_server *server)
     talloc_free(tmp_ctx);
 }
 
-static void sdap_finalize(struct tevent_context *ev,
-                          struct tevent_signal *se,
-                          int signum,
-                          int count,
-                          void *siginfo,
-                          void *private_data)
-{
-    orderly_shutdown(0);
-}
-
-errno_t sdap_install_sigterm_handler(TALLOC_CTX *mem_ctx,
-                                     struct tevent_context *ev,
-                                     const char *realm)
-{
-    char *sig_realm;
-    struct tevent_signal *sige;
-
-    BlockSignals(false, SIGTERM);
-
-    sig_realm = talloc_strdup(mem_ctx, realm);
-    if (sig_realm == NULL) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup failed!\n");
-        return ENOMEM;
-    }
-
-    sige = tevent_add_signal(ev, mem_ctx, SIGTERM, SA_SIGINFO, sdap_finalize,
-                             sig_realm);
-    if (sige == NULL) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "tevent_add_signal failed.\n");
-        talloc_free(sig_realm);
-        return ENOMEM;
-    }
-    talloc_steal(sige, sig_realm);
-
-    return EOK;
-}
-
 errno_t
 sdap_set_sasl_options(struct sdap_options *id_opts,
                       char *default_primary,
@@ -381,12 +342,6 @@ int sdap_gssapi_init(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    ret = sdap_install_sigterm_handler(mem_ctx, bectx->ev, krb5_realm);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_FATAL_FAILURE, "Failed to install sigterm handler\n");
-        goto done;
-    }
-
     sdap_service->kinit_service_name = talloc_strdup(sdap_service,
                                                      service->name);
     if (sdap_service->kinit_service_name == NULL) {
diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h
index 60e3ef2974..350adc7dfb 100644
--- a/src/providers/ldap/ldap_common.h
+++ b/src/providers/ldap/ldap_common.h
@@ -187,10 +187,6 @@ errno_t sdap_install_offline_callback(TALLOC_CTX *mem_ctx,
                                       const char *realm,
                                       const char *service_name);
 
-errno_t sdap_install_sigterm_handler(TALLOC_CTX *mem_ctx,
-                                     struct tevent_context *ev,
-                                     const char *realm);
-
 void sdap_remove_kdcinfo_files_callback(void *pvt);
 
 /* options parser */
diff --git a/src/util/server.c b/src/util/server.c
index 70f86e9bdc..645c03d74a 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -256,6 +256,8 @@ static void default_quit(struct tevent_context *ev,
                          void *siginfo,
                          void *private_data)
 {
+    struct main_context *ctx = talloc_get_type(private_data, struct main_context);
+    talloc_free(ctx);
     orderly_shutdown(0);
 }
 
@@ -540,29 +542,29 @@ int server_setup(const char *name, int flags,
         return 1;
     }
 
+    ctx = talloc(event_ctx, struct main_context);
+    if (ctx == NULL) {
+        DEBUG(SSSDBG_FATAL_FAILURE, "Out of memory, aborting!\n");
+        return ENOMEM;
+    }
+
+    ctx->parent_pid = getppid();
+    ctx->event_ctx = event_ctx;
+
     /* Set up an event handler for a SIGINT */
     tes = tevent_add_signal(event_ctx, event_ctx, SIGINT, 0,
-                            default_quit, NULL);
+                            default_quit, ctx);
     if (tes == NULL) {
         return EIO;
     }
 
     /* Set up an event handler for a SIGTERM */
     tes = tevent_add_signal(event_ctx, event_ctx, SIGTERM, 0,
-                            default_quit, NULL);
+                            default_quit, ctx);
     if (tes == NULL) {
         return EIO;
     }
 
-    ctx = talloc(event_ctx, struct main_context);
-    if (ctx == NULL) {
-        DEBUG(SSSDBG_FATAL_FAILURE, "Out of memory, aborting!\n");
-        return ENOMEM;
-    }
-
-    ctx->parent_pid = getppid();
-    ctx->event_ctx = event_ctx;
-
     conf_db = talloc_asprintf(ctx, "%s/%s",
                               get_db_path(), CONFDB_FILE);
     if (conf_db == NULL) {
_______________________________________________
sssd-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/[email protected]

Reply via email to