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 941aa936d773f465a26ab64738538d104e34f7a4 Mon Sep 17 00:00:00 2001 From: Tomas Halman <[email protected]> Date: Thu, 15 Aug 2019 15:19:03 +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 Resolves: https://pagure.io/SSSD/sssd/issue/XXXX --- src/providers/data_provider_be.c | 37 ++++++++++++++++++++++ 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, 50 insertions(+), 103 deletions(-) diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index 147627b798..a4d99798fc 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, @@ -545,6 +575,13 @@ errno_t be_process_init(TALLOC_CTX *mem_ctx, goto done; } + /* install signal handler */ + ret = be_process_install_sigterm_handler(be_ctx); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, "be_install_sigterm_handler failed.\n"); + goto done; + } + refresh_interval = be_ctx->domain->refresh_expired_interval; if (refresh_interval > 0) { ret = be_ptask_create(be_ctx, be_ctx, refresh_interval, 30, 5, 0, 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 441c52b342..cf94654877 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 5d6302dcd1..5e90d3ae6c 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]
