Add common SIGCHLD handling for providers.

5 files changed, 269 insertions, 8 deletions

diff --git a/Makefile.am b/Makefile.am
index a3eb05cc0..7ebf29f46 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -461,6 +461,7 @@ sssd_sudo_LDADD = \
 endif
 
 sssd_be_SOURCES = \
+    src/providers/child_common.c \
     src/providers/data_provider_be.c \
     src/providers/data_provider_fo.c \
     src/providers/data_provider_opts.c \
@@ -1028,7 +1029,9 @@ krb5_child_SOURCES = \
     src/providers/child_common.c \
     src/providers/dp_pam_data_util.c \
     src/util/user_info_msg.c \
-    src/util/sss_krb5.c
+    src/util/sss_krb5.c \
+    src/util/util.c \
+    src/util/signal.c
 krb5_child_CFLAGS = \
     $(AM_CFLAGS) \
     $(POPT_CFLAGS) \
@@ -1038,12 +1041,15 @@ krb5_child_LDADD = \
     $(TALLOC_LIBS) \
     $(TEVENT_LIBS) \
     $(POPT_LIBS) \
+    $(DHASH_LIBS) \
     $(KRB5_LIBS)
 
 ldap_child_SOURCES = \
     src/providers/ldap/ldap_child.c \
     src/providers/child_common.c \
-    src/util/sss_krb5.c
+    src/util/sss_krb5.c \
+    src/util/util.c \
+    src/util/signal.c
 ldap_child_CFLAGS = \
     $(AM_CFLAGS) \
     $(POPT_CFLAGS) \
@@ -1054,6 +1060,7 @@ ldap_child_LDADD = \
     $(TEVENT_LIBS) \
     $(POPT_LIBS) \
     $(OPENLDAP_LIBS) \
+    $(DHASH_LIBS) \
     $(KRB5_LIBS)
 
 proxy_child_SOURCES = \
diff --git a/src/providers/child_common.c b/src/providers/child_common.c
index 5920ebc74..34602a41f 100644
--- a/src/providers/child_common.c
+++ b/src/providers/child_common.c
@@ -33,7 +33,225 @@
 #include "db/sysdb.h"
 #include "providers/child_common.h"
 
