On 08/04/2016 05:01 PM, Petr Cech wrote:
On 08/04/2016 04:35 PM, Petr Cech wrote:
Hi list,
there is the first version of patch for [1]. I need
to investigate if we have the same issue in other
*_childs.
I tested it with sleep() added into ldap_child code.
[1] https://fedorahosted.org/sssd/ticket/3106
I know, it is not perfect. For example 2 seconds between SIGTERM and
SIGKILL should be constant. I will send the second version with other
childs, but not today.
Regards
Hello all,
there is fixed patch set. There is only one minor change -- constant for
time between signals.
Is there better name for global_ccname_file_dummy? I did not want to
break the naming... but it looks really long.
I looked at code of others children. I saw the same schema at
src/providers/ad/ad_gpo_child.c:361 (temporary file renamed to solid one).
In my opinion we could fix ad_qpo_child in next patch (not the highest
priority for now) and make SIGTERM/SIGKILL more general. Maybe all child
processes could have it.
Regards
--
Petr^4 Čech
>From 1fb9c73fdb512bd0292cc9a597ca1f98aa384e54 Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Thu, 4 Aug 2016 16:25:28 +0200
Subject: [PATCH 1/2] LDAP: Adding support for SIGTERM signal
We add support for handling SIGTERM signal. If ldap_child receives
SIGTERM signal it removes temporary file.
Resolves:
https://fedorahosted.org/sssd/ticket/3106
---
src/providers/ldap/ldap_child.c | 45 +++++++++++++++++++++++++++++++++--------
1 file changed, 37 insertions(+), 8 deletions(-)
diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c
index 52c271f36cb57b774e6091e7322e4b1394c78969..69595db159b0f784b067135f9384e6ca15359540 100644
--- a/src/providers/ldap/ldap_child.c
+++ b/src/providers/ldap/ldap_child.c
@@ -33,6 +33,30 @@
#include "providers/backend.h"
#include "providers/krb5/krb5_common.h"
+char *global_ccname_file_dummy = NULL;
+
+static void sig_term_handler(int sig)
+{
+ int ret;
+
+ DEBUG(SSSDBG_FATAL_FAILURE, "Received signal [%s] [%i], shutting down\n",
+ strsignal(sig), sig);
+
+ if (global_ccname_file_dummy != NULL) {
+ ret = unlink(global_ccname_file_dummy);
+ if (ret != 0) {
+ DEBUG(SSSDBG_FATAL_FAILURE, "Unlink file [%s] failed [%i][%s]\n",
+ global_ccname_file_dummy,
+ errno, strerror(errno));
+ } else {
+ DEBUG(SSSDBG_FATAL_FAILURE, "Unlink file [%s]\n",
+ global_ccname_file_dummy);
+ }
+ }
+
+ _exit(0);
+}
+
static krb5_context krb5_error_ctx;
#define LDAP_CHILD_DEBUG(level, error) KRB5_DEBUG(level, krb5_error_ctx, error)
@@ -271,7 +295,6 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx,
int kdc_time_offset_usec;
int ret;
TALLOC_CTX *tmp_ctx;
- char *ccname_file_dummy = NULL;
char *ccname_file;
tmp_ctx = talloc_new(memctx);
@@ -396,9 +419,9 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx,
goto done;
}
- ccname_file_dummy = talloc_asprintf(tmp_ctx, "%s/ccache_%s_XXXXXX",
- DB_PATH, realm_name);
- if (ccname_file_dummy == NULL) {
+ global_ccname_file_dummy = talloc_asprintf(tmp_ctx, "%s/ccache_%s_XXXXXX",
+ DB_PATH, realm_name);
+ if (global_ccname_file_dummy == NULL) {
krberr = ENOMEM;
DEBUG(SSSDBG_CRIT_FAILURE,
"talloc_asprintf failed: %s:[%d].\n",
@@ -406,7 +429,7 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx,
goto done;
}
- ret = sss_unique_filename(tmp_ctx, ccname_file_dummy);
+ ret = sss_unique_filename(tmp_ctx, global_ccname_file_dummy);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
"sss_unique_filename failed: %s:[%d].\n",
@@ -432,7 +455,8 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx,
}
DEBUG(SSSDBG_TRACE_INTERNAL, "credentials initialized\n");
- ccname_dummy = talloc_asprintf(tmp_ctx, "FILE:%s", ccname_file_dummy);
+ ccname_dummy = talloc_asprintf(tmp_ctx, "FILE:%s",
+ global_ccname_file_dummy);
ccname = talloc_asprintf(tmp_ctx, "FILE:%s", ccname_file);
if (ccname_dummy == NULL || ccname == NULL) {
krberr = ENOMEM;
@@ -482,14 +506,16 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx,
#endif
DEBUG(SSSDBG_TRACE_INTERNAL,
- "Renaming [%s] to [%s]\n", ccname_file_dummy, ccname_file);
- ret = rename(ccname_file_dummy, ccname_file);
+ "Renaming [%s] to [%s]\n", global_ccname_file_dummy, ccname_file);
+ ret = rename(global_ccname_file_dummy, ccname_file);
if (ret == -1) {
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE,
"rename failed [%d][%s].\n", ret, strerror(ret));
goto done;
}
+ talloc_free(global_ccname_file_dummy);
+ global_ccname_file_dummy = NULL;
krberr = 0;
*ccname_out = talloc_steal(memctx, ccname);
@@ -631,6 +657,9 @@ int main(int argc, const char *argv[])
}
}
+ BlockSignals(false, SIGTERM);
+ CatchSignal(SIGTERM, sig_term_handler);
+
DEBUG(SSSDBG_TRACE_FUNC, "ldap_child started.\n");
main_ctx = talloc_new(NULL);
--
2.7.4
>From 3f97fac15c7e72fad79a54c47c91860b02bda46c Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Thu, 4 Aug 2016 16:27:40 +0200
Subject: [PATCH 2/2] LDAP: Adding SIGTERM signal before SIGKILL
We add better termination of ldap_child. If ldap_child reaches
the timeout for termination parent sents SIGTERM signal. Child
has 2 seconds for removing temporary file and exit.
If it is not sufficient there is SIGKILL send to the child.
Resolves:
https://fedorahosted.org/sssd/ticket/3106
---
src/providers/ldap/sdap_child_helpers.c | 39 +++++++++++++++++++++++++++++----
src/util/child_common.h | 2 ++
2 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/src/providers/ldap/sdap_child_helpers.c b/src/providers/ldap/sdap_child_helpers.c
index 92642e8e47e530e5aef5f78bf1c7a2427979275e..9f9c9b57a7ca5e1217d56ab560cc941432084df8 100644
--- a/src/providers/ldap/sdap_child_helpers.c
+++ b/src/providers/ldap/sdap_child_helpers.c
@@ -416,9 +416,7 @@ int sdap_get_tgt_recv(struct tevent_req *req,
return EOK;
}
-
-
-static void get_tgt_timeout_handler(struct tevent_context *ev,
+static void get_tgt_sigkill_handler(struct tevent_context *ev,
struct tevent_timer *te,
struct timeval tv, void *pvt)
{
@@ -428,7 +426,8 @@ static void get_tgt_timeout_handler(struct tevent_context *ev,
int ret;
DEBUG(SSSDBG_TRACE_ALL,
- "timeout for tgt child [%d] reached.\n", state->child->pid);
+ "timeout for sending SIGKILL to tgt child [%d] reached.\n",
+ state->child->pid);
ret = kill(state->child->pid, SIGKILL);
if (ret == -1) {
@@ -439,6 +438,38 @@ static void get_tgt_timeout_handler(struct tevent_context *ev,
tevent_req_error(req, ETIMEDOUT);
}
+static void get_tgt_timeout_handler(struct tevent_context *ev,
+ struct tevent_timer *te,
+ struct timeval tv, void *pvt)
+{
+ struct tevent_req *req = talloc_get_type(pvt, struct tevent_req);
+ struct sdap_get_tgt_state *state = tevent_req_data(req,
+ struct sdap_get_tgt_state);
+ int ret;
+
+ DEBUG(SSSDBG_TRACE_ALL,
+ "timeout for sending SIGTERM to tgt child [%d] reached.\n",
+ state->child->pid);
+
+ ret = kill(state->child->pid, SIGTERM);
+ if (ret == -1) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Sending SIGTERM failed [%d][%s].\n", errno, strerror(errno));
+ }
+
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "Setting %d seconds timeout for sending SIGKILL to tgt child\n",
+ SIGTERM_TO_SIGKILL_TIME);
+
+ tv = tevent_timeval_current_ofs(SIGTERM_TO_SIGKILL_TIME, 0);
+
+ te = tevent_add_timer(ev, req, tv, get_tgt_sigkill_handler, req);
+ if (te == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "tevent_add_timer failed.\n");
+ tevent_req_error(req, ECANCELED);
+ }
+}
+
static errno_t set_tgt_child_timeout(struct tevent_req *req,
struct tevent_context *ev,
int timeout)
diff --git a/src/util/child_common.h b/src/util/child_common.h
index 2a62869034a466b465a481286950678af73667ab..eb7623987238d5d9e9032b9bbbbef49188f5b1d2 100644
--- a/src/util/child_common.h
+++ b/src/util/child_common.h
@@ -35,6 +35,8 @@
#define IN_BUF_SIZE 512
#define CHILD_MSG_CHUNK 256
+#define SIGTERM_TO_SIGKILL_TIME 2
+
struct response {
uint8_t *buf;
size_t size;
--
2.7.4
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
