From 992e8f224b91cacc3d6589bea7882c7ab9c0911b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Jul 2016 17:23:28 +0200 Subject: util-lib: rework /tmp and /var/tmp handling code Beef up the existing var_tmp() call, rename it to var_tmp_dir() and add a matching tmp_dir() call (the former looks for the place for /var/tmp, the latter for /tmp). Both calls check $TMPDIR, $TEMP, $TMP, following the algorithm Python3 uses. All dirs are validated before use. secure_getenv() is used in order to limite exposure in suid binaries. This also ports a couple of users over to these new APIs. The var_tmp() return parameter is changed from an allocated buffer the caller will own to a const string either pointing into environ[], or into a static const buffer. Given that environ[] is mostly considered constant (and this is exposed in the very well-known getenv() call), this should be OK behaviour and allows us to avoid memory allocations in most cases. Note that $TMPDIR and friends override both /var/tmp and /tmp usage if set. --- src/basic/fileio.c | 17 ++++++-- src/basic/fs-util.c | 98 +++++++++++++++++++++++++++++++++++--------- src/basic/fs-util.h | 3 +- src/coredump/coredumpctl.c | 9 +++- src/journal/journal-verify.c | 4 +- src/machine/machined-dbus.c | 2 +- src/test/test-fs-util.c | 54 ++++++++++-------------- 7 files changed, 126 insertions(+), 61 deletions(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index f183de4999..6114bf3315 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -1161,8 +1161,8 @@ int tempfn_random_child(const char *p, const char *extra, char **ret) { char *t, *x; uint64_t u; unsigned i; + int r; - assert(p); assert(ret); /* Turns this: @@ -1171,6 +1171,12 @@ int tempfn_random_child(const char *p, const char *extra, char **ret) { * /foo/bar/waldo/.#3c2b6219aa75d7d0 */ + if (!p) { + r = tmp_dir(&p); + if (r < 0) + return r; + } + if (!extra) extra = ""; @@ -1257,10 +1263,13 @@ int fputs_with_space(FILE *f, const char *s, const char *separator, bool *space) int open_tmpfile_unlinkable(const char *directory, int flags) { char *p; - int fd; + int fd, r; - if (!directory) - directory = "/tmp"; + if (!directory) { + r = tmp_dir(&directory); + if (r < 0) + return r; + } /* Returns an unlinked temporary file that cannot be linked into the file system anymore */ diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index f0c6f3265e..ce87257bc1 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -496,34 +496,94 @@ int get_files_in_directory(const char *path, char ***list) { return n; } -int var_tmp(char **ret) { - const char *tmp_dir = NULL; - const char *env_tmp_dir = NULL; - char *c = NULL; - int r; +static int getenv_tmp_dir(const char **ret_path) { + const char *n; + int r, ret = 0; - assert(ret); + assert(ret_path); - env_tmp_dir = getenv("TMPDIR"); - if (env_tmp_dir != NULL) { - r = is_dir(env_tmp_dir, true); - if (r < 0 && r != -ENOENT) - return r; - if (r > 0) - tmp_dir = env_tmp_dir; + /* We use the same order of environment variables python uses in tempfile.gettempdir(): + * https://docs.python.org/3/library/tempfile.html#tempfile.gettempdir */ + FOREACH_STRING(n, "TMPDIR", "TEMP", "TMP") { + const char *e; + + e = secure_getenv(n); + if (!e) + continue; + if (!path_is_absolute(e)) { + r = -ENOTDIR; + goto next; + } + if (!path_is_safe(e)) { + r = -EPERM; + goto next; + } + + r = is_dir(e, true); + if (r < 0) + goto next; + if (r == 0) { + r = -ENOTDIR; + goto next; + } + + *ret_path = e; + return 1; + + next: + /* Remember first error, to make this more debuggable */ + if (ret >= 0) + ret = r; } - if (!tmp_dir) - tmp_dir = "/var/tmp"; + if (ret < 0) + return ret; - c = strdup(tmp_dir); - if (!c) - return -ENOMEM; - *ret = c; + *ret_path = NULL; + return ret; +} +static int tmp_dir_internal(const char *def, const char **ret) { + const char *e; + int r, k; + + assert(def); + assert(ret); + + r = getenv_tmp_dir(&e); + if (r > 0) { + *ret = e; + return 0; + } + + k = is_dir(def, true); + if (k == 0) + k = -ENOTDIR; + if (k < 0) + return r < 0 ? r : k; + + *ret = def; return 0; } +int var_tmp_dir(const char **ret) { + + /* Returns the location for "larger" temporary files, that is backed by physical storage if available, and thus + * even might survive a boot: /var/tmp. If $TMPDIR (or related environment variables) are set, its value is + * returned preferably however. Note that both this function and tmp_dir() below are affected by $TMPDIR, + * making it a variable that overrides all temporary file storage locations. */ + + return tmp_dir_internal("/var/tmp", ret); +} + +int tmp_dir(const char **ret) { + + /* Similar to var_tmp_dir() above, but returns the location for "smaller" temporary files, which is usually + * backed by an in-memory file system: /tmp. */ + + return tmp_dir_internal("/tmp", ret); +} + int inotify_add_watch_fd(int fd, int what, uint32_t mask) { char path[strlen("/proc/self/fd/") + DECIMAL_STR_MAX(int) + 1]; int r; diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h index 075e5942b1..2c3b9a1c74 100644 --- a/src/basic/fs-util.h +++ b/src/basic/fs-util.h @@ -61,7 +61,8 @@ int mkfifo_atomic(const char *path, mode_t mode); int get_files_in_directory(const char *path, char ***list); -int var_tmp(char **ret); +int tmp_dir(const char **ret); +int var_tmp_dir(const char **ret); #define INOTIFY_EVENT_MAX (sizeof(struct inotify_event) + NAME_MAX + 1) diff --git a/src/coredump/coredumpctl.c b/src/coredump/coredumpctl.c index 27b1e0fb3f..bbf8793e57 100644 --- a/src/coredump/coredumpctl.c +++ b/src/coredump/coredumpctl.c @@ -30,6 +30,7 @@ #include "compress.h" #include "fd-util.h" #include "fileio.h" +#include "fs-util.h" #include "journal-internal.h" #include "log.h" #include "macro.h" @@ -609,7 +610,13 @@ static int save_core(sd_journal *j, int fd, char **path, bool *unlink_temp) { char *temp = NULL; if (fd < 0) { - temp = strdup("/var/tmp/coredump-XXXXXX"); + const char *vt; + + r = var_tmp_dir(&vt); + if (r < 0) + return log_error_errno(r, "Failed to acquire temporary directory path: %m"); + + temp = strjoin(vt, "/coredump-XXXXXX", NULL); if (!temp) return log_oom(); diff --git a/src/journal/journal-verify.c b/src/journal/journal-verify.c index f61f158e8a..4105abfccc 100644 --- a/src/journal/journal-verify.c +++ b/src/journal/journal-verify.c @@ -826,7 +826,7 @@ int journal_file_verify( int data_fd = -1, entry_fd = -1, entry_array_fd = -1; unsigned i; bool found_last = false; - _cleanup_free_ char *tmp_dir = NULL; + const char *tmp_dir = NULL; #ifdef HAVE_GCRYPT uint64_t last_tag = 0; @@ -846,7 +846,7 @@ int journal_file_verify( } else if (f->seal) return -ENOKEY; - r = var_tmp(&tmp_dir); + r = var_tmp_dir(&tmp_dir); if (r < 0) { log_error_errno(r, "Failed to determine temporary directory: %m"); goto fail; diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 1923e8b971..5e2462cba2 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -954,7 +954,7 @@ static int method_clean_pool(sd_bus_message *message, void *userdata, sd_bus_err /* Create a temporary file we can dump information about deleted images into. We use a temporary file for this * instead of a pipe or so, since this might grow quit large in theory and we don't want to process this * continuously */ - result_fd = open_tmpfile_unlinkable("/tmp/", O_RDWR|O_CLOEXEC); + result_fd = open_tmpfile_unlinkable(NULL, O_RDWR|O_CLOEXEC); if (result_fd < 0) return -errno; diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index e0c040f39b..93eec3ef9c 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -83,47 +83,35 @@ static void test_get_files_in_directory(void) { } static void test_var_tmp(void) { - char *tmp_dir = NULL; - char *tmpdir_backup = NULL; - const char *default_var_tmp = NULL; - const char *var_name; - bool do_overwrite = true; - - default_var_tmp = "/var/tmp"; - var_name = "TMPDIR"; - - if (getenv(var_name) != NULL) { - tmpdir_backup = strdup(getenv(var_name)); - assert_se(tmpdir_backup != NULL); - } - - unsetenv(var_name); + _cleanup_free_ char *tmpdir_backup = NULL; + const char *tmp_dir = NULL, *t; - var_tmp(&tmp_dir); - assert_se(!strcmp(tmp_dir, default_var_tmp)); - - free(tmp_dir); + t = getenv("TMPDIR"); + if (t) { + tmpdir_backup = strdup(t); + assert_se(tmpdir_backup); + } - setenv(var_name, "/tmp", do_overwrite); - assert_se(!strcmp(getenv(var_name), "/tmp")); + assert(unsetenv("TMPDIR") >= 0); - var_tmp(&tmp_dir); - assert_se(!strcmp(tmp_dir, "/tmp")); + assert_se(var_tmp_dir(&tmp_dir) >= 0); + assert_se(streq(tmp_dir, "/var/tmp")); - free(tmp_dir); + assert_se(setenv("TMPDIR", "/tmp", true) >= 0); + assert_se(streq(getenv("TMPDIR"), "/tmp")); - setenv(var_name, "/88_does_not_exist_88", do_overwrite); - assert_se(!strcmp(getenv(var_name), "/88_does_not_exist_88")); + assert_se(var_tmp_dir(&tmp_dir) >= 0); + assert_se(streq(tmp_dir, "/tmp")); - var_tmp(&tmp_dir); - assert_se(!strcmp(tmp_dir, default_var_tmp)); + assert_se(setenv("TMPDIR", "/88_does_not_exist_88", true) >= 0); + assert_se(streq(getenv("TMPDIR"), "/88_does_not_exist_88")); - free(tmp_dir); + assert_se(var_tmp_dir(&tmp_dir) >= 0); + assert_se(streq(tmp_dir, "/var/tmp")); - if (tmpdir_backup != NULL) { - setenv(var_name, tmpdir_backup, do_overwrite); - assert_se(!strcmp(getenv(var_name), tmpdir_backup)); - free(tmpdir_backup); + if (tmpdir_backup) { + assert_se(setenv("TMPDIR", tmpdir_backup, true) >= 0); + assert_se(streq(getenv("TMPDIR"), tmpdir_backup)); } } -- cgit v1.2.3-54-g00ecf From c39f1ce24ddb1aa683991c5099dcc2afbfcbc57c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Jul 2016 17:40:35 +0200 Subject: core: turn various execution flags into a proper flags parameter The ExecParameters structure contains a number of bit-flags, that were so far exposed as bool:1, change this to a proper, single binary bit flag field. This makes things a bit more expressive, and is helpful as we add more flags, since these booleans are passed around in various callers, for example service_spawn(), whose signature can be made much shorter now. Not all bit booleans from ExecParameters are moved into the flags field for now, but this can be added later. --- src/core/execute.c | 21 ++++++++----- src/core/execute.h | 17 ++++++++--- src/core/mount.c | 12 +++----- src/core/service.c | 90 +++++++++++++++++++----------------------------------- src/core/socket.c | 12 +++----- src/core/swap.c | 12 +++----- 6 files changed, 72 insertions(+), 92 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index 77a75245cb..bc0fd27402 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -427,7 +427,7 @@ static int setup_input( return STDIN_FILENO; } - i = fixup_input(context->std_input, socket_fd, params->apply_tty_stdin); + i = fixup_input(context->std_input, socket_fd, params->flags & EXEC_APPLY_TTY_STDIN); switch (i) { @@ -502,7 +502,7 @@ static int setup_output( return STDERR_FILENO; } - i = fixup_input(context->std_input, socket_fd, params->apply_tty_stdin); + i = fixup_input(context->std_input, socket_fd, params->flags & EXEC_APPLY_TTY_STDIN); o = fixup_output(context->std_output, socket_fd); if (fileno == STDERR_FILENO) { @@ -1675,7 +1675,7 @@ static int exec_child( exec_context_tty_reset(context, params); - if (params->confirm_spawn) { + if (params->flags & EXEC_CONFIRM_SPAWN) { char response; r = ask_for_confirmation(&response, argv); @@ -1940,7 +1940,7 @@ static int exec_child( umask(context->umask); - if (params->apply_permissions && !command->privileged) { + if ((params->flags & EXEC_APPLY_PERMISSIONS) && !command->privileged) { r = enforce_groups(context, username, gid); if (r < 0) { *exit_status = EXIT_GROUP; @@ -2010,7 +2010,7 @@ static int exec_child( } r = setup_namespace( - params->apply_chroot ? context->root_directory : NULL, + (params->flags & EXEC_APPLY_CHROOT) ? context->root_directory : NULL, context->read_write_paths, context->read_only_paths, context->inaccessible_paths, @@ -2041,7 +2041,7 @@ static int exec_child( else wd = "/"; - if (params->apply_chroot) { + if (params->flags & EXEC_APPLY_CHROOT) { if (!needs_mount_namespace && context->root_directory) if (chroot(context->root_directory) < 0) { *exit_status = EXIT_CHROOT; @@ -2065,7 +2065,12 @@ static int exec_child( } #ifdef HAVE_SELINUX - if (params->apply_permissions && mac_selinux_use() && params->selinux_context_net && socket_fd >= 0 && !command->privileged) { + if ((params->flags & EXEC_APPLY_PERMISSIONS) && + mac_selinux_use() && + params->selinux_context_net && + socket_fd >= 0 && + !command->privileged) { + r = mac_selinux_get_child_mls_label(socket_fd, command->path, context->selinux_context, &mac_selinux_context_net); if (r < 0) { *exit_status = EXIT_SELINUX_CONTEXT; @@ -2090,7 +2095,7 @@ static int exec_child( return r; } - if (params->apply_permissions && !command->privileged) { + if ((params->flags & EXEC_APPLY_PERMISSIONS) && !command->privileged) { bool use_address_families = context->address_families_whitelist || !set_isempty(context->address_families); diff --git a/src/core/execute.h b/src/core/execute.h index 48cc18fbb3..77418ea2ad 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -208,6 +208,17 @@ struct ExecContext { bool no_new_privileges_set:1; }; +typedef enum ExecFlags { + EXEC_CONFIRM_SPAWN = 1U << 0, + EXEC_APPLY_PERMISSIONS = 1U << 1, + EXEC_APPLY_CHROOT = 1U << 2, + EXEC_APPLY_TTY_STDIN = 1U << 3, + + /* The following are not usec by execute.c, but by consumers internally */ + EXEC_PASS_FDS = 1U << 4, + EXEC_IS_CONTROL = 1U << 5, +} ExecFlags; + struct ExecParameters { char **argv; char **environment; @@ -216,11 +227,7 @@ struct ExecParameters { char **fd_names; unsigned n_fds; - bool apply_permissions:1; - bool apply_chroot:1; - bool apply_tty_stdin:1; - - bool confirm_spawn:1; + ExecFlags flags; bool selinux_context_net:1; bool cgroup_delegate:1; diff --git a/src/core/mount.c b/src/core/mount.c index afb20af9e2..3f6ac7fcf9 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -701,12 +701,10 @@ static int mount_spawn(Mount *m, ExecCommand *c, pid_t *_pid) { pid_t pid; int r; ExecParameters exec_params = { - .apply_permissions = true, - .apply_chroot = true, - .apply_tty_stdin = true, - .stdin_fd = -1, - .stdout_fd = -1, - .stderr_fd = -1, + .flags = EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN, + .stdin_fd = -1, + .stdout_fd = -1, + .stderr_fd = -1, }; assert(m); @@ -732,7 +730,7 @@ static int mount_spawn(Mount *m, ExecCommand *c, pid_t *_pid) { return r; exec_params.environment = UNIT(m)->manager->environment; - exec_params.confirm_spawn = UNIT(m)->manager->confirm_spawn; + exec_params.flags |= UNIT(m)->manager->confirm_spawn ? EXEC_CONFIRM_SPAWN : 0; exec_params.cgroup_supported = UNIT(m)->manager->cgroup_supported; exec_params.cgroup_path = UNIT(m)->cgroup_path; exec_params.cgroup_delegate = m->cgroup_context.delegate; diff --git a/src/core/service.c b/src/core/service.c index eb125cb999..b4db7d17ed 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1152,11 +1152,7 @@ static int service_spawn( Service *s, ExecCommand *c, usec_t timeout, - bool pass_fds, - bool apply_permissions, - bool apply_chroot, - bool apply_tty_stdin, - bool is_control, + ExecFlags flags, pid_t *_pid) { _cleanup_strv_free_ char **argv = NULL, **final_env = NULL, **our_env = NULL, **fd_names = NULL; @@ -1166,12 +1162,10 @@ static int service_spawn( pid_t pid; ExecParameters exec_params = { - .apply_permissions = apply_permissions, - .apply_chroot = apply_chroot, - .apply_tty_stdin = apply_tty_stdin, - .stdin_fd = -1, - .stdout_fd = -1, - .stderr_fd = -1, + .flags = flags, + .stdin_fd = -1, + .stdout_fd = -1, + .stderr_fd = -1, }; int r; @@ -1194,7 +1188,7 @@ static int service_spawn( if (r < 0) return r; - if (pass_fds || + if ((flags & EXEC_PASS_FDS) || s->exec_context.std_input == EXEC_INPUT_SOCKET || s->exec_context.std_output == EXEC_OUTPUT_SOCKET || s->exec_context.std_error == EXEC_OUTPUT_SOCKET) { @@ -1218,7 +1212,7 @@ static int service_spawn( if (!our_env) return -ENOMEM; - if (is_control ? s->notify_access == NOTIFY_ALL : s->notify_access != NOTIFY_NONE) + if ((flags & EXEC_IS_CONTROL) ? s->notify_access == NOTIFY_ALL : s->notify_access != NOTIFY_NONE) if (asprintf(our_env + n_env++, "NOTIFY_SOCKET=%s", UNIT(s)->manager->notify_socket) < 0) return -ENOMEM; @@ -1226,7 +1220,7 @@ static int service_spawn( if (asprintf(our_env + n_env++, "MAINPID="PID_FMT, s->main_pid) < 0) return -ENOMEM; - if (!MANAGER_IS_SYSTEM(UNIT(s)->manager)) + if (MANAGER_IS_USER(UNIT(s)->manager)) if (asprintf(our_env + n_env++, "MANAGERPID="PID_FMT, getpid()) < 0) return -ENOMEM; @@ -1266,18 +1260,18 @@ static int service_spawn( if (!final_env) return -ENOMEM; - if (is_control && UNIT(s)->cgroup_path) { + if ((flags & EXEC_IS_CONTROL) && UNIT(s)->cgroup_path) { path = strjoina(UNIT(s)->cgroup_path, "/control"); (void) cg_create(SYSTEMD_CGROUP_CONTROLLER, path); } else path = UNIT(s)->cgroup_path; exec_params.argv = argv; + exec_params.environment = final_env; exec_params.fds = fds; exec_params.fd_names = fd_names; exec_params.n_fds = n_fds; - exec_params.environment = final_env; - exec_params.confirm_spawn = UNIT(s)->manager->confirm_spawn; + exec_params.flags |= UNIT(s)->manager->confirm_spawn ? EXEC_CONFIRM_SPAWN : 0; exec_params.cgroup_supported = UNIT(s)->manager->cgroup_supported; exec_params.cgroup_path = path; exec_params.cgroup_delegate = s->cgroup_context.delegate; @@ -1465,11 +1459,9 @@ static void service_enter_stop_post(Service *s, ServiceResult f) { r = service_spawn(s, s->control_command, s->timeout_stop_usec, - false, - !s->permissions_start_only, - !s->root_directory_start_only, - true, - true, + (s->permissions_start_only ? 0 : EXEC_APPLY_PERMISSIONS) | + (s->root_directory_start_only ? 0 : EXEC_APPLY_CHROOT) | + EXEC_APPLY_TTY_STDIN | EXEC_IS_CONTROL, &s->control_pid); if (r < 0) goto fail; @@ -1580,11 +1572,9 @@ static void service_enter_stop(Service *s, ServiceResult f) { r = service_spawn(s, s->control_command, s->timeout_stop_usec, - false, - !s->permissions_start_only, - !s->root_directory_start_only, - false, - true, + (s->permissions_start_only ? 0 : EXEC_APPLY_PERMISSIONS) | + (s->root_directory_start_only ? 0 : EXEC_APPLY_CHROOT) | + EXEC_IS_CONTROL, &s->control_pid); if (r < 0) goto fail; @@ -1661,11 +1651,9 @@ static void service_enter_start_post(Service *s) { r = service_spawn(s, s->control_command, s->timeout_start_usec, - false, - !s->permissions_start_only, - !s->root_directory_start_only, - false, - true, + (s->permissions_start_only ? 0 : EXEC_APPLY_PERMISSIONS)| + (s->root_directory_start_only ? 0 : EXEC_APPLY_CHROOT)| + EXEC_IS_CONTROL, &s->control_pid); if (r < 0) goto fail; @@ -1735,11 +1723,7 @@ static void service_enter_start(Service *s) { r = service_spawn(s, c, timeout, - true, - true, - true, - true, - false, + EXEC_PASS_FDS|EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN, &pid); if (r < 0) goto fail; @@ -1798,11 +1782,9 @@ static void service_enter_start_pre(Service *s) { r = service_spawn(s, s->control_command, s->timeout_start_usec, - false, - !s->permissions_start_only, - !s->root_directory_start_only, - true, - true, + (s->permissions_start_only ? 0 : EXEC_APPLY_PERMISSIONS) | + (s->root_directory_start_only ? 0: EXEC_APPLY_CHROOT) | + EXEC_IS_CONTROL|EXEC_APPLY_TTY_STDIN, &s->control_pid); if (r < 0) goto fail; @@ -1877,11 +1859,9 @@ static void service_enter_reload(Service *s) { r = service_spawn(s, s->control_command, s->timeout_start_usec, - false, - !s->permissions_start_only, - !s->root_directory_start_only, - false, - true, + (s->permissions_start_only ? 0 : EXEC_APPLY_PERMISSIONS) | + (s->root_directory_start_only ? 0 : EXEC_APPLY_CHROOT) | + EXEC_IS_CONTROL, &s->control_pid); if (r < 0) goto fail; @@ -1919,12 +1899,10 @@ static void service_run_next_control(Service *s) { r = service_spawn(s, s->control_command, timeout, - false, - !s->permissions_start_only, - !s->root_directory_start_only, - s->control_command_id == SERVICE_EXEC_START_PRE || - s->control_command_id == SERVICE_EXEC_STOP_POST, - true, + (s->permissions_start_only ? 0 : EXEC_APPLY_PERMISSIONS) | + (s->root_directory_start_only ? 0 : EXEC_APPLY_CHROOT) | + (IN_SET(s->control_command_id, SERVICE_EXEC_START_PRE, SERVICE_EXEC_STOP_POST) ? EXEC_APPLY_TTY_STDIN : 0)| + EXEC_IS_CONTROL, &s->control_pid); if (r < 0) goto fail; @@ -1962,11 +1940,7 @@ static void service_run_next_main(Service *s) { r = service_spawn(s, s->main_command, s->timeout_start_usec, - true, - true, - true, - true, - false, + EXEC_PASS_FDS|EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN, &pid); if (r < 0) goto fail; diff --git a/src/core/socket.c b/src/core/socket.c index ff55885fb3..82363e2157 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1664,12 +1664,10 @@ static int socket_spawn(Socket *s, ExecCommand *c, pid_t *_pid) { pid_t pid; int r; ExecParameters exec_params = { - .apply_permissions = true, - .apply_chroot = true, - .apply_tty_stdin = true, - .stdin_fd = -1, - .stdout_fd = -1, - .stderr_fd = -1, + .flags = EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN, + .stdin_fd = -1, + .stdout_fd = -1, + .stderr_fd = -1, }; assert(s); @@ -1700,7 +1698,7 @@ static int socket_spawn(Socket *s, ExecCommand *c, pid_t *_pid) { exec_params.argv = argv; exec_params.environment = UNIT(s)->manager->environment; - exec_params.confirm_spawn = UNIT(s)->manager->confirm_spawn; + exec_params.flags |= UNIT(s)->manager->confirm_spawn ? EXEC_CONFIRM_SPAWN : 0; exec_params.cgroup_supported = UNIT(s)->manager->cgroup_supported; exec_params.cgroup_path = UNIT(s)->cgroup_path; exec_params.cgroup_delegate = s->cgroup_context.delegate; diff --git a/src/core/swap.c b/src/core/swap.c index 66a318d01f..0ba4c4d881 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -611,12 +611,10 @@ static int swap_spawn(Swap *s, ExecCommand *c, pid_t *_pid) { pid_t pid; int r; ExecParameters exec_params = { - .apply_permissions = true, - .apply_chroot = true, - .apply_tty_stdin = true, - .stdin_fd = -1, - .stdout_fd = -1, - .stderr_fd = -1, + .flags = EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN, + .stdin_fd = -1, + .stdout_fd = -1, + .stderr_fd = -1, }; assert(s); @@ -642,7 +640,7 @@ static int swap_spawn(Swap *s, ExecCommand *c, pid_t *_pid) { goto fail; exec_params.environment = UNIT(s)->manager->environment; - exec_params.confirm_spawn = UNIT(s)->manager->confirm_spawn; + exec_params.flags |= UNIT(s)->manager->confirm_spawn ? EXEC_CONFIRM_SPAWN : 0; exec_params.cgroup_supported = UNIT(s)->manager->cgroup_supported; exec_params.cgroup_path = UNIT(s)->cgroup_path; exec_params.cgroup_delegate = s->cgroup_context.delegate; -- cgit v1.2.3-54-g00ecf From 9c1a61adba1ed61a405bc30675f08b8442eefd70 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Jul 2016 17:53:07 +0200 Subject: core: move masking of chroot/permission masking into service_spawn() Let's fix up the flags fields in service_spawn() rather than its callers, in order to simplify things a bit. --- src/core/execute.h | 2 +- src/core/service.c | 34 +++++++++++++++------------------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/core/execute.h b/src/core/execute.h index 77418ea2ad..8d659ca178 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -214,7 +214,7 @@ typedef enum ExecFlags { EXEC_APPLY_CHROOT = 1U << 2, EXEC_APPLY_TTY_STDIN = 1U << 3, - /* The following are not usec by execute.c, but by consumers internally */ + /* The following are not used by execute.c, but by consumers internally */ EXEC_PASS_FDS = 1U << 4, EXEC_IS_CONTROL = 1U << 5, } ExecFlags; diff --git a/src/core/service.c b/src/core/service.c index b4db7d17ed..32b8e7d2c5 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1174,6 +1174,14 @@ static int service_spawn( assert(c); assert(_pid); + if (flags & EXEC_IS_CONTROL) { + /* If this is a control process, mask the permissions/chroot application if this is requested. */ + if (s->permissions_start_only) + exec_params.flags &= ~EXEC_APPLY_PERMISSIONS; + if (s->root_directory_start_only) + exec_params.flags &= ~EXEC_APPLY_CHROOT; + } + (void) unit_realize_cgroup(UNIT(s)); if (s->reset_cpu_usage) { (void) unit_reset_cpu_usage(UNIT(s)); @@ -1459,9 +1467,7 @@ static void service_enter_stop_post(Service *s, ServiceResult f) { r = service_spawn(s, s->control_command, s->timeout_stop_usec, - (s->permissions_start_only ? 0 : EXEC_APPLY_PERMISSIONS) | - (s->root_directory_start_only ? 0 : EXEC_APPLY_CHROOT) | - EXEC_APPLY_TTY_STDIN | EXEC_IS_CONTROL, + EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN|EXEC_IS_CONTROL, &s->control_pid); if (r < 0) goto fail; @@ -1572,9 +1578,7 @@ static void service_enter_stop(Service *s, ServiceResult f) { r = service_spawn(s, s->control_command, s->timeout_stop_usec, - (s->permissions_start_only ? 0 : EXEC_APPLY_PERMISSIONS) | - (s->root_directory_start_only ? 0 : EXEC_APPLY_CHROOT) | - EXEC_IS_CONTROL, + EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_IS_CONTROL, &s->control_pid); if (r < 0) goto fail; @@ -1651,9 +1655,7 @@ static void service_enter_start_post(Service *s) { r = service_spawn(s, s->control_command, s->timeout_start_usec, - (s->permissions_start_only ? 0 : EXEC_APPLY_PERMISSIONS)| - (s->root_directory_start_only ? 0 : EXEC_APPLY_CHROOT)| - EXEC_IS_CONTROL, + EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_IS_CONTROL, &s->control_pid); if (r < 0) goto fail; @@ -1782,9 +1784,7 @@ static void service_enter_start_pre(Service *s) { r = service_spawn(s, s->control_command, s->timeout_start_usec, - (s->permissions_start_only ? 0 : EXEC_APPLY_PERMISSIONS) | - (s->root_directory_start_only ? 0: EXEC_APPLY_CHROOT) | - EXEC_IS_CONTROL|EXEC_APPLY_TTY_STDIN, + EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_IS_CONTROL|EXEC_APPLY_TTY_STDIN, &s->control_pid); if (r < 0) goto fail; @@ -1859,9 +1859,7 @@ static void service_enter_reload(Service *s) { r = service_spawn(s, s->control_command, s->timeout_start_usec, - (s->permissions_start_only ? 0 : EXEC_APPLY_PERMISSIONS) | - (s->root_directory_start_only ? 0 : EXEC_APPLY_CHROOT) | - EXEC_IS_CONTROL, + EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_IS_CONTROL, &s->control_pid); if (r < 0) goto fail; @@ -1899,10 +1897,8 @@ static void service_run_next_control(Service *s) { r = service_spawn(s, s->control_command, timeout, - (s->permissions_start_only ? 0 : EXEC_APPLY_PERMISSIONS) | - (s->root_directory_start_only ? 0 : EXEC_APPLY_CHROOT) | - (IN_SET(s->control_command_id, SERVICE_EXEC_START_PRE, SERVICE_EXEC_STOP_POST) ? EXEC_APPLY_TTY_STDIN : 0)| - EXEC_IS_CONTROL, + EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_IS_CONTROL| + (IN_SET(s->control_command_id, SERVICE_EXEC_START_PRE, SERVICE_EXEC_STOP_POST) ? EXEC_APPLY_TTY_STDIN : 0), &s->control_pid); if (r < 0) goto fail; -- cgit v1.2.3-54-g00ecf From af9d16e10a23899b821af19e54e339486a86bd82 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 27 Jul 2016 11:50:37 +0200 Subject: core: use the correct APIs to determine whether a dual timestamp is initialized --- src/core/execute.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index bc0fd27402..9028139723 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -2969,12 +2969,12 @@ void exec_status_dump(ExecStatus *s, FILE *f, const char *prefix) { "%sPID: "PID_FMT"\n", prefix, s->pid); - if (s->start_timestamp.realtime > 0) + if (dual_timestamp_is_set(&s->start_timestamp)) fprintf(f, "%sStart Timestamp: %s\n", prefix, format_timestamp(buf, sizeof(buf), s->start_timestamp.realtime)); - if (s->exit_timestamp.realtime > 0) + if (dual_timestamp_is_set(&s->exit_timestamp)) fprintf(f, "%sExit Timestamp: %s\n" "%sExit Code: %s\n" -- cgit v1.2.3-54-g00ecf From 136dc4c4350dfcb1d61ceaac982d0d0a8e6e3863 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 27 Jul 2016 11:51:11 +0200 Subject: core: set $SERVICE_RESULT, $EXIT_CODE and $EXIT_STATUS in ExecStop=/ExecStopPost= commands This should simplify monitoring tools for services, by passing the most basic information about service result/exit information via environment variables, thus making it unnecessary to retrieve them explicitly via the bus. --- man/systemd.exec.xml | 37 +++++++++++++++++++++++++++++++++++++ man/systemd.service.xml | 8 +++++++- src/core/execute.h | 1 + src/core/service.c | 27 +++++++++++++++++++++++---- 4 files changed, 68 insertions(+), 5 deletions(-) diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index 58ba582911..0fc658f180 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -1602,6 +1602,43 @@ functions) if their standard output or standard error output is connected to the journal anyway, thus enabling delivery of structured metadata along with logged messages. + + + $SERVICE_RESULT + + Only defined for the service unit type, this environment variable is passed to all + ExecStop= and ExecStopPost= processes, and encodes the service + "result". Currently, the following values are defined: timeout (in case of an operation + timeout), exit-code (if a service process exited with a non-zero exit code; see + $EXIT_STATUS below for the actual exit status returned), signal (if a + service process was terminated abnormally by a signal; see $EXIT_STATUS below for the actual + signal used for the termination), core-dump (if a service process terminated abnormally and + dumped core), watchdog (if the watchdog keep-alive ping was enabled for the service but it + missed the deadline), or resources (a catch-all condition in case a system operation + failed). + + This environment variable is useful to monitor failure or successful termination of a service. Even + though this variable is available in both ExecStop= and ExecStopPost=, it + is usually a better choice to place monitoring tools in the latter, as the former is only invoked for services + that managed to start up correctly, and the latter covers both services that failed during their start-up and + those which failed during their runtime. + + + + $EXIT_CODE + $EXIT_STATUS + + Only defined for the service unit type, these environment variables are passed to all + ExecStop=, ExecStopPost= processes and contain exit status/code + information of the main process of the service. For the precise definition of the exit code and status, see + wait2. $EXIT_CODE + is one of exited, killed, + dumped. $EXIT_STATUS contains the numeric exit code formatted as string + if $EXIT_CODE is exited, and the signal name in all other cases. Note + that these environment variables are only set if the service manager succeeded to start and identify the main + process of the service. + + Additional variables may be configured by the following diff --git a/man/systemd.service.xml b/man/systemd.service.xml index 875d368fcf..e82edbe93e 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -429,7 +429,13 @@ service failed to start up correctly. Commands configured with this setting need to be able to operate even if the service failed starting up half-way and left incompletely initialized data around. As the service's processes have been terminated already when the commands specified with this setting are executed they should - not attempt to communicate with them. + not attempt to communicate with them. + + Note that all commands that are configured with this setting are invoked with the result code of the + service, as well as the main process' exit code and status, set in the $SERVICE_RESULT, + $EXIT_CODE and $EXIT_STATUS environment variables, see + systemd.exec5 for + details. diff --git a/src/core/execute.h b/src/core/execute.h index 8d659ca178..2b4238ed7e 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -217,6 +217,7 @@ typedef enum ExecFlags { /* The following are not used by execute.c, but by consumers internally */ EXEC_PASS_FDS = 1U << 4, EXEC_IS_CONTROL = 1U << 5, + EXEC_SETENV_RESULT = 1U << 6, } ExecFlags; struct ExecParameters { diff --git a/src/core/service.c b/src/core/service.c index 32b8e7d2c5..0cbea52276 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1216,7 +1216,7 @@ static int service_spawn( if (r < 0) return r; - our_env = new0(char*, 6); + our_env = new0(char*, 9); if (!our_env) return -ENOMEM; @@ -1264,6 +1264,24 @@ static int service_spawn( } } + if (flags & EXEC_SETENV_RESULT) { + if (asprintf(our_env + n_env++, "SERVICE_RESULT=%s", service_result_to_string(s->result)) < 0) + return -ENOMEM; + + if (s->main_exec_status.pid > 0 && + dual_timestamp_is_set(&s->main_exec_status.exit_timestamp)) { + if (asprintf(our_env + n_env++, "EXIT_CODE=%s", sigchld_code_to_string(s->main_exec_status.code)) < 0) + return -ENOMEM; + + if (s->main_exec_status.code == CLD_EXITED) + r = asprintf(our_env + n_env++, "EXIT_STATUS=%i", s->main_exec_status.status); + else + r = asprintf(our_env + n_env++, "EXIT_STATUS=%s", signal_to_string(s->main_exec_status.status)); + if (r < 0) + return -ENOMEM; + } + } + final_env = strv_env_merge(2, UNIT(s)->manager->environment, our_env, NULL); if (!final_env) return -ENOMEM; @@ -1467,7 +1485,7 @@ static void service_enter_stop_post(Service *s, ServiceResult f) { r = service_spawn(s, s->control_command, s->timeout_stop_usec, - EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN|EXEC_IS_CONTROL, + EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN|EXEC_IS_CONTROL|EXEC_SETENV_RESULT, &s->control_pid); if (r < 0) goto fail; @@ -1578,7 +1596,7 @@ static void service_enter_stop(Service *s, ServiceResult f) { r = service_spawn(s, s->control_command, s->timeout_stop_usec, - EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_IS_CONTROL, + EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_IS_CONTROL|EXEC_SETENV_RESULT, &s->control_pid); if (r < 0) goto fail; @@ -1898,7 +1916,8 @@ static void service_run_next_control(Service *s) { s->control_command, timeout, EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_IS_CONTROL| - (IN_SET(s->control_command_id, SERVICE_EXEC_START_PRE, SERVICE_EXEC_STOP_POST) ? EXEC_APPLY_TTY_STDIN : 0), + (IN_SET(s->control_command_id, SERVICE_EXEC_START_PRE, SERVICE_EXEC_STOP_POST) ? EXEC_APPLY_TTY_STDIN : 0)| + (IN_SET(s->control_command_id, SERVICE_EXEC_STOP, SERVICE_EXEC_STOP_POST) ? EXEC_SETENV_RESULT : 0), &s->control_pid); if (r < 0) goto fail; -- cgit v1.2.3-54-g00ecf From 2129011e92feee93383b450bb2d20008786b90a0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 27 Jul 2016 13:14:01 +0200 Subject: nss-systemd: resolve root/nobody statically Let's extend nss-systemd to also synthesize user/group entries for the UIDs/GIDs 0 and 65534 which have special kernel meaning. Given that nss-systemd is listed in /etc/nsswitch.conf only very late any explicit listing in /etc/passwd or /etc/group takes precedence. This functionality is useful in minimal container-like setups that lack /etc/passwd files (or only have incompletely populated ones). --- configure.ac | 22 ++++++++- man/nss-systemd.xml | 4 ++ src/nss-systemd/nss-systemd.c | 101 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index a86deca471..4d1c96606f 100644 --- a/configure.ac +++ b/configure.ac @@ -556,12 +556,30 @@ AC_SUBST(CERTIFICATEROOT) AC_ARG_WITH([support-url], AS_HELP_STRING([--with-support-url=URL], - [Specify the supoport URL to show in catalog entries included in systemd]), + [Specify the support URL to show in catalog entries included in systemd]), [SUPPORT_URL="$withval"], [SUPPORT_URL=http://lists.freedesktop.org/mailman/listinfo/systemd-devel]) AC_SUBST(SUPPORT_URL) +AC_ARG_WITH([nobody-user], + AS_HELP_STRING([--with-nobody-user=NAME], + [Specify the name of the nobody user (the one with UID 65534)]), + [NOBODY_USER_NAME="$withval"], + [NOBODY_USER_NAME=nobody]) + +AC_SUBST(NOBODY_USER_NAME) +AC_DEFINE_UNQUOTED(NOBODY_USER_NAME, ["$NOBODY_USER_NAME"], [The name of the nobody user (the one with UID 65534)]) + +AC_ARG_WITH([nobody-group], + AS_HELP_STRING([--with-nobody-group=NAME], + [Specify the name of the nobody group (the one with GID 65534)]), + [NOBODY_GROUP_NAME="$withval"], + [NOBODY_GROUP_NAME=nobody]) + +AC_SUBST(NOBODY_GROUP_NAME) +AC_DEFINE_UNQUOTED(NOBODY_GROUP_NAME, ["$NOBODY_GROUP_NAME"], [The name of the nobody group (the one with GID 65534)]) + # ------------------------------------------------------------------------------ have_xz=no AC_ARG_ENABLE(xz, AS_HELP_STRING([--disable-xz], [Disable optional XZ support])) @@ -1677,6 +1695,8 @@ AC_MSG_RESULT([ Maximum System GID: ${SYSTEM_GID_MAX} Certificate root: ${CERTIFICATEROOT} Support URL: ${SUPPORT_URL} + Nobody User Name: ${NOBODY_USER_NAME} + Nobody Group Name: ${NOBODY_GROUP_NAME} CFLAGS: ${OUR_CFLAGS} ${CFLAGS} CPPFLAGS: ${OUR_CPPFLAGS} ${CPPFLAGS} diff --git a/man/nss-systemd.xml b/man/nss-systemd.xml index 4228372e51..56d26e7d1f 100644 --- a/man/nss-systemd.xml +++ b/man/nss-systemd.xml @@ -61,6 +61,10 @@ systemd.exec5 for details on this option. + This module also ensures that the root and nobody users and groups (i.e. the users/groups with the UIDs/GIDs + 0 and 65534) remain resolvable at all times, even if they aren't listed in /etc/passwd or + /etc/group, or if these files are missing. + To activate the NSS module, add systemd to the lines starting with passwd: and group: in /etc/nsswitch.conf. diff --git a/src/nss-systemd/nss-systemd.c b/src/nss-systemd/nss-systemd.c index e7a4393bb0..7078c0c50c 100644 --- a/src/nss-systemd/nss-systemd.c +++ b/src/nss-systemd/nss-systemd.c @@ -26,9 +26,52 @@ #include "macro.h" #include "nss-util.h" #include "signal-util.h" +#include "string-util.h" #include "user-util.h" #include "util.h" +#ifndef NOBODY_USER_NAME +#define NOBODY_USER_NAME "nobody" +#endif + +#ifndef NOBODY_GROUP_NAME +#define NOBODY_GROUP_NAME "nobody" +#endif + +static const struct passwd root_passwd = { + .pw_name = (char*) "root", + .pw_passwd = (char*) "x", /* see shadow file */ + .pw_uid = 0, + .pw_gid = 0, + .pw_gecos = (char*) "Super User", + .pw_dir = (char*) "/root", + .pw_shell = (char*) "/bin/sh", +}; + +static const struct passwd nobody_passwd = { + .pw_name = (char*) NOBODY_USER_NAME, + .pw_passwd = (char*) "*", /* locked */ + .pw_uid = 65534, + .pw_gid = 65534, + .pw_gecos = (char*) "User Nobody", + .pw_dir = (char*) "/", + .pw_shell = (char*) "/sbin/nologin", +}; + +static const struct group root_group = { + .gr_name = (char*) "root", + .gr_gid = 0, + .gr_passwd = (char*) "x", /* see shadow file */ + .gr_mem = (char*[]) { NULL }, +}; + +static const struct group nobody_group = { + .gr_name = (char*) NOBODY_GROUP_NAME, + .gr_gid = 65534, + .gr_passwd = (char*) "*", /* locked */ + .gr_mem = (char*[]) { NULL }, +}; + NSS_GETPW_PROTOTYPES(systemd); NSS_GETGR_PROTOTYPES(systemd); @@ -50,6 +93,23 @@ enum nss_status _nss_systemd_getpwnam_r( assert(name); assert(pwd); + if (!valid_user_group_name(name)) { + r = -EINVAL; + goto fail; + } + + /* Synthesize entries for the root and nobody users, in case they are missing in /etc/passwd */ + if (streq(name, root_passwd.pw_name)) { + *pwd = root_passwd; + *errnop = 0; + return NSS_STATUS_SUCCESS; + } + if (streq(name, nobody_passwd.pw_name)) { + *pwd = nobody_passwd; + *errnop = 0; + return NSS_STATUS_SUCCESS; + } + /* Make sure that we don't go in circles when allocating a dynamic UID by checking our own database */ if (getenv_bool("SYSTEMD_NSS_DYNAMIC_BYPASS") > 0) goto not_found; @@ -126,6 +186,18 @@ enum nss_status _nss_systemd_getpwuid_r( goto fail; } + /* Synthesize data for the root user and for nobody in case they are missing from /etc/passwd */ + if (uid == root_passwd.pw_uid) { + *pwd = root_passwd; + *errnop = 0; + return NSS_STATUS_SUCCESS; + } + if (uid == nobody_passwd.pw_uid) { + *pwd = nobody_passwd; + *errnop = 0; + return NSS_STATUS_SUCCESS; + } + if (uid <= SYSTEM_UID_MAX) goto not_found; @@ -202,6 +274,23 @@ enum nss_status _nss_systemd_getgrnam_r( assert(name); assert(gr); + if (!valid_user_group_name(name)) { + r = -EINVAL; + goto fail; + } + + /* Synthesize records for root and nobody, in case they are missing form /etc/group */ + if (streq(name, root_group.gr_name)) { + *gr = root_group; + *errnop = 0; + return NSS_STATUS_SUCCESS; + } + if (streq(name, nobody_group.gr_name)) { + *gr = nobody_group; + *errnop = 0; + return NSS_STATUS_SUCCESS; + } + if (getenv_bool("SYSTEMD_NSS_DYNAMIC_BYPASS") > 0) goto not_found; @@ -275,6 +364,18 @@ enum nss_status _nss_systemd_getgrgid_r( goto fail; } + /* Synthesize records for root and nobody, in case they are missing from /etc/group */ + if (gid == root_group.gr_gid) { + *gr = root_group; + *errnop = 0; + return NSS_STATUS_SUCCESS; + } + if (gid == nobody_group.gr_gid) { + *gr = nobody_group; + *errnop = 0; + return NSS_STATUS_SUCCESS; + } + if (gid <= SYSTEM_GID_MAX) goto not_found; -- cgit v1.2.3-54-g00ecf From d47f681b285b1dfb7ce68200205bfe8b835657a2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 27 Jul 2016 13:30:58 +0200 Subject: update TODO --- TODO | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/TODO b/TODO index bb36522bf9..723292cde0 100644 --- a/TODO +++ b/TODO @@ -35,27 +35,17 @@ Features: * RemoveIPC= in unit files for removing POSIX/SysV IPC objects -* Set SERVICE_RESULT= as env var while running ExecStop= - * Introduce ProtectSystem=strict for making the entire OS hierarchy read-only except for a select few * nspawn: start UID allocation loop from hash of container name -* in the DynamicUser=1 nss module, also map "nobody" and "root" statically - -* pid1: log about all processes we kill with with SIGKILL or in abandoned scopes, as this should normally not happen - * nspawn: support that /proc, /sys/, /dev are pre-mounted -* nspawn: mount esp, so that bootctl can work - * define gpt header bits to select volatility mode * nspawn: mount loopback filesystems with "discard" -* Make TasksMax= take percentages, taken relative to the pids_max sysctl and pids.max cgroup limit - * ProtectKernelLogs= (drops CAP_SYSLOG, add seccomp for syslog() syscall, and DeviceAllow to /dev/kmsg) in service files * ProtectClock= (drops CAP_SYS_TIMES, adds seecomp filters for settimeofday, adjtimex), sets DeviceAllow o /dev/rtc -- cgit v1.2.3-54-g00ecf From a0fef983ab200db4e2b151beb06c9cf8fef6c5ab Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 4 Aug 2016 21:14:27 +0200 Subject: core: remember first unit failure, not last unit failure Previously, the result value of a unit was overriden with each failure that took place, so that the result always reported the last failure that took place. With this commit this is changed, so that the first failure taking place is stored instead. This should normally not matter much as multiple failures are sufficiently uncommon. However, it improves one behaviour: if we send SIGABRT to a service due to a watchdog timeout, then this currently would be reported as "coredump" failure, rather than the "watchodg" failure it really is. Hence, in order to report information about the type of the failure, and not about the effect of it, let's change this from all unit type to store the first, not the last failure. This addresses the issue pointed out here: https://github.com/systemd/systemd/pull/3818#discussion_r73433520 --- src/core/automount.c | 2 +- src/core/busname.c | 6 +++--- src/core/mount.c | 8 ++++---- src/core/path.c | 2 +- src/core/scope.c | 4 ++-- src/core/service.c | 14 +++++++------- src/core/socket.c | 10 +++++----- src/core/swap.c | 8 ++++---- src/core/timer.c | 2 +- 9 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/core/automount.c b/src/core/automount.c index 20a73c76f9..00295cf769 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -301,7 +301,7 @@ static void automount_dump(Unit *u, FILE *f, const char *prefix) { static void automount_enter_dead(Automount *a, AutomountResult f) { assert(a); - if (f != AUTOMOUNT_SUCCESS) + if (a->result == AUTOMOUNT_SUCCESS) a->result = f; automount_set_state(a, a->result != AUTOMOUNT_SUCCESS ? AUTOMOUNT_FAILED : AUTOMOUNT_DEAD); diff --git a/src/core/busname.c b/src/core/busname.c index 730be2ee14..7952cd31aa 100644 --- a/src/core/busname.c +++ b/src/core/busname.c @@ -442,7 +442,7 @@ fail: static void busname_enter_dead(BusName *n, BusNameResult f) { assert(n); - if (f != BUSNAME_SUCCESS) + if (n->result == BUSNAME_SUCCESS) n->result = f; busname_set_state(n, n->result != BUSNAME_SUCCESS ? BUSNAME_FAILED : BUSNAME_DEAD); @@ -454,7 +454,7 @@ static void busname_enter_signal(BusName *n, BusNameState state, BusNameResult f assert(n); - if (f != BUSNAME_SUCCESS) + if (n->result == BUSNAME_SUCCESS) n->result = f; kill_context_init(&kill_context); @@ -882,7 +882,7 @@ static void busname_sigchld_event(Unit *u, pid_t pid, int code, int status) { log_unit_full(u, f == BUSNAME_SUCCESS ? LOG_DEBUG : LOG_NOTICE, 0, "Control process exited, code=%s status=%i", sigchld_code_to_string(code), status); - if (f != BUSNAME_SUCCESS) + if (n->result == BUSNAME_SUCCESS) n->result = f; switch (n->state) { diff --git a/src/core/mount.c b/src/core/mount.c index 3f6ac7fcf9..f3ccf6d48a 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -759,7 +759,7 @@ static int mount_spawn(Mount *m, ExecCommand *c, pid_t *_pid) { static void mount_enter_dead(Mount *m, MountResult f) { assert(m); - if (f != MOUNT_SUCCESS) + if (m->result == MOUNT_SUCCESS) m->result = f; mount_set_state(m, m->result != MOUNT_SUCCESS ? MOUNT_FAILED : MOUNT_DEAD); @@ -775,7 +775,7 @@ static void mount_enter_dead(Mount *m, MountResult f) { static void mount_enter_mounted(Mount *m, MountResult f) { assert(m); - if (f != MOUNT_SUCCESS) + if (m->result == MOUNT_SUCCESS) m->result = f; mount_set_state(m, MOUNT_MOUNTED); @@ -786,7 +786,7 @@ static void mount_enter_signal(Mount *m, MountState state, MountResult f) { assert(m); - if (f != MOUNT_SUCCESS) + if (m->result == MOUNT_SUCCESS) m->result = f; r = unit_kill_context( @@ -1158,7 +1158,7 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) { else assert_not_reached("Unknown code"); - if (f != MOUNT_SUCCESS) + if (m->result == MOUNT_SUCCESS) m->result = f; if (m->control_command) { diff --git a/src/core/path.c b/src/core/path.c index 0dd0d375d8..10f9b06974 100644 --- a/src/core/path.c +++ b/src/core/path.c @@ -454,7 +454,7 @@ static int path_coldplug(Unit *u) { static void path_enter_dead(Path *p, PathResult f) { assert(p); - if (f != PATH_SUCCESS) + if (p->result == PATH_SUCCESS) p->result = f; path_set_state(p, p->result != PATH_SUCCESS ? PATH_FAILED : PATH_DEAD); diff --git a/src/core/scope.c b/src/core/scope.c index b45e238974..b278aed3d6 100644 --- a/src/core/scope.c +++ b/src/core/scope.c @@ -221,7 +221,7 @@ static void scope_dump(Unit *u, FILE *f, const char *prefix) { static void scope_enter_dead(Scope *s, ScopeResult f) { assert(s); - if (f != SCOPE_SUCCESS) + if (s->result == SCOPE_SUCCESS) s->result = f; scope_set_state(s, s->result != SCOPE_SUCCESS ? SCOPE_FAILED : SCOPE_DEAD); @@ -233,7 +233,7 @@ static void scope_enter_signal(Scope *s, ScopeState state, ScopeResult f) { assert(s); - if (f != SCOPE_SUCCESS) + if (s->result == SCOPE_SUCCESS) s->result = f; unit_watch_all_pids(UNIT(s)); diff --git a/src/core/service.c b/src/core/service.c index 0cbea52276..a6793b813b 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1423,7 +1423,7 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart) int r; assert(s); - if (f != SERVICE_SUCCESS) + if (s->result == SERVICE_SUCCESS) s->result = f; service_set_state(s, s->result != SERVICE_SUCCESS ? SERVICE_FAILED : SERVICE_DEAD); @@ -1472,7 +1472,7 @@ static void service_enter_stop_post(Service *s, ServiceResult f) { int r; assert(s); - if (f != SERVICE_SUCCESS) + if (s->result == SERVICE_SUCCESS) s->result = f; service_unwatch_control_pid(s); @@ -1525,7 +1525,7 @@ static void service_enter_signal(Service *s, ServiceState state, ServiceResult f assert(s); - if (f != SERVICE_SUCCESS) + if (s->result == SERVICE_SUCCESS) s->result = f; unit_watch_all_pids(UNIT(s)); @@ -1583,7 +1583,7 @@ static void service_enter_stop(Service *s, ServiceResult f) { assert(s); - if (f != SERVICE_SUCCESS) + if (s->result == SERVICE_SUCCESS) s->result = f; service_unwatch_control_pid(s); @@ -1635,7 +1635,7 @@ static bool service_good(Service *s) { static void service_enter_running(Service *s, ServiceResult f) { assert(s); - if (f != SERVICE_SUCCESS) + if (s->result == SERVICE_SUCCESS) s->result = f; service_unwatch_control_pid(s); @@ -2609,7 +2609,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { "EXIT_STATUS=%i", status, NULL); - if (f != SERVICE_SUCCESS) + if (s->result == SERVICE_SUCCESS) s->result = f; if (s->main_command && @@ -2690,7 +2690,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { "Control process exited, code=%s status=%i", sigchld_code_to_string(code), status); - if (f != SERVICE_SUCCESS) + if (s->result == SERVICE_SUCCESS) s->result = f; /* Immediately get rid of the cgroup, so that the diff --git a/src/core/socket.c b/src/core/socket.c index 82363e2157..c919e89b7d 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1810,7 +1810,7 @@ fail: static void socket_enter_dead(Socket *s, SocketResult f) { assert(s); - if (f != SOCKET_SUCCESS) + if (s->result == SOCKET_SUCCESS) s->result = f; socket_set_state(s, s->result != SOCKET_SUCCESS ? SOCKET_FAILED : SOCKET_DEAD); @@ -1829,7 +1829,7 @@ static void socket_enter_stop_post(Socket *s, SocketResult f) { int r; assert(s); - if (f != SOCKET_SUCCESS) + if (s->result == SOCKET_SUCCESS) s->result = f; socket_unwatch_control_pid(s); @@ -1857,7 +1857,7 @@ static void socket_enter_signal(Socket *s, SocketState state, SocketResult f) { assert(s); - if (f != SOCKET_SUCCESS) + if (s->result == SOCKET_SUCCESS) s->result = f; r = unit_kill_context( @@ -1901,7 +1901,7 @@ static void socket_enter_stop_pre(Socket *s, SocketResult f) { int r; assert(s); - if (f != SOCKET_SUCCESS) + if (s->result == SOCKET_SUCCESS) s->result = f; socket_unwatch_control_pid(s); @@ -2822,7 +2822,7 @@ static void socket_sigchld_event(Unit *u, pid_t pid, int code, int status) { "Control process exited, code=%s status=%i", sigchld_code_to_string(code), status); - if (f != SOCKET_SUCCESS) + if (s->result == SOCKET_SUCCESS) s->result = f; if (s->control_command && diff --git a/src/core/swap.c b/src/core/swap.c index 0ba4c4d881..2c802da3b5 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -673,7 +673,7 @@ fail: static void swap_enter_dead(Swap *s, SwapResult f) { assert(s); - if (f != SWAP_SUCCESS) + if (s->result == SWAP_SUCCESS) s->result = f; swap_set_state(s, s->result != SWAP_SUCCESS ? SWAP_FAILED : SWAP_DEAD); @@ -689,7 +689,7 @@ static void swap_enter_dead(Swap *s, SwapResult f) { static void swap_enter_active(Swap *s, SwapResult f) { assert(s); - if (f != SWAP_SUCCESS) + if (s->result == SWAP_SUCCESS) s->result = f; swap_set_state(s, SWAP_ACTIVE); @@ -700,7 +700,7 @@ static void swap_enter_signal(Swap *s, SwapState state, SwapResult f) { assert(s); - if (f != SWAP_SUCCESS) + if (s->result == SWAP_SUCCESS) s->result = f; r = unit_kill_context( @@ -997,7 +997,7 @@ static void swap_sigchld_event(Unit *u, pid_t pid, int code, int status) { else assert_not_reached("Unknown code"); - if (f != SWAP_SUCCESS) + if (s->result == SWAP_SUCCESS) s->result = f; if (s->control_command) { diff --git a/src/core/timer.c b/src/core/timer.c index 3206296f09..e2b43f02f8 100644 --- a/src/core/timer.c +++ b/src/core/timer.c @@ -291,7 +291,7 @@ static int timer_coldplug(Unit *u) { static void timer_enter_dead(Timer *t, TimerResult f) { assert(t); - if (f != TIMER_SUCCESS) + if (t->result == TIMER_SUCCESS) t->result = f; timer_set_state(t, t->result != TIMER_SUCCESS ? TIMER_FAILED : TIMER_DEAD); -- cgit v1.2.3-54-g00ecf From b08af3b12706f352f651e70e117f6d6dcf11a911 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 4 Aug 2016 22:11:29 +0200 Subject: core: only set the watchdog variables in ExecStart= lines --- src/core/execute.c | 2 +- src/core/execute.h | 1 + src/core/service.c | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index 9028139723..7aafb1b058 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -1425,7 +1425,7 @@ static int build_environment( our_env[n_env++] = x; } - if (p->watchdog_usec > 0) { + if ((p->flags & EXEC_SET_WATCHDOG) && p->watchdog_usec > 0) { if (asprintf(&x, "WATCHDOG_PID="PID_FMT, getpid()) < 0) return -ENOMEM; our_env[n_env++] = x; diff --git a/src/core/execute.h b/src/core/execute.h index 2b4238ed7e..2d05ca39aa 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -218,6 +218,7 @@ typedef enum ExecFlags { EXEC_PASS_FDS = 1U << 4, EXEC_IS_CONTROL = 1U << 5, EXEC_SETENV_RESULT = 1U << 6, + EXEC_SET_WATCHDOG = 1U << 7, } ExecFlags; struct ExecParameters { diff --git a/src/core/service.c b/src/core/service.c index a6793b813b..c21653f5fa 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1743,7 +1743,7 @@ static void service_enter_start(Service *s) { r = service_spawn(s, c, timeout, - EXEC_PASS_FDS|EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN, + EXEC_PASS_FDS|EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN|EXEC_SET_WATCHDOG, &pid); if (r < 0) goto fail; @@ -1955,7 +1955,7 @@ static void service_run_next_main(Service *s) { r = service_spawn(s, s->main_command, s->timeout_start_usec, - EXEC_PASS_FDS|EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN, + EXEC_PASS_FDS|EXEC_APPLY_PERMISSIONS|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN|EXEC_SET_WATCHDOG, &pid); if (r < 0) goto fail; -- cgit v1.2.3-54-g00ecf