Resolves: upstream#4088 - server/be: SIGTERM handling is incorrect

This commit is contained in:
Michal Židek 2020-02-27 03:19:00 +01:00
parent 6c1563e282
commit 9781b52c91
4 changed files with 179 additions and 1 deletions

View File

@ -0,0 +1,56 @@
From 1d4a7ffdcf8b303a40058db49d5e1be4bfb8271a Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikhono@redhat.com>
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 <mzidek@redhat.com>
---
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

View File

@ -0,0 +1,84 @@
From e41e9b37e4d3fcd8544fb6c591dafbaef0954438 Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikhono@redhat.com>
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 <mzidek@redhat.com>
---
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

View File

@ -0,0 +1,32 @@
From 3f52de891cba55230730602d41c3811cf1b17d96 Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikhono@redhat.com>
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 <mzidek@redhat.com>
---
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

View File

@ -36,7 +36,7 @@
Name: sssd Name: sssd
Version: 2.2.3 Version: 2.2.3
Release: 5%{?dist} Release: 6%{?dist}
Summary: System Security Services Daemon Summary: System Security Services Daemon
License: GPLv3+ License: GPLv3+
URL: https://pagure.io/SSSD/sssd/ 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 Patch0004: 0004-certmap-mention-special-regex-characters-in-man-page.patch
Patch0005: 0005-ldap_child-do-not-try-PKINIT.patch Patch0005: 0005-ldap_child-do-not-try-PKINIT.patch
Patch0006: 0006-util-watchdog-fixed-watchdog-implementation.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 ### ### Downstream only patches ###
Patch0502: 0502-SYSTEMD-Use-capabilities.patch Patch0502: 0502-SYSTEMD-Use-capabilities.patch
@ -1077,6 +1080,9 @@ fi
%{_libdir}/%{name}/modules/libwbclient.so %{_libdir}/%{name}/modules/libwbclient.so
%changelog %changelog
* Wed Feb 26 2020 Michal Židek <mzidek@redhat.com> - 2.2.3-6
- Resolves: upstream#4088 - server/be: SIGTERM handling is incorrect
* Wed Feb 26 2020 Michal Židek <mzidek@redhat.com> - 2.2.3-5 * Wed Feb 26 2020 Michal Židek <mzidek@redhat.com> - 2.2.3-5
- Resolves: upstream##4089 Watchdog implementation or usage is incorrect - Resolves: upstream##4089 Watchdog implementation or usage is incorrect