+struct sss_sigchild_ctx {
+    struct tevent_context *ev;
+    hash_table_t *children;
+    int options;
+};
+
 struct sss_child_ctx {
+    pid_t pid;
+    sss_child_fn_t cb;
+    void *pvt;
+    struct sss_sigchild_ctx *sigchld_ctx;
+};
+
+errno_t sss_sigchld_init(TALLOC_CTX *mem_ctx,
+                         struct tevent_context *ev,
+                         struct sss_sigchild_ctx **child_ctx)
+{
+    errno_t ret;
+    struct sss_sigchild_ctx *sigchld_ctx;
+    struct tevent_signal *tes;
+
+    sigchld_ctx = talloc_zero(mem_ctx, struct sss_sigchild_ctx);
+    if (!sigchld_ctx) {
+        DEBUG(0, ("fatal error initializing sss_sigchild_ctx\n"));
+        return ENOMEM;
+    }
+    sigchld_ctx->ev = ev;
+
+    ret = sss_hash_create(sigchld_ctx, 10, &sigchld_ctx->children);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_FATAL_FAILURE,
+              ("fatal error initializing children hash table: [%s]\n",
+               strerror(ret)));
+        talloc_free(sigchld_ctx);
+        return ret;
+    }
+
+    BlockSignals(false, SIGCHLD);
+    tes = tevent_add_signal(ev, sigchld_ctx, SIGCHLD, SA_SIGINFO,
+                            sss_child_handler, sigchld_ctx);
+    if (tes == NULL) {
+        talloc_free(sigchld_ctx);
+        return EIO;
+    }
+
+    *child_ctx = sigchld_ctx;
+    return EOK;
+}
+
+static int sss_child_destructor(void *ptr)
+{
+    struct sss_child_ctx *child_ctx;
+    hash_key_t key;
+    int error;
+
+    child_ctx = talloc_get_type(ptr, struct sss_child_ctx);
+    key.type = HASH_KEY_ULONG;
+    key.ul = child_ctx->pid;
+
+    error = hash_delete(child_ctx->sigchld_ctx->children, &key);
+    if (error != HASH_SUCCESS && error != HASH_ERROR_KEY_NOT_FOUND) {
+        DEBUG(SSSDBG_TRACE_INTERNAL,
+              ("failed to delete child_ctx from hash table [%d]: %s\n",
+               error, hash_error_string(error)));
+    }
+
+    return 0;
+}
+
+errno_t sss_child_register(TALLOC_CTX *mem_ctx,
+                           struct sss_sigchild_ctx *sigchld_ctx,
+                           pid_t pid,
+                           sss_child_fn_t cb,
+                           void *pvt,
+                           struct sss_child_ctx **child_ctx)
+{
+    struct sss_child_ctx *child;
+    hash_key_t key;
+    hash_value_t value;
+    int error;
+
+    child = talloc_zero(mem_ctx, struct sss_child_ctx);
+    if (child == NULL) {
+        return ENOMEM;
+    }
+
+    child->pid = pid;
+    child->cb = cb;
+    child->pvt = pvt;
+    child->sigchld_ctx = sigchld_ctx;
+
+    key.type = HASH_KEY_ULONG;
+    key.ul = pid;
+
+    value.type = HASH_VALUE_PTR;
+    value.ptr = child;
+
+    error = hash_enter(sigchld_ctx->children, &key, &value);
+    if (error != HASH_SUCCESS) {
+        talloc_free(child);
+        return ENOMEM;
+    }
+
+    talloc_set_destructor((TALLOC_CTX *) child, sss_child_destructor);
+
+    *child_ctx = child;
+    return EOK;
+}
+
+struct sss_child_cb_pvt {
+    struct sss_child_ctx *child_ctx;
+    int wait_status;
+};
+
+static void sss_child_invoke_cb(struct tevent_context *ev,
+                                struct tevent_immediate *imm,
+                                void *pvt)
+{
+    struct sss_child_cb_pvt *cb_pvt;
+    struct sss_child_ctx *child_ctx;
+    hash_key_t key;
+    int error;
+
+    cb_pvt = talloc_get_type(pvt, struct sss_child_cb_pvt);
+    child_ctx = cb_pvt->child_ctx;
+
+    key.type = HASH_KEY_ULONG;
+    key.ul = child_ctx->pid;
+
+    error = hash_delete(child_ctx->sigchld_ctx->children, &key);
+    if (error != HASH_SUCCESS && error != HASH_ERROR_KEY_NOT_FOUND) {
+        DEBUG(SSSDBG_OP_FAILURE,
+              ("failed to delete child_ctx from hash table [%d]: %s\n",
+               error, hash_error_string(error)));
+    }
+
+    if (child_ctx->cb) {
+        child_ctx->cb(child_ctx->pid, cb_pvt->wait_status, child_ctx->pvt);
+    }
+}
+
+void sss_child_handler(struct tevent_context *ev,
+                       struct tevent_signal *se,
+                       int signum,
+                       int count,
+                       void *siginfo,
+                       void *private_data)
+{
+    struct sss_sigchild_ctx *sigchld_ctx;
+    struct tevent_immediate *imm;
+    struct sss_child_cb_pvt *invoke_pvt;
+    struct sss_child_ctx *child_ctx;
+    hash_key_t key;
+    hash_value_t value;
+    int error;
+    int wait_status;
+    pid_t pid;
+
+    sigchld_ctx = talloc_get_type(private_data, struct sss_sigchild_ctx);
+    key.type = HASH_KEY_ULONG;
+
+    do {
+        do {
+            errno = 0;
+            pid = waitpid(-1, &wait_status, WNOHANG | sigchld_ctx->options);
+        } while (pid == -1 && errno == EINTR);
+
+        if (pid == -1) {
+            DEBUG(SSSDBG_TRACE_INTERNAL,
+                  ("waitpid failed [%d]: %s\n", errno, strerror(errno)));
+            return;
+        } else if (pid == 0) continue;
+
+        key.ul = pid;
+        error = hash_lookup(sigchld_ctx->children, &key, &value);
+        if (error == HASH_SUCCESS) {
+            child_ctx = talloc_get_type(value.ptr, struct sss_child_ctx);
+
+            imm = tevent_create_immediate(sigchld_ctx->ev);
+            if (imm == NULL) {
+                DEBUG(SSSDBG_CRIT_FAILURE,
+                      ("Out of memory invoking SIGCHLD callback\n"));
+                return;
+            }
+
+            invoke_pvt = talloc_zero(child_ctx, struct sss_child_cb_pvt);
+            if (invoke_pvt == NULL) {
+                DEBUG(SSSDBG_CRIT_FAILURE,
+                      ("out of memory invoking SIGCHLD callback\n"));
+                return;
+            }
+            invoke_pvt->child_ctx = child_ctx;
+            invoke_pvt->wait_status = wait_status;
+
+            tevent_schedule_immediate(imm, sigchld_ctx->ev,
+                                      sss_child_invoke_cb, invoke_pvt);
+        } else if (error == HASH_ERROR_KEY_NOT_FOUND) {
+            DEBUG(SSSDBG_TRACE_LIBS,
+                 ("BUG: waitpid() returned [%d] but it was not in the table. "
+                  "This could be due to a linked library creating processes "
+                  "without registering them with the sigchld handler\n",
+                  pid));
+            /* We will simply ignore this and return to the loop
+             * This will prevent a zombie, but may cause unexpected
+             * behavior in the code that was trying to handle this
+             * pid.
+             */
+        } else {
+            DEBUG(SSSDBG_OP_FAILURE,
+                  ("SIGCHLD hash table error [%d]: %s\n",
+                   error, hash_error_string(error)));
+            /* This is bad, but we should try to check for other
+             * children anyway, to avoid potential zombies.
+             */
+        }
+    } while (pid != 0);
+}
+
+struct sss_child_ctx_old {
     struct tevent_signal *sige;
     pid_t pid;
     int child_status;
@@ -44,11 +262,11 @@ struct sss_child_ctx {
 int child_handler_setup(struct tevent_context *ev, int pid,
                         sss_child_callback_t cb, void *pvt)
 {
-    struct sss_child_ctx *child_ctx;
+    struct sss_child_ctx_old *child_ctx;
 
     DEBUG(8, ("Setting up signal handler up for pid [%d]\n", pid));
 
-    child_ctx = talloc_zero(ev, struct sss_child_ctx);
+    child_ctx = talloc_zero(ev, struct sss_child_ctx_old);
     if (child_ctx == NULL) {
         return ENOMEM;
     }
@@ -300,7 +518,7 @@ void child_sig_handler(struct tevent_context *ev,
                        int count, void *__siginfo, void *pvt)
 {
     int ret, err;
-    struct sss_child_ctx *child_ctx;
+    struct sss_child_ctx_old *child_ctx;
     struct tevent_immediate *imm;
 
     if (count <= 0) {
@@ -308,7 +526,7 @@ void child_sig_handler(struct tevent_context *ev,
         return;
     }
 
-    child_ctx = talloc_get_type(pvt, struct sss_child_ctx);
+    child_ctx = talloc_get_type(pvt, struct sss_child_ctx_old);
     DEBUG(7, ("Waiting for child [%d].\n", child_ctx->pid));
 
     errno = 0;
@@ -363,8 +581,8 @@ static void child_invoke_callback(struct tevent_context *ev,
                                   struct tevent_immediate *imm,
                                   void *pvt)
 {
-    struct sss_child_ctx *child_ctx =
-            talloc_get_type(pvt, struct sss_child_ctx);
+    struct sss_child_ctx_old *child_ctx =
+            talloc_get_type(pvt, struct sss_child_ctx_old);
     if (child_ctx->cb) {
         child_ctx->cb(child_ctx->child_status, child_ctx->sige, child_ctx->pvt);
     }
diff --git a/src/providers/child_common.h b/src/providers/child_common.h
index 22a77dbbe..1e9f1b6c1 100644
--- a/src/providers/child_common.h
+++ b/src/providers/child_common.h
@@ -45,6 +45,31 @@ struct io_buffer {
     size_t size;
 };
 
+/* COMMON SIGCHLD HANDLING */
+typedef void (*sss_child_fn_t)(int pid, int wait_status, void *pvt);
+
+struct sss_sigchild_ctx;
+struct sss_child_ctx;
+
+/* Create a new child context to manage callbacks */
+errno_t sss_sigchld_init(TALLOC_CTX *mem_ctx,
+                         struct tevent_context *ev,
+                         struct sss_sigchild_ctx **child_ctx);
+
+errno_t sss_child_register(TALLOC_CTX *mem_ctx,
+                           struct sss_sigchild_ctx *sigchld_ctx,
+                           pid_t pid,
+                           sss_child_fn_t cb,
+                           void *pvt,
+                           struct sss_child_ctx **child_ctx);
+
+void sss_child_handler(struct tevent_context *ev,
+                       struct tevent_signal *se,
+                       int signum,
+                       int count,
+                       void *siginfo,
+                       void *private_data);
+
 /* Callback to be invoked when a sigchld handler is called.
  * The tevent_signal * associated with the handler will be
  * freed automatically when this function returns.
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index 2de986cc0..fb4da87c4 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -42,6 +42,7 @@
 #include "sbus/sssd_dbus.h"
 #include "providers/dp_backend.h"
 #include "providers/fail_over.h"
+#include "providers/child_common.h"
 #include "resolv/async_resolv.h"
 #include "monitor/monitor_interfaces.h"
 
@@ -1334,6 +1335,14 @@ int be_process_init(TALLOC_CTX *mem_ctx,
         return EIO;
     }
 
+    ret = sss_sigchld_init(ctx, ctx->ev, &ctx->sigchld_ctx);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_FATAL_FAILURE,
+              ("Could not initialize sigchld context: [%s]\n",
+               strerror(ret)));
+        return ret;
+    }
+
     return EOK;
 }
 
diff --git a/src/providers/dp_backend.h b/src/providers/dp_backend.h
index ef9fe0d48..3d1c6bf25 100644
--- a/src/providers/dp_backend.h
+++ b/src/providers/dp_backend.h
@@ -24,6 +24,7 @@
 
 #include "providers/data_provider.h"
 #include "providers/fail_over.h"
+#include "providers/child_common.h"
 #include "db/sysdb.h"
 
 /* a special token, if used in place of the hostname, denotes that real
@@ -94,6 +95,7 @@ struct be_ctx {
     const char *identity;
     const char *conf_path;
     struct be_failover_ctx *be_fo;
+    struct sss_sigchild_ctx *sigchld_ctx;
 
     /* Functions to be invoked when the
      * backend goes online or offline