From 6468d9f2bb30147967a724075c504afb3481d465 Mon Sep 17 00:00:00 2001

From: Jakub Hrozek <jhrozek@redhat.com>

Date: Mon, 1 Dec 2014 12:25:24 +0100

Subject: [PATCH 127/128] Add extra_args to exec_child()

Related:

    https://fedorahosted.org/sssd/ticket/2503

Currently all child processes use the same arguments, the construction

of argv[] is even hardcoded in exec_child(). Add an extra_args[] array

that extends the common set of argvs so that we can have child-specific

arguments. Also adds a unit test.

Reviewed-by: Sumit Bose <sbose@redhat.com>

---

 src/providers/ad/ad_gpo.c               |  2 +-

 src/providers/ipa/ipa_selinux.c         |  3 ++-

 src/providers/krb5/krb5_child_handler.c |  3 ++-

 src/providers/ldap/sdap_child_helpers.c |  3 ++-

 src/tests/cmocka/test_child.c           | 31 ++++++++++++----------

 src/tests/cmocka/test_child_common.c    | 47 ++++++++++++++++++++++++++++++++-

 src/util/child_common.c                 | 23 ++++++++++++++--

 src/util/child_common.h                 |  3 ++-

 8 files changed, 93 insertions(+), 22 deletions(-)

diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c

index 83edbe4fb5a7e617cab95c0330ceae31392b18b2..62715861c91484fa2a57e7cc13ba403c9096d9a7 100644

--- a/src/providers/ad/ad_gpo.c

+++ b/src/providers/ad/ad_gpo.c

@@ -3960,7 +3960,7 @@ gpo_fork_child(struct tevent_req *req)

     if (pid == 0) { /* child */

         err = exec_child(state,

                          pipefd_to_child, pipefd_from_child,

-                         GPO_CHILD, gpo_child_debug_fd);

+                         GPO_CHILD, gpo_child_debug_fd, NULL);

         DEBUG(SSSDBG_CRIT_FAILURE, "Could not exec gpo_child: [%d][%s].\n",

               err, strerror(err));

         return err;

diff --git a/src/providers/ipa/ipa_selinux.c b/src/providers/ipa/ipa_selinux.c

index 30ad6f0a7c4622ca5eb9a75ae4f57183543515c6..531258dac5c033b5896598e44e28a373d6cf5e3b 100644

--- a/src/providers/ipa/ipa_selinux.c

+++ b/src/providers/ipa/ipa_selinux.c

@@ -1036,7 +1036,8 @@ static errno_t selinux_fork_child(struct selinux_child_state *state)

     if (pid == 0) { /* child */

         ret = exec_child(state,

                          pipefd_to_child, pipefd_from_child,

-                         SELINUX_CHILD, selinux_child_debug_fd);

+                         SELINUX_CHILD, selinux_child_debug_fd,

+                         NULL);

         DEBUG(SSSDBG_CRIT_FAILURE, "Could not exec selinux_child: [%d][%s].\n",

               ret, sss_strerror(ret));

         return ret;

diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c

index 93961172c7a3a5d8f2a4fb320370037f188b5909..9bb61f65437694d8aa2109513b5f061dfc9ee21c 100644

--- a/src/providers/krb5/krb5_child_handler.c

+++ b/src/providers/krb5/krb5_child_handler.c

