From 9781b52c91524bdaa9f8b28fff3130ebad431983 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= Date: Thu, 27 Feb 2020 03:19:00 +0100 Subject: [PATCH] Resolves: upstream#4088 - server/be: SIGTERM handling is incorrect --- ...roviders-krb5-got-rid-of-unused-code.patch | 56 +++++++++++++ ...-got-rid-of-duplicating-SIGTERM-hand.patch | 84 +++++++++++++++++++ ...il-server-improved-debug-at-shutdown.patch | 32 +++++++ sssd.spec | 8 +- 4 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 0007-providers-krb5-got-rid-of-unused-code.patch create mode 100644 0008-data_provider_be-got-rid-of-duplicating-SIGTERM-hand.patch create mode 100644 0009-util-server-improved-debug-at-shutdown.patch diff --git a/0007-providers-krb5-got-rid-of-unused-code.patch b/0007-providers-krb5-got-rid-of-unused-code.patch new file mode 100644 index 0000000..0d6fd45 --- /dev/null +++ b/0007-providers-krb5-got-rid-of-unused-code.patch @@ -0,0 +1,56 @@ +From 1d4a7ffdcf8b303a40058db49d5e1be4bfb8271a Mon Sep 17 00:00:00 2001 +From: Alexey Tikhonov +Date: Mon, 9 Dec 2019 17:20:28 +0100 +Subject: [PATCH 7/9] providers/krb5: got rid of unused code +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Reviewed-by: Michal Židek +--- + src/providers/krb5/krb5_common.c | 10 ---------- + src/providers/krb5/krb5_common.h | 7 ------- + 2 files changed, 17 deletions(-) + +diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c +index bfda561c1..5c11c347b 100644 +--- a/src/providers/krb5/krb5_common.c ++++ b/src/providers/krb5/krb5_common.c +@@ -1133,16 +1133,6 @@ void remove_krb5_info_files_callback(void *pvt) + talloc_free(ctx); + } + +-void krb5_finalize(struct tevent_context *ev, +- struct tevent_signal *se, +- int signum, +- int count, +- void *siginfo, +- void *private_data) +-{ +- orderly_shutdown(0); +-} +- + 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 cc9313115..493d12e5f 100644 +--- a/src/providers/krb5/krb5_common.h ++++ b/src/providers/krb5/krb5_common.h +@@ -196,13 +196,6 @@ int krb5_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx, + + void remove_krb5_info_files_callback(void *pvt); + +-void krb5_finalize(struct tevent_context *ev, +- struct tevent_signal *se, +- int signum, +- int count, +- void *siginfo, +- void *private_data); +- + 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, +-- +2.20.1 + diff --git a/0008-data_provider_be-got-rid-of-duplicating-SIGTERM-hand.patch b/0008-data_provider_be-got-rid-of-duplicating-SIGTERM-hand.patch new file mode 100644 index 0000000..a57b30b --- /dev/null +++ b/0008-data_provider_be-got-rid-of-duplicating-SIGTERM-hand.patch @@ -0,0 +1,84 @@ +From e41e9b37e4d3fcd8544fb6c591dafbaef0954438 Mon Sep 17 00:00:00 2001 +From: Alexey Tikhonov +Date: Mon, 9 Dec 2019 17:48:14 +0100 +Subject: [PATCH 8/9] data_provider_be: got rid of duplicating SIGTERM handler +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +It was wrong to install two libtevent SIGTERM handlers both of which did +orderly_shutdown()->exit(). Naturally only one of the handlers was executed +(as process was terminated with exit()) and libtevent docs doesn't say +anything about order of execution. But chances are, be_process_finalize() +was executed first so default_quit() was not executed and main_ctx was not +freed. + +Moreover there is just no reason to have separate be_process_finalize() +at all: default server handler default_quit() frees main_ctx. And be_ctx +is linked to main_ctx so will be freed by default handler as well. + +Resolves: https://pagure.io/SSSD/sssd/issue/4088 + +Reviewed-by: Michal Židek +--- + src/providers/data_provider_be.c | 37 -------------------------------- + 1 file changed, 37 deletions(-) + +diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c +index cfcf0268d..ce00231ff 100644 +--- a/src/providers/data_provider_be.c ++++ b/src/providers/data_provider_be.c +@@ -445,36 +445,6 @@ 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, +@@ -566,13 +536,6 @@ 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; +- } +- + req = dp_init_send(be_ctx, be_ctx->ev, be_ctx, be_ctx->uid, be_ctx->gid); + if (req == NULL) { + ret = ENOMEM; +-- +2.20.1 + diff --git a/0009-util-server-improved-debug-at-shutdown.patch b/0009-util-server-improved-debug-at-shutdown.patch new file mode 100644 index 0000000..edf634e --- /dev/null +++ b/0009-util-server-improved-debug-at-shutdown.patch @@ -0,0 +1,32 @@ +From 3f52de891cba55230730602d41c3811cf1b17d96 Mon Sep 17 00:00:00 2001 +From: Alexey Tikhonov +Date: Mon, 9 Dec 2019 18:26:56 +0100 +Subject: [PATCH 9/9] util/server: improved debug at shutdown +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Relates: https://pagure.io/SSSD/sssd/issue/4088 + +Reviewed-by: Michal Židek +--- + src/util/server.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/src/util/server.c b/src/util/server.c +index ee57ac128..33524066e 100644 +--- a/src/util/server.c ++++ b/src/util/server.c +@@ -242,7 +242,8 @@ void orderly_shutdown(int status) + kill(-getpgrp(), SIGTERM); + } + #endif +- if (status == 0) sss_log(SSS_LOG_INFO, "Shutting down"); ++ DEBUG(SSSDBG_IMPORTANT_INFO, "Shutting down (status = %d)", status); ++ sss_log(SSS_LOG_INFO, "Shutting down (status = %d)", status); + exit(status); + } + +-- +2.20.1 + diff --git a/sssd.spec b/sssd.spec index bc3617f..ede046f 100644 --- a/sssd.spec +++ b/sssd.spec @@ -36,7 +36,7 @@ Name: sssd Version: 2.2.3 -Release: 5%{?dist} +Release: 6%{?dist} Summary: System Security Services Daemon License: GPLv3+ URL: https://pagure.io/SSSD/sssd/ @@ -49,6 +49,9 @@ Patch0003: 0003-INI-sssctl-config-check-command-error-messages.patch Patch0004: 0004-certmap-mention-special-regex-characters-in-man-page.patch Patch0005: 0005-ldap_child-do-not-try-PKINIT.patch Patch0006: 0006-util-watchdog-fixed-watchdog-implementation.patch +Patch0007: 0007-providers-krb5-got-rid-of-unused-code.patch +Patch0008: 0008-data_provider_be-got-rid-of-duplicating-SIGTERM-hand.patch +Patch0009: 0009-util-server-improved-debug-at-shutdown.patch ### Downstream only patches ### Patch0502: 0502-SYSTEMD-Use-capabilities.patch @@ -1077,6 +1080,9 @@ fi %{_libdir}/%{name}/modules/libwbclient.so %changelog +* Wed Feb 26 2020 Michal Židek - 2.2.3-6 +- Resolves: upstream#4088 - server/be: SIGTERM handling is incorrect + * Wed Feb 26 2020 Michal Židek - 2.2.3-5 - Resolves: upstream##4089 Watchdog implementation or usage is incorrect