From 86b623111e4ab4fb176c68c6cea2fbf099f7e21a Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Thu, 14 Jan 2016 13:33:53 +0100 Subject: [PATCH 1/2] UTIL: allow to skip default options for child processes Currently the SSSD default options like e.g. --debug-level are added unconditionally to the command line options of a child process when started with the child helper functions. If a binary from a different source should be started as a child by SSSD those options might not be known or used differently. This patch adds an option to exec_child_ex() which allows to skip the default options and only add specific options. --- src/providers/ad/ad_gpo.c | 2 +- src/providers/krb5/krb5_child_handler.c | 2 +- src/responder/pam/pamsrv_p11.c | 2 +- src/tests/cmocka/test_child_common.c | 4 +- src/util/child_common.c | 73 ++++++++++++++++++--------------- src/util/child_common.h | 2 +- 6 files changed, 47 insertions(+), 38 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index cca5e58618bf861c0d754ef1916583a73c32141a..069196c3b616ebf7661ac9ffe577504b6ebcb674 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -4144,7 +4144,7 @@ gpo_fork_child(struct tevent_req *req) if (pid == 0) { /* child */ err = exec_child_ex(state, pipefd_to_child, pipefd_from_child, - GPO_CHILD, gpo_child_debug_fd, NULL, + GPO_CHILD, gpo_child_debug_fd, NULL, false, STDIN_FILENO, AD_GPO_CHILD_OUT_FILENO); DEBUG(SSSDBG_CRIT_FAILURE, "Could not exec gpo_child: [%d][%s].\n", err, strerror(err)); diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c index fa1055eb7fc7e9aa6fabef1c1759c272b217a395..167a2b2ad09b67908cdce8051d8a37e557c91545 100644 --- a/src/providers/krb5/krb5_child_handler.c +++ b/src/providers/krb5/krb5_child_handler.c @@ -312,7 +312,7 @@ static errno_t fork_child(struct tevent_req *req) err = exec_child_ex(state, pipefd_to_child, pipefd_from_child, KRB5_CHILD, state->kr->krb5_ctx->child_debug_fd, - k5c_extra_args, STDIN_FILENO, STDOUT_FILENO); + k5c_extra_args, false, STDIN_FILENO, STDOUT_FILENO); if (err != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "Could not exec KRB5 child: [%d][%s].\n", err, strerror(err)); diff --git a/src/responder/pam/pamsrv_p11.c b/src/responder/pam/pamsrv_p11.c index bcac439dde57bda2adeab07a4123672060be9259..ad1670136dbf8efc41df6950af744ff8b06e6a11 100644 --- a/src/responder/pam/pamsrv_p11.c +++ b/src/responder/pam/pamsrv_p11.c @@ -322,7 +322,7 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx, child_pid = fork(); if (child_pid == 0) { /* child */ ret = exec_child_ex(state, pipefd_to_child, pipefd_from_child, - P11_CHILD_PATH, child_debug_fd, extra_args, + P11_CHILD_PATH, child_debug_fd, extra_args, false, STDIN_FILENO, STDOUT_FILENO); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "Could not exec p11 child: [%d][%s].\n", diff --git a/src/tests/cmocka/test_child_common.c b/src/tests/cmocka/test_child_common.c index bf500fa5a1f2b2fe79833e23a53cdf0b06b81260..9ed9c1ae42dd93cef833b738c29259a18e791339 100644 --- a/src/tests/cmocka/test_child_common.c +++ b/src/tests/cmocka/test_child_common.c @@ -139,7 +139,7 @@ void test_exec_child_extra_args(void **state) ret = exec_child_ex(child_tctx, child_tctx->pipefd_to_child, child_tctx->pipefd_from_child, - CHILD_DIR"/"TEST_BIN, 2, extra_args, + CHILD_DIR"/"TEST_BIN, 2, extra_args, false, STDIN_FILENO, STDOUT_FILENO); assert_int_equal(ret, EOK); } else { @@ -287,7 +287,7 @@ void test_exec_child_echo(void **state) ret = exec_child_ex(child_tctx, child_tctx->pipefd_to_child, child_tctx->pipefd_from_child, - CHILD_DIR"/"TEST_BIN, 2, NULL, + CHILD_DIR"/"TEST_BIN, 2, NULL, false, STDIN_FILENO, 3); assert_int_equal(ret, EOK); } diff --git a/src/util/child_common.c b/src/util/child_common.c index a6131cd20e7cfff5e5d58806aa2c178327eb9baa..60466c146b5bd9147e9425736072f1ea6ed73663 100644 --- a/src/util/child_common.c +++ b/src/util/child_common.c @@ -612,6 +612,7 @@ static errno_t prepare_child_argv(TALLOC_CTX *mem_ctx, int child_debug_fd, const char *binary, const char *extra_argv[], + bool extra_args_only, char ***_argv) { /* @@ -619,18 +620,24 @@ static errno_t prepare_child_argv(TALLOC_CTX *mem_ctx, * debug_microseconds and NULL */ uint_t argc = 5; - char ** argv; + char ** argv = NULL; errno_t ret = EINVAL; size_t i; + if (extra_args_only) { + argc = 2; /* program name and NULL */ + } + /* Save the current state in case an interrupt changes it */ bool child_debug_to_file = debug_to_file; bool child_debug_timestamps = debug_timestamps; bool child_debug_microseconds = debug_microseconds; bool child_debug_stderr = debug_to_stderr; - if (child_debug_to_file) argc++; - if (child_debug_stderr) argc++; + if (!extra_args_only) { + if (child_debug_to_file) argc++; + if (child_debug_stderr) argc++; + } if (extra_argv) { for (i = 0; extra_argv[i]; i++) argc++; @@ -659,42 +666,44 @@ static errno_t prepare_child_argv(TALLOC_CTX *mem_ctx, } } - argv[--argc] = talloc_asprintf(argv, "--debug-level=%#.4x", - debug_level); - if (argv[argc] == NULL) { - ret = ENOMEM; - goto fail; - } - - if (child_debug_stderr) { - argv[--argc] = talloc_strdup(argv, "--debug-to-stderr"); + if (!extra_args_only) { + argv[--argc] = talloc_asprintf(argv, "--debug-level=%#.4x", + debug_level); if (argv[argc] == NULL) { ret = ENOMEM; goto fail; } - } - if (child_debug_to_file) { - argv[--argc] = talloc_asprintf(argv, "--debug-fd=%d", - child_debug_fd); + if (child_debug_stderr) { + argv[--argc] = talloc_strdup(argv, "--debug-to-stderr"); + if (argv[argc] == NULL) { + ret = ENOMEM; + goto fail; + } + } + + if (child_debug_to_file) { + argv[--argc] = talloc_asprintf(argv, "--debug-fd=%d", + child_debug_fd); + if (argv[argc] == NULL) { + ret = ENOMEM; + goto fail; + } + } + + argv[--argc] = talloc_asprintf(argv, "--debug-timestamps=%d", + child_debug_timestamps); if (argv[argc] == NULL) { ret = ENOMEM; goto fail; } - } - argv[--argc] = talloc_asprintf(argv, "--debug-timestamps=%d", - child_debug_timestamps); - if (argv[argc] == NULL) { - ret = ENOMEM; - goto fail; - } - - argv[--argc] = talloc_asprintf(argv, "--debug-microseconds=%d", - child_debug_microseconds); - if (argv[argc] == NULL) { - ret = ENOMEM; - goto fail; + argv[--argc] = talloc_asprintf(argv, "--debug-microseconds=%d", + child_debug_microseconds); + if (argv[argc] == NULL) { + ret = ENOMEM; + goto fail; + } } argv[--argc] = talloc_strdup(argv, binary); @@ -720,7 +729,7 @@ fail: errno_t exec_child_ex(TALLOC_CTX *mem_ctx, int *pipefd_to_child, int *pipefd_from_child, const char *binary, int debug_fd, - const char *extra_argv[], + const char *extra_argv[], bool extra_args_only, int child_in_fd, int child_out_fd) { int ret; @@ -746,7 +755,7 @@ errno_t exec_child_ex(TALLOC_CTX *mem_ctx, } ret = prepare_child_argv(mem_ctx, debug_fd, - binary, extra_argv, + binary, extra_argv, extra_args_only, &argv); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "prepare_child_argv.\n"); @@ -764,7 +773,7 @@ errno_t exec_child(TALLOC_CTX *mem_ctx, const char *binary, int debug_fd) { return exec_child_ex(mem_ctx, pipefd_to_child, pipefd_from_child, - binary, debug_fd, NULL, + binary, debug_fd, NULL, false, STDIN_FILENO, STDOUT_FILENO); } diff --git a/src/util/child_common.h b/src/util/child_common.h index b93991832b7389177f9da05e694ab729ef50cdc7..0111f2cdb26af8543d68e6a6661d656d1c9c45ac 100644 --- a/src/util/child_common.h +++ b/src/util/child_common.h @@ -104,7 +104,7 @@ void fd_nonblocking(int fd); errno_t exec_child_ex(TALLOC_CTX *mem_ctx, int *pipefd_to_child, int *pipefd_from_child, const char *binary, int debug_fd, - const char *extra_argv[], + const char *extra_argv[], bool extra_args_only, int child_in_fd, int child_out_fd); /* Same as exec_child_ex() except child_in_fd is set to STDIN_FILENO and -- 2.1.0