From 89711996b3f561522508306e0b5ecf34f6016638 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 22 Jan 2017 12:35:08 -0500 Subject: basic/util: move execute_directory() to separate file It's a fairly specialized function. Let's make new files for it and the tests. --- src/basic/exec-util.c | 181 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 181 insertions(+) create mode 100644 src/basic/exec-util.c (limited to 'src/basic/exec-util.c') diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c new file mode 100644 index 0000000000..757cd3b4ff --- /dev/null +++ b/src/basic/exec-util.c @@ -0,0 +1,181 @@ +/*** + This file is part of systemd. + + Copyright 2010 Lennart Poettering + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see . +***/ + +#include +#include +#include +#include +#include + +#include "alloc-util.h" +#include "dirent-util.h" +#include "exec-util.h" +#include "fd-util.h" +#include "hashmap.h" +#include "macro.h" +#include "process-util.h" +#include "set.h" +#include "signal-util.h" +#include "stat-util.h" +#include "string-util.h" +#include "strv.h" +#include "util.h" + +/* Put this test here for a lack of better place */ +assert_cc(EAGAIN == EWOULDBLOCK); + +static int do_execute(char **directories, usec_t timeout, char *argv[]) { + _cleanup_hashmap_free_free_ Hashmap *pids = NULL; + _cleanup_set_free_free_ Set *seen = NULL; + char **directory; + + /* We fork this all off from a child process so that we can + * somewhat cleanly make use of SIGALRM to set a time limit */ + + (void) reset_all_signal_handlers(); + (void) reset_signal_mask(); + + assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); + + pids = hashmap_new(NULL); + if (!pids) + return log_oom(); + + seen = set_new(&string_hash_ops); + if (!seen) + return log_oom(); + + STRV_FOREACH(directory, directories) { + _cleanup_closedir_ DIR *d; + struct dirent *de; + + d = opendir(*directory); + if (!d) { + if (errno == ENOENT) + continue; + + return log_error_errno(errno, "Failed to open directory %s: %m", *directory); + } + + FOREACH_DIRENT(de, d, break) { + _cleanup_free_ char *path = NULL; + pid_t pid; + int r; + + if (!dirent_is_file(de)) + continue; + + if (set_contains(seen, de->d_name)) { + log_debug("%1$s/%2$s skipped (%2$s was already seen).", *directory, de->d_name); + continue; + } + + r = set_put_strdup(seen, de->d_name); + if (r < 0) + return log_oom(); + + path = strjoin(*directory, "/", de->d_name); + if (!path) + return log_oom(); + + if (null_or_empty_path(path)) { + log_debug("%s is empty (a mask).", path); + continue; + } + + pid = fork(); + if (pid < 0) { + log_error_errno(errno, "Failed to fork: %m"); + continue; + } else if (pid == 0) { + char *_argv[2]; + + assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); + + if (!argv) { + _argv[0] = path; + _argv[1] = NULL; + argv = _argv; + } else + argv[0] = path; + + execv(path, argv); + return log_error_errno(errno, "Failed to execute %s: %m", path); + } + + log_debug("Spawned %s as " PID_FMT ".", path, pid); + + r = hashmap_put(pids, PID_TO_PTR(pid), path); + if (r < 0) + return log_oom(); + path = NULL; + } + } + + /* Abort execution of this process after the timout. We simply + * rely on SIGALRM as default action terminating the process, + * and turn on alarm(). */ + + if (timeout != USEC_INFINITY) + alarm((timeout + USEC_PER_SEC - 1) / USEC_PER_SEC); + + while (!hashmap_isempty(pids)) { + _cleanup_free_ char *path = NULL; + pid_t pid; + + pid = PTR_TO_PID(hashmap_first_key(pids)); + assert(pid > 0); + + path = hashmap_remove(pids, PID_TO_PTR(pid)); + assert(path); + + wait_for_terminate_and_warn(path, pid, true); + } + + return 0; +} + +void execute_directories(const char* const* directories, usec_t timeout, char *argv[]) { + pid_t executor_pid; + int r; + char *name; + char **dirs = (char**) directories; + + assert(!strv_isempty(dirs)); + + name = basename(dirs[0]); + assert(!isempty(name)); + + /* Executes all binaries in the directories in parallel and waits + * for them to finish. Optionally a timeout is applied. If a file + * with the same name exists in more than one directory, the + * earliest one wins. */ + + executor_pid = fork(); + if (executor_pid < 0) { + log_error_errno(errno, "Failed to fork: %m"); + return; + + } else if (executor_pid == 0) { + r = do_execute(dirs, timeout, argv); + _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS); + } + + wait_for_terminate_and_warn(name, executor_pid, true); +} -- cgit v1.2.3-54-g00ecf From cf55fc180783e5350ec243daf281200c6f6a3191 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 22 Jan 2017 13:06:27 -0500 Subject: basic/exec-util: split out actual execution to a different function This corrects an error in error handling: if execution fails, we should never use return, but immediately _exit(). --- src/basic/exec-util.c | 60 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 25 deletions(-) (limited to 'src/basic/exec-util.c') diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c index 757cd3b4ff..23dcac3274 100644 --- a/src/basic/exec-util.c +++ b/src/basic/exec-util.c @@ -40,6 +40,39 @@ /* Put this test here for a lack of better place */ assert_cc(EAGAIN == EWOULDBLOCK); +static int do_spawn(const char *path, char *argv[], pid_t *pid) { + pid_t _pid; + + if (null_or_empty_path(path)) { + log_debug("%s is empty (a mask).", path); + return 0; + } + + _pid = fork(); + if (_pid < 0) + return log_error_errno(errno, "Failed to fork: %m"); + if (_pid == 0) { + char *_argv[2]; + + assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); + + if (!argv) { + _argv[0] = (char*) path; + _argv[1] = NULL; + argv = _argv; + } else + argv[0] = (char*) path; + + execv(path, argv); + log_error_errno(errno, "Failed to execute %s: %m", path); + _exit(EXIT_FAILURE); + } + + log_debug("Spawned %s as " PID_FMT ".", path, _pid); + *pid = _pid; + return 1; +} + static int do_execute(char **directories, usec_t timeout, char *argv[]) { _cleanup_hashmap_free_free_ Hashmap *pids = NULL; _cleanup_set_free_free_ Set *seen = NULL; @@ -94,32 +127,9 @@ static int do_execute(char **directories, usec_t timeout, char *argv[]) { if (!path) return log_oom(); - if (null_or_empty_path(path)) { - log_debug("%s is empty (a mask).", path); + r = do_spawn(path, argv, &pid); + if (r <= 0) continue; - } - - pid = fork(); - if (pid < 0) { - log_error_errno(errno, "Failed to fork: %m"); - continue; - } else if (pid == 0) { - char *_argv[2]; - - assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); - - if (!argv) { - _argv[0] = path; - _argv[1] = NULL; - argv = _argv; - } else - argv[0] = path; - - execv(path, argv); - return log_error_errno(errno, "Failed to execute %s: %m", path); - } - - log_debug("Spawned %s as " PID_FMT ".", path, pid); r = hashmap_put(pids, PID_TO_PTR(pid), path); if (r < 0) -- cgit v1.2.3-54-g00ecf From 2e4cfe65b87075d79f261e0fb3dd6bac1290b06d Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 22 Jan 2017 14:44:34 -0500 Subject: basic/exec-util: use conf_files_list_strv to list executables Essentially the same logic as in conf_files_list() was independently implemented in do_execute(). With previous commit, do_execute() can just call conf_files_list() to get a list of executable paths. --- src/basic/exec-util.c | 75 +++++++++++++++++---------------------------------- 1 file changed, 25 insertions(+), 50 deletions(-) (limited to 'src/basic/exec-util.c') diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c index 23dcac3274..46eafc7b90 100644 --- a/src/basic/exec-util.c +++ b/src/basic/exec-util.c @@ -24,9 +24,8 @@ #include #include "alloc-util.h" -#include "dirent-util.h" +#include "conf-files.h" #include "exec-util.h" -#include "fd-util.h" #include "hashmap.h" #include "macro.h" #include "process-util.h" @@ -75,8 +74,9 @@ static int do_spawn(const char *path, char *argv[], pid_t *pid) { static int do_execute(char **directories, usec_t timeout, char *argv[]) { _cleanup_hashmap_free_free_ Hashmap *pids = NULL; - _cleanup_set_free_free_ Set *seen = NULL; - char **directory; + _cleanup_strv_free_ char **paths = NULL; + char **path; + int r; /* We fork this all off from a child process so that we can * somewhat cleanly make use of SIGALRM to set a time limit */ @@ -86,56 +86,31 @@ static int do_execute(char **directories, usec_t timeout, char *argv[]) { assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); + r = conf_files_list_strv(&paths, NULL, NULL, (const char* const*) directories); + if (r < 0) + return r; + pids = hashmap_new(NULL); if (!pids) return log_oom(); - seen = set_new(&string_hash_ops); - if (!seen) - return log_oom(); - - STRV_FOREACH(directory, directories) { - _cleanup_closedir_ DIR *d; - struct dirent *de; - - d = opendir(*directory); - if (!d) { - if (errno == ENOENT) - continue; - - return log_error_errno(errno, "Failed to open directory %s: %m", *directory); - } - - FOREACH_DIRENT(de, d, break) { - _cleanup_free_ char *path = NULL; - pid_t pid; - int r; - - if (!dirent_is_file(de)) - continue; - - if (set_contains(seen, de->d_name)) { - log_debug("%1$s/%2$s skipped (%2$s was already seen).", *directory, de->d_name); - continue; - } + STRV_FOREACH(path, paths) { + _cleanup_free_ char *t = NULL; + pid_t pid; - r = set_put_strdup(seen, de->d_name); - if (r < 0) - return log_oom(); + t = strdup(*path); + if (!t) + return log_oom(); - path = strjoin(*directory, "/", de->d_name); - if (!path) - return log_oom(); + r = do_spawn(t, argv, &pid); + if (r <= 0) + continue; - r = do_spawn(path, argv, &pid); - if (r <= 0) - continue; + r = hashmap_put(pids, PID_TO_PTR(pid), t); + if (r < 0) + return log_oom(); - r = hashmap_put(pids, PID_TO_PTR(pid), path); - if (r < 0) - return log_oom(); - path = NULL; - } + t = NULL; } /* Abort execution of this process after the timout. We simply @@ -146,16 +121,16 @@ static int do_execute(char **directories, usec_t timeout, char *argv[]) { alarm((timeout + USEC_PER_SEC - 1) / USEC_PER_SEC); while (!hashmap_isempty(pids)) { - _cleanup_free_ char *path = NULL; + _cleanup_free_ char *t = NULL; pid_t pid; pid = PTR_TO_PID(hashmap_first_key(pids)); assert(pid > 0); - path = hashmap_remove(pids, PID_TO_PTR(pid)); - assert(path); + t = hashmap_remove(pids, PID_TO_PTR(pid)); + assert(t); - wait_for_terminate_and_warn(path, pid, true); + wait_for_terminate_and_warn(t, pid, true); } return 0; -- cgit v1.2.3-54-g00ecf From c6e47247a745f9245eb3dbfb29fc066ef72d3886 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 22 Jan 2017 15:22:37 -0500 Subject: basic/exec-util: add support for synchronous (ordered) execution The output of processes can be gathered, and passed back to the callee. (This commit just implements the basic functionality and tests.) After the preparation in previous commits, the change in functionality is relatively simple. For coding convenience, alarm is prepared *before* any children are executed, and not before. This shouldn't matter usually, since just forking of the children should be pretty quick. One could also argue that this is more correct, because we will also catch the case when (for whatever reason), forking itself is slow. Three callback functions and three levels of serialization are used: - from individual generator processes to the generator forker - from the forker back to the main process - deserialization in the main process v2: - replace an structure with an indexed array of callbacks --- src/basic/exec-util.c | 149 ++++++++++++++++++++++++------- src/basic/exec-util.h | 18 +++- src/core/manager.c | 3 +- src/core/shutdown.c | 2 +- src/sleep/sleep.c | 4 +- src/test/test-exec-util.c | 220 +++++++++++++++++++++++++++++++++++++++++++--- 6 files changed, 349 insertions(+), 47 deletions(-) (limited to 'src/basic/exec-util.c') diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c index 46eafc7b90..d69039c489 100644 --- a/src/basic/exec-util.c +++ b/src/basic/exec-util.c @@ -22,10 +22,14 @@ #include #include #include +#include #include "alloc-util.h" #include "conf-files.h" +#include "env-util.h" #include "exec-util.h" +#include "fd-util.h" +#include "fileio.h" #include "hashmap.h" #include "macro.h" #include "process-util.h" @@ -34,12 +38,14 @@ #include "stat-util.h" #include "string-util.h" #include "strv.h" +#include "terminal-util.h" #include "util.h" /* Put this test here for a lack of better place */ assert_cc(EAGAIN == EWOULDBLOCK); -static int do_spawn(const char *path, char *argv[], pid_t *pid) { +static int do_spawn(const char *path, char *argv[], int stdout_fd, pid_t *pid) { + pid_t _pid; if (null_or_empty_path(path)) { @@ -55,6 +61,15 @@ static int do_spawn(const char *path, char *argv[], pid_t *pid) { assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); + if (stdout_fd >= 0) { + /* If the fd happens to be in the right place, go along with that */ + if (stdout_fd != STDOUT_FILENO && + dup2(stdout_fd, STDOUT_FILENO) < 0) + return -errno; + + fd_cloexec(STDOUT_FILENO, false); + } + if (!argv) { _argv[0] = (char*) path; _argv[1] = NULL; @@ -72,14 +87,24 @@ static int do_spawn(const char *path, char *argv[], pid_t *pid) { return 1; } -static int do_execute(char **directories, usec_t timeout, char *argv[]) { +static int do_execute( + char **directories, + usec_t timeout, + gather_stdout_callback_t const callbacks[_STDOUT_CONSUME_MAX], + void* const callback_args[_STDOUT_CONSUME_MAX], + int output_fd, + char *argv[]) { + _cleanup_hashmap_free_free_ Hashmap *pids = NULL; _cleanup_strv_free_ char **paths = NULL; char **path; int r; - /* We fork this all off from a child process so that we can - * somewhat cleanly make use of SIGALRM to set a time limit */ + /* We fork this all off from a child process so that we can somewhat cleanly make + * use of SIGALRM to set a time limit. + * + * If callbacks is nonnull, execution is serial. Otherwise, we default to parallel. + */ (void) reset_all_signal_handlers(); (void) reset_signal_mask(); @@ -90,35 +115,62 @@ static int do_execute(char **directories, usec_t timeout, char *argv[]) { if (r < 0) return r; - pids = hashmap_new(NULL); - if (!pids) - return log_oom(); + if (!callbacks) { + pids = hashmap_new(NULL); + if (!pids) + return log_oom(); + } + + /* Abort execution of this process after the timout. We simply rely on SIGALRM as + * default action terminating the process, and turn on alarm(). */ + + if (timeout != USEC_INFINITY) + alarm((timeout + USEC_PER_SEC - 1) / USEC_PER_SEC); STRV_FOREACH(path, paths) { _cleanup_free_ char *t = NULL; + _cleanup_close_ int fd = -1; pid_t pid; t = strdup(*path); if (!t) return log_oom(); - r = do_spawn(t, argv, &pid); + if (callbacks) { + fd = open_serialization_fd(basename(*path)); + if (fd < 0) + return log_error_errno(fd, "Failed to open serialization file: %m"); + } + + r = do_spawn(t, argv, fd, &pid); if (r <= 0) continue; - r = hashmap_put(pids, PID_TO_PTR(pid), t); - if (r < 0) - return log_oom(); - - t = NULL; + if (pids) { + r = hashmap_put(pids, PID_TO_PTR(pid), t); + if (r < 0) + return log_oom(); + t = NULL; + } else { + r = wait_for_terminate_and_warn(t, pid, true); + if (r < 0) + continue; + + if (lseek(fd, 0, SEEK_SET) < 0) + return log_error_errno(errno, "Failed to seek on serialization fd: %m"); + + r = callbacks[STDOUT_GENERATE](fd, callback_args[STDOUT_GENERATE]); + fd = -1; + if (r < 0) + return log_error_errno(r, "Failed to process output from %s: %m", *path); + } } - /* Abort execution of this process after the timout. We simply - * rely on SIGALRM as default action terminating the process, - * and turn on alarm(). */ - - if (timeout != USEC_INFINITY) - alarm((timeout + USEC_PER_SEC - 1) / USEC_PER_SEC); + if (callbacks) { + r = callbacks[STDOUT_COLLECT](output_fd, callback_args[STDOUT_COLLECT]); + if (r < 0) + return log_error_errno(r, "Callback two failed: %m"); + } while (!hashmap_isempty(pids)) { _cleanup_free_ char *t = NULL; @@ -136,31 +188,66 @@ static int do_execute(char **directories, usec_t timeout, char *argv[]) { return 0; } -void execute_directories(const char* const* directories, usec_t timeout, char *argv[]) { +int execute_directories( + const char* const* directories, + usec_t timeout, + gather_stdout_callback_t const callbacks[_STDOUT_CONSUME_MAX], + void* const callback_args[_STDOUT_CONSUME_MAX], + char *argv[]) { + pid_t executor_pid; - int r; char *name; char **dirs = (char**) directories; + _cleanup_close_ int fd = -1; + int r; assert(!strv_isempty(dirs)); name = basename(dirs[0]); assert(!isempty(name)); - /* Executes all binaries in the directories in parallel and waits - * for them to finish. Optionally a timeout is applied. If a file - * with the same name exists in more than one directory, the - * earliest one wins. */ + if (callbacks) { + assert(callback_args); + assert(callbacks[STDOUT_GENERATE]); + assert(callbacks[STDOUT_COLLECT]); + assert(callbacks[STDOUT_CONSUME]); + + fd = open_serialization_fd(name); + if (fd < 0) + return log_error_errno(fd, "Failed to open serialization file: %m"); + } + + /* Executes all binaries in the directories serially or in parallel and waits for + * them to finish. Optionally a timeout is applied. If a file with the same name + * exists in more than one directory, the earliest one wins. */ executor_pid = fork(); - if (executor_pid < 0) { - log_error_errno(errno, "Failed to fork: %m"); - return; + if (executor_pid < 0) + return log_error_errno(errno, "Failed to fork: %m"); - } else if (executor_pid == 0) { - r = do_execute(dirs, timeout, argv); + if (executor_pid == 0) { + r = do_execute(dirs, timeout, callbacks, callback_args, fd, argv); _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS); } - wait_for_terminate_and_warn(name, executor_pid, true); + r = wait_for_terminate_and_warn(name, executor_pid, true); + if (r < 0) + return log_error_errno(r, "Execution failed: %m"); + if (r > 0) { + /* non-zero return code from child */ + log_error("Forker process failed."); + return -EREMOTEIO; + } + + if (!callbacks) + return 0; + + if (lseek(fd, 0, SEEK_SET) < 0) + return log_error_errno(errno, "Failed to rewind serialization fd: %m"); + + r = callbacks[STDOUT_CONSUME](fd, callback_args[STDOUT_CONSUME]); + fd = -1; + if (r < 0) + return log_error_errno(r, "Failed to parse returned data: %m"); + return 0; } diff --git a/src/basic/exec-util.h b/src/basic/exec-util.h index 9f8daa9fc8..2c58e4bd5c 100644 --- a/src/basic/exec-util.h +++ b/src/basic/exec-util.h @@ -17,6 +17,22 @@ along with systemd; If not, see . ***/ +#include + #include "time-util.h" -void execute_directories(const char* const* directories, usec_t timeout, char *argv[]); +typedef int (*gather_stdout_callback_t) (int fd, void *arg); + +enum { + STDOUT_GENERATE, /* from generators to helper process */ + STDOUT_COLLECT, /* from helper process to main process */ + STDOUT_CONSUME, /* process data in main process */ + _STDOUT_CONSUME_MAX, +}; + +int execute_directories( + const char* const* directories, + usec_t timeout, + gather_stdout_callback_t const callbacks[_STDOUT_CONSUME_MAX], + void* const callback_args[_STDOUT_CONSUME_MAX], + char *argv[]); diff --git a/src/core/manager.c b/src/core/manager.c index d432512a59..73ac7499bd 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3047,7 +3047,8 @@ static int manager_run_generators(Manager *m) { argv[4] = NULL; RUN_WITH_UMASK(0022) - execute_directories((const char* const*) paths, DEFAULT_TIMEOUT_USEC, (char**) argv); + execute_directories((const char* const*) paths, DEFAULT_TIMEOUT_USEC, + NULL, NULL, (char**) argv); finish: lookup_paths_trim_generator(&m->lookup_paths); diff --git a/src/core/shutdown.c b/src/core/shutdown.c index 56a035e234..a2309b7726 100644 --- a/src/core/shutdown.c +++ b/src/core/shutdown.c @@ -322,7 +322,7 @@ int main(int argc, char *argv[]) { arguments[0] = NULL; arguments[1] = arg_verb; arguments[2] = NULL; - execute_directories(dirs, DEFAULT_TIMEOUT_USEC, arguments); + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments); if (!in_container && !in_initrd() && access("/run/initramfs/shutdown", X_OK) == 0) { diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index b0f992fc9c..a6978b6273 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -107,7 +107,7 @@ static int execute(char **modes, char **states) { if (r < 0) return r; - execute_directories(dirs, DEFAULT_TIMEOUT_USEC, arguments); + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments); log_struct(LOG_INFO, LOG_MESSAGE_ID(SD_MESSAGE_SLEEP_START), @@ -126,7 +126,7 @@ static int execute(char **modes, char **states) { NULL); arguments[1] = (char*) "post"; - execute_directories(dirs, DEFAULT_TIMEOUT_USEC, arguments); + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments); return r; } diff --git a/src/test/test-exec-util.c b/src/test/test-exec-util.c index 26533f0bf6..41cbef74b1 100644 --- a/src/test/test-exec-util.c +++ b/src/test/test-exec-util.c @@ -24,21 +24,59 @@ #include #include +#include "alloc-util.h" +#include "copy.h" #include "def.h" #include "exec-util.h" +#include "fd-util.h" #include "fileio.h" #include "fs-util.h" #include "log.h" #include "macro.h" #include "rm-rf.h" #include "string-util.h" +#include "strv.h" -static void test_execute_directory(void) { - char template_lo[] = "/tmp/test-readlink_and_make_absolute-lo.XXXXXXX"; - char template_hi[] = "/tmp/test-readlink_and_make_absolute-hi.XXXXXXX"; +static int here = 0, here2 = 0, here3 = 0; +void *ignore_stdout_args[] = {&here, &here2, &here3}; + +/* noop handlers, just check that arguments are passed correctly */ +static int ignore_stdout_func(int fd, void *arg) { + assert(fd >= 0); + assert(arg == &here); + safe_close(fd); + + return 0; +} +static int ignore_stdout_func2(int fd, void *arg) { + assert(fd >= 0); + assert(arg == &here2); + safe_close(fd); + + return 0; +} +static int ignore_stdout_func3(int fd, void *arg) { + assert(fd >= 0); + assert(arg == &here3); + safe_close(fd); + + return 0; +} + +static const gather_stdout_callback_t ignore_stdout[] = { + ignore_stdout_func, + ignore_stdout_func2, + ignore_stdout_func3, +}; + +static void test_execute_directory(bool gather_stdout) { + char template_lo[] = "/tmp/test-exec-util.XXXXXXX"; + char template_hi[] = "/tmp/test-exec-util.XXXXXXX"; const char * dirs[] = {template_hi, template_lo, NULL}; const char *name, *name2, *name3, *overridden, *override, *masked, *mask; + log_info("/* %s (%s) */", __func__, gather_stdout ? "gathering stdout" : "asynchronous"); + assert_se(mkdtemp(template_lo)); assert_se(mkdtemp(template_hi)); @@ -50,20 +88,34 @@ static void test_execute_directory(void) { masked = strjoina(template_lo, "/masked"); mask = strjoina(template_hi, "/masked"); - assert_se(write_string_file(name, "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/it_works", WRITE_STRING_FILE_CREATE) == 0); - assert_se(write_string_file(name2, "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/it_works2", WRITE_STRING_FILE_CREATE) == 0); - assert_se(write_string_file(overridden, "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/failed", WRITE_STRING_FILE_CREATE) == 0); - assert_se(write_string_file(override, "#!/bin/sh\necho 'Executing '$0", WRITE_STRING_FILE_CREATE) == 0); - assert_se(write_string_file(masked, "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/failed", WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(name, + "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/it_works", + WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(name2, + "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/it_works2", + WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(overridden, + "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/failed", + WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(override, + "#!/bin/sh\necho 'Executing '$0", + WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(masked, + "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/failed", + WRITE_STRING_FILE_CREATE) == 0); assert_se(symlink("/dev/null", mask) == 0); + assert_se(touch(name3) >= 0); + assert_se(chmod(name, 0755) == 0); assert_se(chmod(name2, 0755) == 0); assert_se(chmod(overridden, 0755) == 0); assert_se(chmod(override, 0755) == 0); assert_se(chmod(masked, 0755) == 0); - assert_se(touch(name3) >= 0); - execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL); + if (gather_stdout) + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, ignore_stdout, ignore_stdout_args, NULL); + else + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, NULL); assert_se(chdir(template_lo) == 0); assert_se(access("it_works", F_OK) >= 0); @@ -77,11 +129,157 @@ static void test_execute_directory(void) { (void) rm_rf(template_hi, REMOVE_ROOT|REMOVE_PHYSICAL); } +static void test_execution_order(void) { + char template_lo[] = "/tmp/test-exec-util-lo.XXXXXXX"; + char template_hi[] = "/tmp/test-exec-util-hi.XXXXXXX"; + const char *dirs[] = {template_hi, template_lo, NULL}; + const char *name, *name2, *name3, *overridden, *override, *masked, *mask; + const char *output, *t; + _cleanup_free_ char *contents = NULL; + + assert_se(mkdtemp(template_lo)); + assert_se(mkdtemp(template_hi)); + + output = strjoina(template_hi, "/output"); + + log_info("/* %s >>%s */", __func__, output); + + /* write files in "random" order */ + name2 = strjoina(template_lo, "/90-bar"); + name = strjoina(template_hi, "/80-foo"); + name3 = strjoina(template_lo, "/last"); + overridden = strjoina(template_lo, "/30-override"); + override = strjoina(template_hi, "/30-override"); + masked = strjoina(template_lo, "/10-masked"); + mask = strjoina(template_hi, "/10-masked"); + + t = strjoina("#!/bin/sh\necho $(basename $0) >>", output); + assert_se(write_string_file(name, t, WRITE_STRING_FILE_CREATE) == 0); + + t = strjoina("#!/bin/sh\necho $(basename $0) >>", output); + assert_se(write_string_file(name2, t, WRITE_STRING_FILE_CREATE) == 0); + + t = strjoina("#!/bin/sh\necho $(basename $0) >>", output); + assert_se(write_string_file(name3, t, WRITE_STRING_FILE_CREATE) == 0); + + t = strjoina("#!/bin/sh\necho OVERRIDDEN >>", output); + assert_se(write_string_file(overridden, t, WRITE_STRING_FILE_CREATE) == 0); + + t = strjoina("#!/bin/sh\necho $(basename $0) >>", output); + assert_se(write_string_file(override, t, WRITE_STRING_FILE_CREATE) == 0); + + t = strjoina("#!/bin/sh\necho MASKED >>", output); + assert_se(write_string_file(masked, t, WRITE_STRING_FILE_CREATE) == 0); + + assert_se(symlink("/dev/null", mask) == 0); + + assert_se(chmod(name, 0755) == 0); + assert_se(chmod(name2, 0755) == 0); + assert_se(chmod(name3, 0755) == 0); + assert_se(chmod(overridden, 0755) == 0); + assert_se(chmod(override, 0755) == 0); + assert_se(chmod(masked, 0755) == 0); + + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, ignore_stdout, ignore_stdout_args, NULL); + + assert_se(read_full_file(output, &contents, NULL) >= 0); + assert_se(streq(contents, "30-override\n80-foo\n90-bar\nlast\n")); + + (void) rm_rf(template_lo, REMOVE_ROOT|REMOVE_PHYSICAL); + (void) rm_rf(template_hi, REMOVE_ROOT|REMOVE_PHYSICAL); +} + +static int gather_stdout_one(int fd, void *arg) { + char ***s = arg, *t; + char buf[128] = {}; + + assert_se(s); + assert_se(read(fd, buf, sizeof buf) >= 0); + safe_close(fd); + + assert_se(t = strndup(buf, sizeof buf)); + assert_se(strv_push(s, t) >= 0); + + return 0; +} +static int gather_stdout_two(int fd, void *arg) { + char ***s = arg, **t; + + STRV_FOREACH(t, *s) + assert_se(write(fd, *t, strlen(*t)) == (ssize_t) strlen(*t)); + safe_close(fd); + + return 0; +} +static int gather_stdout_three(int fd, void *arg) { + char **s = arg; + char buf[128] = {}; + + assert_se(read(fd, buf, sizeof buf - 1) > 0); + safe_close(fd); + assert_se(*s = strndup(buf, sizeof buf)); + + return 0; +} + +const gather_stdout_callback_t const gather_stdout[] = { + gather_stdout_one, + gather_stdout_two, + gather_stdout_three, +}; + + +static void test_stdout_gathering(void) { + char template[] = "/tmp/test-exec-util.XXXXXXX"; + const char *dirs[] = {template, NULL}; + const char *name, *name2, *name3; + int r; + + char **tmp = NULL; /* this is only used in the forked process, no cleanup here */ + _cleanup_free_ char *output = NULL; + + void* args[] = {&tmp, &tmp, &output}; + + assert_se(mkdtemp(template)); + + log_info("/* %s */", __func__); + + /* write files */ + name = strjoina(template, "/10-foo"); + name2 = strjoina(template, "/20-bar"); + name3 = strjoina(template, "/30-last"); + + assert_se(write_string_file(name, + "#!/bin/sh\necho a\necho b\necho c\n", + WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(name2, + "#!/bin/sh\necho d\n", + WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(name3, + "#!/bin/sh\nsleep 1", + WRITE_STRING_FILE_CREATE) == 0); + + assert_se(chmod(name, 0755) == 0); + assert_se(chmod(name2, 0755) == 0); + assert_se(chmod(name3, 0755) == 0); + + r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_stdout, args, NULL); + assert_se(r >= 0); + + log_info("got: %s", output); + + assert_se(streq(output, "a\nb\nc\nd\n")); +} + int main(int argc, char *argv[]) { + log_set_max_level(LOG_DEBUG); log_parse_environment(); log_open(); - test_execute_directory(); + test_execute_directory(true); + test_execute_directory(false); + test_execution_order(); + test_stdout_gathering(); return 0; } -- cgit v1.2.3-54-g00ecf From 3303d1b2dc3f9ac88927c23abce020fc77c1e92c Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Fri, 10 Feb 2017 21:49:01 -0500 Subject: exec-util: implement a set of callbacks to pass variables around Only tests are added, otherwise the new code is unused. --- src/basic/exec-util.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++ src/basic/exec-util.h | 2 + src/test/test-exec-util.c | 55 +++++++++++++++++++++++++ 3 files changed, 159 insertions(+) (limited to 'src/basic/exec-util.c') diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c index d69039c489..207fac8dc1 100644 --- a/src/basic/exec-util.c +++ b/src/basic/exec-util.c @@ -251,3 +251,105 @@ int execute_directories( return log_error_errno(r, "Failed to parse returned data: %m"); return 0; } + +static int gather_environment_generate(int fd, void *arg) { + char ***env = arg, **x, **y; + _cleanup_fclose_ FILE *f = NULL; + _cleanup_strv_free_ char **new; + int r; + + /* Read a series of VAR=value assignments from fd, use them to update the list of + * variables in env. Also update the exported environment. + * + * fd is always consumed, even on error. + */ + + assert(env); + + f = fdopen(fd, "r"); + if (!f) { + safe_close(fd); + return -errno; + } + + r = load_env_file_pairs(f, NULL, NULL, &new); + if (r < 0) + return r; + + STRV_FOREACH_PAIR(x, y, new) { + char *p; + + p = strjoin(*x, "=", *y); + if (!p) + return -ENOMEM; + + r = strv_env_replace(env, p); + if (r < 0) + return r; + + if (setenv(*x, *y, true) < 0) + return -errno; + } + + return r; +} + +static int gather_environment_collect(int fd, void *arg) { + char ***env = arg; + _cleanup_fclose_ FILE *f = NULL; + int r; + + /* Write out a series of env=cescape(VAR=value) assignments to fd. */ + + assert(env); + + f = fdopen(fd, "w"); + if (!f) { + safe_close(fd); + return -errno; + } + + r = serialize_environment(f, *env); + if (r < 0) + return r; + + if (ferror(f)) + return errno > 0 ? -errno : -EIO; + + return 0; +} + +static int gather_environment_consume(int fd, void *arg) { + char ***env = arg; + _cleanup_fclose_ FILE *f = NULL; + char line[LINE_MAX]; + int r = 0, k; + + /* Read a series of env=cescape(VAR=value) assignments from fd into env. */ + + assert(env); + + f = fdopen(fd, "r"); + if (!f) { + safe_close(fd); + return -errno; + } + + FOREACH_LINE(line, f, return -EIO) { + truncate_nl(line); + + k = deserialize_environment(env, line); + if (k < 0) + log_error_errno(k, "Invalid line \"%s\": %m", line); + if (k < 0 && r == 0) + r = k; + } + + return r; +} + +const gather_stdout_callback_t gather_environment[] = { + gather_environment_generate, + gather_environment_collect, + gather_environment_consume, +}; diff --git a/src/basic/exec-util.h b/src/basic/exec-util.h index 2c58e4bd5c..72009799b2 100644 --- a/src/basic/exec-util.h +++ b/src/basic/exec-util.h @@ -36,3 +36,5 @@ int execute_directories( gather_stdout_callback_t const callbacks[_STDOUT_CONSUME_MAX], void* const callback_args[_STDOUT_CONSUME_MAX], char *argv[]); + +extern const gather_stdout_callback_t gather_environment[_STDOUT_CONSUME_MAX]; diff --git a/src/test/test-exec-util.c b/src/test/test-exec-util.c index 41cbef74b1..94fe042aa9 100644 --- a/src/test/test-exec-util.c +++ b/src/test/test-exec-util.c @@ -27,6 +27,7 @@ #include "alloc-util.h" #include "copy.h" #include "def.h" +#include "env-util.h" #include "exec-util.h" #include "fd-util.h" #include "fileio.h" @@ -271,6 +272,59 @@ static void test_stdout_gathering(void) { assert_se(streq(output, "a\nb\nc\nd\n")); } +static void test_environment_gathering(void) { + char template[] = "/tmp/test-exec-util.XXXXXXX", **p; + const char *dirs[] = {template, NULL}; + const char *name, *name2, *name3; + int r; + + char **tmp = NULL; /* this is only used in the forked process, no cleanup here */ + _cleanup_strv_free_ char **env = NULL; + + void* const args[] = { &tmp, &tmp, &env }; + + assert_se(mkdtemp(template)); + + log_info("/* %s */", __func__); + + /* write files */ + name = strjoina(template, "/10-foo"); + name2 = strjoina(template, "/20-bar"); + name3 = strjoina(template, "/30-last"); + + assert_se(write_string_file(name, + "#!/bin/sh\n" + "echo A=23\n", + WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(name2, + "#!/bin/sh\n" + "echo A=22:$A\n\n\n", /* substitution from previous generator */ + WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(name3, + "#!/bin/sh\n" + "echo A=$A:24\n" + "echo B=12\n" + "echo C=000\n" + "echo C=001\n" /* variable overwriting */ + "echo PATH=$PATH:/no/such/file", /* variable from manager */ + WRITE_STRING_FILE_CREATE) == 0); + + assert_se(chmod(name, 0755) == 0); + assert_se(chmod(name2, 0755) == 0); + assert_se(chmod(name3, 0755) == 0); + + r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_environment, args, NULL); + assert_se(r >= 0); + + STRV_FOREACH(p, env) + log_info("got env: \"%s\"", *p); + + assert_se(streq(strv_env_get(env, "A"), "22:23:24")); + assert_se(streq(strv_env_get(env, "B"), "12")); + assert_se(streq(strv_env_get(env, "C"), "001")); + assert_se(endswith(strv_env_get(env, "PATH"), ":/no/such/file")); +} + int main(int argc, char *argv[]) { log_set_max_level(LOG_DEBUG); log_parse_environment(); @@ -280,6 +334,7 @@ int main(int argc, char *argv[]) { test_execute_directory(false); test_execution_order(); test_stdout_gathering(); + test_environment_gathering(); return 0; } -- cgit v1.2.3-54-g00ecf From 184d19047370652184a44686cf85bf5001bdf413 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Fri, 17 Feb 2017 22:56:28 -0500 Subject: Tighten checking for variable validity In the future we might want to allow additional syntax (for example "unset VAR". But let's check that the data we're getting does not contain anything unexpected. --- man/environment.d.xml | 3 +++ src/basic/exec-util.c | 5 +++++ src/basic/fileio.c | 10 ++++++++++ src/test/test-exec-util.c | 12 ++++++++++-- src/test/test-fileio.c | 40 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 68 insertions(+), 2 deletions(-) (limited to 'src/basic/exec-util.c') diff --git a/man/environment.d.xml b/man/environment.d.xml index 2302992fa5..4022c25c36 100644 --- a/man/environment.d.xml +++ b/man/environment.d.xml @@ -81,6 +81,9 @@ and $OTHER_KEY format. No other elements of shell syntax are supported. + EachKEY must be a valid variable name. Empty lines + and lines beginning with the comment character # are ignored. + Example diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c index 207fac8dc1..aced9e8e3d 100644 --- a/src/basic/exec-util.c +++ b/src/basic/exec-util.c @@ -279,6 +279,11 @@ static int gather_environment_generate(int fd, void *arg) { STRV_FOREACH_PAIR(x, y, new) { char *p; + if (!env_name_is_valid(*x)) { + log_warning("Invalid variable assignment \"%s=...\", ignoring.", *x); + continue; + } + p = strjoin(*x, "=", *y); if (!p) return -ENOMEM; diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 3c2dab1855..8185f67e00 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -773,6 +773,16 @@ static int merge_env_file_push( assert(env); + if (!value) { + log_error("%s:%u: invalid syntax (around \"%s\"), ignoring.", strna(filename), line, key); + return 0; + } + + if (!env_name_is_valid(key)) { + log_error("%s:%u: invalid variable name \"%s\", ignoring.", strna(filename), line, key); + return 0; + } + expanded_value = replace_env(value, *env, REPLACE_ENV_USE_ENVIRONMENT|REPLACE_ENV_ALLOW_BRACELESS); if (!expanded_value) diff --git a/src/test/test-exec-util.c b/src/test/test-exec-util.c index 94fe042aa9..482b0751b9 100644 --- a/src/test/test-exec-util.c +++ b/src/test/test-exec-util.c @@ -305,8 +305,16 @@ static void test_environment_gathering(void) { "echo A=$A:24\n" "echo B=12\n" "echo C=000\n" - "echo C=001\n" /* variable overwriting */ - "echo PATH=$PATH:/no/such/file", /* variable from manager */ + "echo C=001\n" /* variable overwriting */ + /* various invalid entries */ + "echo unset A\n" + "echo unset A=\n" + "echo unset A=B\n" + "echo unset \n" + "echo A B=C\n" + "echo A\n" + /* test variable assignment without newline */ + "echo PATH=$PATH:/no/such/file", /* no newline */ WRITE_STRING_FILE_CREATE) == 0); assert_se(chmod(name, 0755) == 0); diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c index c204cbae22..b117335db8 100644 --- a/src/test/test-fileio.c +++ b/src/test/test-fileio.c @@ -263,6 +263,45 @@ static void test_merge_env_file(void) { assert_se(a[6] == NULL); } +static void test_merge_env_file_invalid(void) { + char t[] = "/tmp/test-fileio-XXXXXX"; + int fd, r; + FILE *f; + _cleanup_strv_free_ char **a = NULL; + char **i; + + fd = mkostemp_safe(t); + assert_se(fd >= 0); + + log_info("/* %s (%s) */", __func__, t); + + f = fdopen(fd, "w"); + assert_se(f); + + r = write_string_stream(f, + "unset one \n" + "unset one= \n" + "unset one=1 \n" + "one \n" + "one = \n" + "one two =\n" + "\x20two=\n" + "#comment=comment\n" + ";comment2=comment2\n" + "#\n" + "\n\n" /* empty line */ + , false); + assert(r >= 0); + + r = merge_env_file(&a, NULL, t); + assert_se(r >= 0); + + STRV_FOREACH(i, a) + log_info("Got: <%s>", *i); + + assert_se(strv_isempty(a)); +} + static void test_executable_is_script(void) { char t[] = "/tmp/test-executable-XXXXXX"; int fd, r; @@ -620,6 +659,7 @@ int main(int argc, char *argv[]) { test_parse_env_file(); test_parse_multiline_env_file(); test_merge_env_file(); + test_merge_env_file_invalid(); test_executable_is_script(); test_status_field(); test_capeff(); -- cgit v1.2.3-54-g00ecf