@@ -299,7 +299,8 @@ static errno_t fork_child(struct tevent_req *req)

     if (pid == 0) { /* child */

         err = exec_child(state,

                          pipefd_to_child, pipefd_from_child,

-                         KRB5_CHILD, state->kr->krb5_ctx->child_debug_fd);

+                         KRB5_CHILD, state->kr->krb5_ctx->child_debug_fd,

+                         NULL);

         if (err != EOK) {

             DEBUG(SSSDBG_CRIT_FAILURE, "Could not exec KRB5 child: [%d][%s].\n",

                       err, strerror(err));

diff --git a/src/providers/ldap/sdap_child_helpers.c b/src/providers/ldap/sdap_child_helpers.c

index 40010989021eb7cf77b96876b2d1c4119ed39163..b60891d2b41f9a359856eb22174128d7f07559fb 100644

--- a/src/providers/ldap/sdap_child_helpers.c

+++ b/src/providers/ldap/sdap_child_helpers.c

@@ -108,7 +108,8 @@ static errno_t sdap_fork_child(struct tevent_context *ev,

     if (pid == 0) { /* child */

         err = exec_child(child,

                          pipefd_to_child, pipefd_from_child,

-                         LDAP_CHILD, ldap_child_debug_fd);

+                         LDAP_CHILD, ldap_child_debug_fd,

+                         NULL);

         DEBUG(SSSDBG_CRIT_FAILURE, "Could not exec LDAP child: [%d][%s].\n",

                                     err, strerror(err));

         return err;

diff --git a/src/tests/cmocka/test_child.c b/src/tests/cmocka/test_child.c

index a857afce1fe7c4f3949fe77fe4e0a24e15961a4f..0bb50a4e005a26b1fbd332b17558d1284de77ea1 100644

--- a/src/tests/cmocka/test_child.c

+++ b/src/tests/cmocka/test_child.c

@@ -35,13 +35,9 @@ int main(int argc, const char *argv[])

     int opt;

     int debug_fd = -1;

     poptContext pc;

-    const char *action;

-    ssize_t len = 0;

-    ssize_t written;

-    errno_t ret;

-    uint8_t *buf[IN_BUF_SIZE];

-    uid_t uid;

-    gid_t gid;

+    const char *action = NULL;

+    const char *guitar;

+    const char *drums;

     struct poptOption long_options[] = {

         POPT_AUTOHELP

@@ -54,8 +50,9 @@ int main(int argc, const char *argv[])

         {"debug-fd", 0, POPT_ARG_INT, &debug_fd, 0,

          _("An open file descriptor for the debug logs"), NULL},

         {"debug-to-stderr", 0, POPT_ARG_NONE | POPT_ARGFLAG_DOC_HIDDEN, &debug_to_stderr, 0, \

-         _("Send the debug output to stderr directly."), NULL }, \

-        SSSD_SERVER_OPTS(uid, gid)

+         _("Send the debug output to stderr directly."), NULL },

+        {"guitar", 0, POPT_ARG_STRING, &guitar, 0, _("Who plays guitar"), NULL },

+        {"drums", 0, POPT_ARG_STRING, &drums, 0, _("Who plays drums"), NULL },

         POPT_TABLEEND

     };

@@ -73,11 +70,17 @@ int main(int argc, const char *argv[])

         }

     }

+    action = getenv("TEST_CHILD_ACTION");

+    if (action) {

+        if (strcasecmp(action, "check_extra_args") == 0) {

+            if (!(strcmp(guitar, "george") == 0 \

+                        && strcmp(drums, "ringo") == 0)) {

+                DEBUG(SSSDBG_CRIT_FAILURE, "This band sounds weird\n");

+                _exit(1);

+            }

+        }

+    }

+

     DEBUG(SSSDBG_TRACE_FUNC, "test_child completed successfully\n");

     _exit(0);

-

-fail:

-    DEBUG(SSSDBG_TRACE_FUNC, "test_child completed successfully\n");

-    close(STDOUT_FILENO);

-    _exit(-1);

 }

diff --git a/src/tests/cmocka/test_child_common.c b/src/tests/cmocka/test_child_common.c

index f2cd8c0081c2e85432db1c9696102780dc990e75..112ed0ad97294bc45eac7c2124155e6b1908ad92 100644

--- a/src/tests/cmocka/test_child_common.c

+++ b/src/tests/cmocka/test_child_common.c

@@ -89,7 +89,49 @@ void test_exec_child(void **state)

         ret = exec_child(child_tctx,

                          child_tctx->pipefd_to_child,

                          child_tctx->pipefd_from_child,

-                         CHILD_DIR"/"TEST_BIN, 2);

+                         CHILD_DIR"/"TEST_BIN, 2, NULL);

+        assert_int_equal(ret, EOK);

+    } else {

+            do {

+                errno = 0;

+                ret = waitpid(child_pid, &status, 0);

+            } while (ret == -1 && errno == EINTR);

+

+            if (ret > 0) {

+                ret = EIO;

+                if (WIFEXITED(status)) {

+                    ret = WEXITSTATUS(status);

+                    assert_int_equal(ret, 0);

+                }

+            } else {

+                DEBUG(SSSDBG_FUNC_DATA,

+                    "Failed to wait for children %d\n", child_pid);

+                ret = EIO;

+            }

+    }

+}

+

+/* Just make sure the exec works. The child does nothing but exits */

+void test_exec_child_extra_args(void **state)

