From 6468d9f2bb30147967a724075c504afb3481d465 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek 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 --- 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