From e74dc34832e84919b1b2cde3c11bd56bd5fdc610 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina@redhat.com>
Date: Mon, 24 Jun 2013 14:53:27 +0200
Subject: [PATCH 2/2] SIGCHLD handler: do not call callback when pvt data
 where freed

https://fedorahosted.org/sssd/ticket/1992
---
 src/providers/dp_dyndns.c               |  6 +++++-
 src/providers/krb5/krb5_child_handler.c |  2 +-
 src/providers/ldap/sdap_child_helpers.c |  2 +-
 src/util/child_common.c                 | 24 +++++++++++++++++++++++-
 src/util/child_common.h                 |  8 +++++++-
 5 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/src/providers/dp_dyndns.c b/src/providers/dp_dyndns.c
index 3467683a3f272a2e2b75e8f8890f1a6bcf22d961..488c2a61434a692dfd5d64115a0cafd6a986b16e 100644
--- a/src/providers/dp_dyndns.c
+++ b/src/providers/dp_dyndns.c
@@ -764,6 +764,7 @@ nsupdate_get_addrs_recv(struct tevent_req *req,
 struct nsupdate_child_state {
     int pipefd_to_child;
     struct tevent_timer *timeout_handler;
+    struct sss_child_ctx_old *child_ctx;
 
     int child_status;
 };
@@ -799,7 +800,8 @@ nsupdate_child_send(TALLOC_CTX *mem_ctx,
     state->pipefd_to_child = pipefd_to_child;
 
     /* Set up SIGCHLD handler */
-    ret = child_handler_setup(ev, child_pid, nsupdate_child_handler, req);
+    ret = child_handler_setup(ev, child_pid, nsupdate_child_handler, req,
+                              &state->child_ctx);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, ("Could not set up child handlers [%d]: %s\n",
               ret, sss_strerror(ret)));
@@ -847,6 +849,8 @@ nsupdate_child_timeout(struct tevent_context *ev,
             tevent_req_data(req, struct nsupdate_child_state);
 
     DEBUG(SSSDBG_CRIT_FAILURE, ("Timeout reached for dynamic DNS update\n"));
+    child_handler_destroy(state->child_ctx);
+    state->child_ctx = NULL;
     state->child_status = ETIMEDOUT;
     tevent_req_error(req, ERR_DYNDNS_TIMEOUT);
 }
diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c
index 1da7e4f422d68510b50df5415017e4118fac57fd..dda53ab1663f39ba8c5964d618b6f43763c83c88 100644
--- a/src/providers/krb5/krb5_child_handler.c
+++ b/src/providers/krb5/krb5_child_handler.c
@@ -329,7 +329,7 @@ static errno_t fork_child(struct tevent_req *req)
         fd_nonblocking(state->io->read_from_child_fd);
         fd_nonblocking(state->io->write_to_child_fd);
 
-        ret = child_handler_setup(state->ev, pid, NULL, NULL);
+        ret = child_handler_setup(state->ev, pid, NULL, NULL, NULL);
         if (ret != EOK) {
             DEBUG(1, ("Could not set up child signal handler\n"));
             return ret;
diff --git a/src/providers/ldap/sdap_child_helpers.c b/src/providers/ldap/sdap_child_helpers.c
index fd4d10f469c565af72441babcb365b2677ff28d4..e57313442fec1723e3cbcc186d9720b7e9a6ce89 100644
--- a/src/providers/ldap/sdap_child_helpers.c
+++ b/src/providers/ldap/sdap_child_helpers.c
@@ -119,7 +119,7 @@ static errno_t sdap_fork_child(struct tevent_context *ev,
         fd_nonblocking(child->read_from_child_fd);
         fd_nonblocking(child->write_to_child_fd);
 
-        ret = child_handler_setup(ev, pid, NULL, NULL);
+        ret = child_handler_setup(ev, pid, NULL, NULL, NULL);
         if (ret != EOK) {
             return ret;
         }
diff --git a/src/util/child_common.c b/src/util/child_common.c
index a42d3a420dc87af3e26ba036bdd867a50b761490..ec2baf0192cf46cda72b596a4def0ceb77880005 100644
--- a/src/util/child_common.c
+++ b/src/util/child_common.c
@@ -262,7 +262,8 @@ struct sss_child_ctx_old {
 };
 
 int child_handler_setup(struct tevent_context *ev, int pid,
-                        sss_child_callback_t cb, void *pvt)
+                        sss_child_callback_t cb, void *pvt,
+                        struct sss_child_ctx_old **_child_ctx)
 {
     struct sss_child_ctx_old *child_ctx;
 
@@ -286,9 +287,30 @@ int child_handler_setup(struct tevent_context *ev, int pid,
     child_ctx->pvt = pvt;
 
     DEBUG(8, ("Signal handler set up for pid [%d]\n", pid));
+
+    if (_child_ctx != NULL) {
+        *_child_ctx = child_ctx;
+    }
+
     return EOK;
 }
 
+void child_handler_destroy(struct sss_child_ctx_old *ctx)
+{
+    errno_t ret;
+
+    /* We still want to wait for the child to finish, but the caller is not
+     * interested in the result anymore (e.g. timeout was reached). */
+    ctx->cb = NULL;
+    ctx->pvt = NULL;
+
+    ret = kill(ctx->pid, SIGKILL);
+    if (ret == -1) {
+        ret = errno;
+        DEBUG(SSSDBG_MINOR_FAILURE, ("kill failed [%d][%s].\n", ret, strerror(ret)));
+    }
+}
+
 /* Async communication with the child process via a pipe */
 
 struct write_pipe_state {
diff --git a/src/util/child_common.h b/src/util/child_common.h
index 237969f2ffb46b7e903551c67db754315427f05b..95865bb529b6b282a57b138d7a85ce93649faa2e 100644
--- a/src/util/child_common.h
+++ b/src/util/child_common.h
@@ -78,9 +78,15 @@ typedef void (*sss_child_callback_t)(int child_status,
                                      struct tevent_signal *sige,
                                      void *pvt);
 
+struct sss_child_ctx_old;
+
 /* Set up child termination signal handler */
 int child_handler_setup(struct tevent_context *ev, int pid,
-                        sss_child_callback_t cb, void *pvt);
+                        sss_child_callback_t cb, void *pvt,
+                        struct sss_child_ctx_old **_child_ctx);
+
+/* Destroy child termination signal handler */
+void child_handler_destroy(struct sss_child_ctx_old *ctx);
 
 /* Async communication with the child process via a pipe */
 struct tevent_req *write_pipe_send(TALLOC_CTX *mem_ctx,
-- 
1.7.11.7