+{

+    errno_t ret;

+    pid_t child_pid;

+    int status;

+    struct child_test_ctx *child_tctx = talloc_get_type(*state,

+                                                        struct child_test_ctx);

+    const char *extra_args[] = { "--guitar=george",

+                                 "--drums=ringo",

+                                 NULL };

+

+    setenv("TEST_CHILD_ACTION", "check_extra_args", 1);

+

+    child_pid = fork();

+    assert_int_not_equal(child_pid, -1);

+    if (child_pid == 0) {

+        ret = exec_child(child_tctx,

+                         child_tctx->pipefd_to_child,

+                         child_tctx->pipefd_from_child,

+                         CHILD_DIR"/"TEST_BIN, 2, extra_args);

         assert_int_equal(ret, EOK);

     } else {

             do {

@@ -126,6 +168,9 @@ int main(int argc, const char *argv[])

         unit_test_setup_teardown(test_exec_child,

                                  child_test_setup,

                                  child_test_teardown),

+        unit_test_setup_teardown(test_exec_child_extra_args,

+                                 child_test_setup,

+                                 child_test_teardown),

     };

     /* Set debug level to invalid value so we can deside if -d 0 was used. */

diff --git a/src/util/child_common.c b/src/util/child_common.c

index cc6a8fa758bfa6efa511c28cd9121e759b590342..9710630f9773ae02258e4f0dd609a3d74978c8f4 100644

--- a/src/util/child_common.c

+++ b/src/util/child_common.c

@@ -623,6 +623,7 @@ static void child_invoke_callback(struct tevent_context *ev,

 static errno_t prepare_child_argv(TALLOC_CTX *mem_ctx,

                                   int child_debug_fd,

                                   const char *binary,

+                                  const char *extra_argv[],

                                   char ***_argv)

 {

     /*

@@ -632,6 +633,7 @@ static errno_t prepare_child_argv(TALLOC_CTX *mem_ctx,

     uint_t argc = 5;

     char ** argv;

     errno_t ret = EINVAL;

+    size_t i;

     /* Save the current state in case an interrupt changes it */

     bool child_debug_to_file = debug_to_file;

@@ -642,6 +644,10 @@ static errno_t prepare_child_argv(TALLOC_CTX *mem_ctx,

     if (child_debug_to_file) argc++;

     if (child_debug_stderr) argc++;

+    if (extra_argv) {

+        for (i = 0; extra_argv[i]; i++) argc++;

+    }

+

     /*

      * program name, debug_level, debug_to_file, debug_timestamps,

      * debug_microseconds and NULL

@@ -654,6 +660,17 @@ static errno_t prepare_child_argv(TALLOC_CTX *mem_ctx,

     argv[--argc] = NULL;

+    /* Add extra_attrs first */

+    if (extra_argv) {

+        for (i = 0; extra_argv[i]; i++) {

+            argv[--argc] = talloc_strdup(argv, extra_argv[i]);

+            if (argv[argc] == NULL) {

+                ret = ENOMEM;

+                goto fail;

+            }

+        }

+    }

+

     argv[--argc] = talloc_asprintf(argv, "--debug-level=%#.4x",

                               debug_level);

     if (argv[argc] == NULL) {

@@ -714,7 +731,8 @@ fail:

 errno_t exec_child(TALLOC_CTX *mem_ctx,

                    int *pipefd_to_child, int *pipefd_from_child,

-                   const char *binary, int debug_fd)

+                   const char *binary, int debug_fd,

+                   const char *extra_argv[])

 {

     int ret;

     errno_t err;

@@ -739,7 +757,8 @@ errno_t exec_child(TALLOC_CTX *mem_ctx,

     }

     ret = prepare_child_argv(mem_ctx, debug_fd,

-                             binary, &argv);

+                             binary, extra_argv,

+                             &argv);

     if (ret != EOK) {

         DEBUG(SSSDBG_CRIT_FAILURE, "prepare_child_argv.\n");

         return ret;

diff --git a/src/util/child_common.h b/src/util/child_common.h

index e159719a2fca70a4ea044d79530a0d21cb3577e8..e659388ece3677b7746c159d7de3e86171bb4146 100644

--- a/src/util/child_common.h

+++ b/src/util/child_common.h

@@ -114,7 +114,8 @@ void child_sig_handler(struct tevent_context *ev,

 /* Never returns EOK, ether returns an error, or doesn't return on success */

 errno_t exec_child(TALLOC_CTX *mem_ctx,

                    int *pipefd_to_child, int *pipefd_from_child,

-                   const char *binary, int debug_fd);

+                   const char *binary, int debug_fd,

+                   const char *extra_argv[]);

 void child_cleanup(int readfd, int writefd);

-- 

1.9.3