diff options
author | Lennart Poettering <lennart@poettering.net> | 2016-11-18 11:08:06 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-11-18 11:08:06 +0100 |
commit | 2e6dbc0fcd45c152f15aed77cde4fd07957c150c (patch) | |
tree | f1110db4a4acf0dde5dc7f2eed58ad5eef47406b /src | |
parent | 9a4550e2589bddb12cea93693b83211d805444e1 (diff) | |
parent | 539622bd8c0b425626cab8562c85a5b0e1dda502 (diff) |
Merge pull request #4538 from fbuihuu/confirm-spawn-fixes
Confirm spawn fixes/enhancements
Diffstat (limited to 'src')
-rw-r--r-- | src/analyze/analyze.c | 2 | ||||
-rw-r--r-- | src/basic/terminal-util.c | 19 | ||||
-rw-r--r-- | src/core/execute.c | 157 | ||||
-rw-r--r-- | src/core/execute.h | 17 | ||||
-rw-r--r-- | src/core/main.c | 45 | ||||
-rw-r--r-- | src/core/manager.c | 62 | ||||
-rw-r--r-- | src/core/manager.h | 6 | ||||
-rw-r--r-- | src/core/mount.c | 2 | ||||
-rw-r--r-- | src/core/service.c | 2 | ||||
-rw-r--r-- | src/core/socket.c | 2 | ||||
-rw-r--r-- | src/core/swap.c | 2 | ||||
-rw-r--r-- | src/core/unit.c | 11 | ||||
-rw-r--r-- | src/core/unit.h | 2 | ||||
-rw-r--r-- | src/shared/nsflags.c | 1 |
14 files changed, 258 insertions, 72 deletions
diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index f744a84501..51d881c5fb 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -36,7 +36,9 @@ #include "log.h" #include "pager.h" #include "parse-util.h" +#ifdef HAVE_SECCOMP #include "seccomp-util.h" +#endif #include "special.h" #include "strv.h" #include "strxcpyx.h" diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index eafdea9eb3..9a8ef825c5 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -144,12 +144,14 @@ int read_one_char(FILE *f, char *ret, usec_t t, bool *need_nl) { return 0; } -int ask_char(char *ret, const char *replies, const char *text, ...) { +#define DEFAULT_ASK_REFRESH_USEC (2*USEC_PER_SEC) + +int ask_char(char *ret, const char *replies, const char *fmt, ...) { int r; assert(ret); assert(replies); - assert(text); + assert(fmt); for (;;) { va_list ap; @@ -159,8 +161,10 @@ int ask_char(char *ret, const char *replies, const char *text, ...) { if (colors_enabled()) fputs(ANSI_HIGHLIGHT, stdout); - va_start(ap, text); - vprintf(text, ap); + putchar('\r'); + + va_start(ap, fmt); + vprintf(fmt, ap); va_end(ap); if (colors_enabled()) @@ -168,9 +172,12 @@ int ask_char(char *ret, const char *replies, const char *text, ...) { fflush(stdout); - r = read_one_char(stdin, &c, USEC_INFINITY, &need_nl); + r = read_one_char(stdin, &c, DEFAULT_ASK_REFRESH_USEC, &need_nl); if (r < 0) { + if (r == -ETIMEDOUT) + continue; + if (r == -EBADMSG) { puts("Bad input, please try again."); continue; @@ -455,7 +462,7 @@ int acquire_terminal( goto fail; } - r = fd_wait_for_event(fd, POLLIN, ts + timeout - n); + r = fd_wait_for_event(notify, POLLIN, ts + timeout - n); if (r < 0) goto fail; diff --git a/src/core/execute.c b/src/core/execute.c index 04c4e511f4..07ab067c05 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -624,7 +624,7 @@ static int chown_terminal(int fd, uid_t uid) { return 0; } -static int setup_confirm_stdio(int *_saved_stdin, int *_saved_stdout) { +static int setup_confirm_stdio(const char *vc, int *_saved_stdin, int *_saved_stdout) { _cleanup_close_ int fd = -1, saved_stdin = -1, saved_stdout = -1; int r; @@ -639,12 +639,7 @@ static int setup_confirm_stdio(int *_saved_stdin, int *_saved_stdout) { if (saved_stdout < 0) return -errno; - fd = acquire_terminal( - "/dev/console", - false, - false, - false, - DEFAULT_CONFIRM_USEC); + fd = acquire_terminal(vc, false, false, false, DEFAULT_CONFIRM_USEC); if (fd < 0) return fd; @@ -674,21 +669,27 @@ static int setup_confirm_stdio(int *_saved_stdin, int *_saved_stdout) { return 0; } -_printf_(1, 2) static int write_confirm_message(const char *format, ...) { +static void write_confirm_error_fd(int err, int fd, const Unit *u) { + assert(err < 0); + + if (err == -ETIMEDOUT) + dprintf(fd, "Confirmation question timed out for %s, assuming positive response.\n", u->id); + else { + errno = -err; + dprintf(fd, "Couldn't ask confirmation for %s: %m, assuming positive response.\n", u->id); + } +} + +static void write_confirm_error(int err, const char *vc, const Unit *u) { _cleanup_close_ int fd = -1; - va_list ap; - assert(format); + assert(vc); - fd = open_terminal("/dev/console", O_WRONLY|O_NOCTTY|O_CLOEXEC); + fd = open_terminal(vc, O_WRONLY|O_NOCTTY|O_CLOEXEC); if (fd < 0) - return fd; - - va_start(ap, format); - vdprintf(fd, format, ap); - va_end(ap); + return; - return 0; + write_confirm_error_fd(err, fd, u); } static int restore_confirm_stdio(int *saved_stdin, int *saved_stdout) { @@ -713,22 +714,96 @@ static int restore_confirm_stdio(int *saved_stdin, int *saved_stdout) { return r; } -static int ask_for_confirmation(char *response, char **argv) { +enum { + CONFIRM_PRETEND_FAILURE = -1, + CONFIRM_PRETEND_SUCCESS = 0, + CONFIRM_EXECUTE = 1, +}; + +static int ask_for_confirmation(const char *vc, Unit *u, const char *cmdline) { int saved_stdout = -1, saved_stdin = -1, r; - _cleanup_free_ char *line = NULL; + _cleanup_free_ char *e = NULL; + char c; - r = setup_confirm_stdio(&saved_stdin, &saved_stdout); - if (r < 0) - return r; + /* For any internal errors, assume a positive response. */ + r = setup_confirm_stdio(vc, &saved_stdin, &saved_stdout); + if (r < 0) { + write_confirm_error(r, vc, u); + return CONFIRM_EXECUTE; + } - line = exec_command_line(argv); - if (!line) - return -ENOMEM; + /* confirm_spawn might have been disabled while we were sleeping. */ + if (manager_is_confirm_spawn_disabled(u->manager)) { + r = 1; + goto restore_stdio; + } - r = ask_char(response, "yns", "Execute %s? [Yes, No, Skip] ", line); + e = ellipsize(cmdline, 60, 100); + if (!e) { + log_oom(); + r = CONFIRM_EXECUTE; + goto restore_stdio; + } - restore_confirm_stdio(&saved_stdin, &saved_stdout); + for (;;) { + r = ask_char(&c, "yfshiDjcn", "Execute %s? [y, f, s – h for help] ", e); + if (r < 0) { + write_confirm_error_fd(r, STDOUT_FILENO, u); + r = CONFIRM_EXECUTE; + goto restore_stdio; + } + + switch (c) { + case 'c': + printf("Resuming normal execution.\n"); + manager_disable_confirm_spawn(); + r = 1; + break; + case 'D': + unit_dump(u, stdout, " "); + continue; /* ask again */ + case 'f': + printf("Failing execution.\n"); + r = CONFIRM_PRETEND_FAILURE; + break; + case 'h': + printf(" c - continue, proceed without asking anymore\n" + " D - dump, show the state of the unit\n" + " f - fail, don't execute the command and pretend it failed\n" + " h - help\n" + " i - info, show a short summary of the unit\n" + " j - jobs, show jobs that are in progress\n" + " s - skip, don't execute the command and pretend it succeeded\n" + " y - yes, execute the command\n"); + continue; /* ask again */ + case 'i': + printf(" Description: %s\n" + " Unit: %s\n" + " Command: %s\n", + u->id, u->description, cmdline); + continue; /* ask again */ + case 'j': + manager_dump_jobs(u->manager, stdout, " "); + continue; /* ask again */ + case 'n': + /* 'n' was removed in favor of 'f'. */ + printf("Didn't understand 'n', did you mean 'f'?\n"); + continue; /* ask again */ + case 's': + printf("Skipping execution.\n"); + r = CONFIRM_PRETEND_SUCCESS; + break; + case 'y': + r = CONFIRM_EXECUTE; + break; + default: + assert_not_reached("Unhandled choice"); + } + break; + } +restore_stdio: + restore_confirm_stdio(&saved_stdin, &saved_stdout); return r; } @@ -2315,22 +2390,24 @@ static int exec_child( exec_context_tty_reset(context, params); - if (params->flags & EXEC_CONFIRM_SPAWN) { - char response; + if (unit_shall_confirm_spawn(unit)) { + const char *vc = params->confirm_spawn; + _cleanup_free_ char *cmdline = NULL; - r = ask_for_confirmation(&response, argv); - if (r == -ETIMEDOUT) - write_confirm_message("Confirmation question timed out, assuming positive response.\n"); - else if (r < 0) - write_confirm_message("Couldn't ask confirmation question, assuming positive response: %s\n", strerror(-r)); - else if (response == 's') { - write_confirm_message("Skipping execution.\n"); + cmdline = exec_command_line(argv); + if (!cmdline) { + *exit_status = EXIT_CONFIRM; + return -ENOMEM; + } + + r = ask_for_confirmation(vc, unit, cmdline); + if (r != CONFIRM_EXECUTE) { + if (r == CONFIRM_PRETEND_SUCCESS) { + *exit_status = EXIT_SUCCESS; + return 0; + } *exit_status = EXIT_CONFIRM; return -ECANCELED; - } else if (response == 'n') { - write_confirm_message("Failing execution.\n"); - *exit_status = 0; - return 0; } } diff --git a/src/core/execute.h b/src/core/execute.h index e52640ee91..951c8f4da3 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -225,16 +225,15 @@ static inline bool exec_context_restrict_namespaces_set(const ExecContext *c) { } typedef enum ExecFlags { - EXEC_CONFIRM_SPAWN = 1U << 0, - EXEC_APPLY_PERMISSIONS = 1U << 1, - EXEC_APPLY_CHROOT = 1U << 2, - EXEC_APPLY_TTY_STDIN = 1U << 3, + EXEC_APPLY_PERMISSIONS = 1U << 0, + EXEC_APPLY_CHROOT = 1U << 1, + EXEC_APPLY_TTY_STDIN = 1U << 2, /* 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, - EXEC_SET_WATCHDOG = 1U << 7, + EXEC_PASS_FDS = 1U << 3, + EXEC_IS_CONTROL = 1U << 4, + EXEC_SETENV_RESULT = 1U << 5, + EXEC_SET_WATCHDOG = 1U << 6, } ExecFlags; struct ExecParameters { @@ -254,6 +253,8 @@ struct ExecParameters { const char *runtime_prefix; + const char *confirm_spawn; + usec_t watchdog_usec; int *idle_pipe; diff --git a/src/core/main.c b/src/core/main.c index f5f7df838d..5f9b1acad3 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -68,6 +68,7 @@ #include "mount-setup.h" #include "pager.h" #include "parse-util.h" +#include "path-util.h" #include "proc-cmdline.h" #include "process-util.h" #include "raw-clone.h" @@ -104,7 +105,7 @@ static bool arg_dump_core = true; static int arg_crash_chvt = -1; static bool arg_crash_shell = false; static bool arg_crash_reboot = false; -static bool arg_confirm_spawn = false; +static char *arg_confirm_spawn = NULL; static ShowStatus arg_show_status = _SHOW_STATUS_UNSET; static bool arg_switched_root = false; static bool arg_no_pager = false; @@ -294,6 +295,28 @@ static int parse_crash_chvt(const char *value) { return 0; } +static int parse_confirm_spawn(const char *value, char **console) { + char *s; + int r; + + r = value ? parse_boolean(value) : 1; + if (r == 0) { + *console = NULL; + return 0; + } + + if (r > 0) /* on with default tty */ + s = strdup("/dev/console"); + else if (is_path(value)) /* on with fully qualified path */ + s = strdup(value); + else /* on with only a tty file name, not a fully qualified path */ + s = strjoin("/dev/", value); + if (!s) + return -ENOMEM; + *console = s; + return 0; +} + static int set_machine_id(const char *m) { sd_id128_t t; assert(m); @@ -355,11 +378,11 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat } else if (streq(key, "systemd.confirm_spawn") && value) { - r = parse_boolean(value); + arg_confirm_spawn = mfree(arg_confirm_spawn); + + r = parse_confirm_spawn(value, &arg_confirm_spawn); if (r < 0) - log_warning("Failed to parse confirm spawn switch %s. Ignoring.", value); - else - arg_confirm_spawn = r; + log_warning_errno(r, "Failed to parse confirm_spawn switch %s. Ignoring.", value); } else if (streq(key, "systemd.show_status") && value) { @@ -952,12 +975,11 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_CONFIRM_SPAWN: - r = optarg ? parse_boolean(optarg) : 1; - if (r < 0) { - log_error("Failed to parse confirm spawn boolean %s.", optarg); - return r; - } - arg_confirm_spawn = r; + arg_confirm_spawn = mfree(arg_confirm_spawn); + + r = parse_confirm_spawn(optarg, &arg_confirm_spawn); + if (r < 0) + return log_error_errno(r, "Failed to parse confirm spawn option: %m"); break; case ARG_SHOW_STATUS: @@ -1991,6 +2013,7 @@ finish: arg_default_rlimit[j] = mfree(arg_default_rlimit[j]); arg_default_unit = mfree(arg_default_unit); + arg_confirm_spawn = mfree(arg_confirm_spawn); arg_join_controllers = strv_free_free(arg_join_controllers); arg_default_environment = strv_free(arg_default_environment); arg_syscall_archs = set_free(arg_syscall_archs); diff --git a/src/core/manager.c b/src/core/manager.c index 31770eef3a..1f663d3c1d 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -111,6 +111,12 @@ static void manager_watch_jobs_in_progress(Manager *m) { assert(m); + /* We do not want to show the cylon animation if the user + * needs to confirm service executions otherwise confirmation + * messages will be screwed by the cylon animation. */ + if (!manager_is_confirm_spawn_disabled(m)) + return; + if (m->jobs_in_progress_event_source) return; @@ -2993,7 +2999,7 @@ void manager_check_finished(Manager *m) { manager_close_idle_pipe(m); /* Turn off confirm spawn now */ - m->confirm_spawn = false; + m->confirm_spawn = NULL; /* No need to update ask password status when we're going non-interactive */ manager_close_ask_password(m); @@ -3178,6 +3184,49 @@ static bool manager_get_show_status(Manager *m, StatusType type) { return false; } +const char *manager_get_confirm_spawn(Manager *m) { + static int last_errno = 0; + const char *vc = m->confirm_spawn; + struct stat st; + int r; + + /* Here's the deal: we want to test the validity of the console but don't want + * PID1 to go through the whole console process which might block. But we also + * want to warn the user only once if something is wrong with the console so we + * cannot do the sanity checks after spawning our children. So here we simply do + * really basic tests to hopefully trap common errors. + * + * If the console suddenly disappear at the time our children will really it + * then they will simply fail to acquire it and a positive answer will be + * assumed. New children will fallback to /dev/console though. + * + * Note: TTYs are devices that can come and go any time, and frequently aren't + * available yet during early boot (consider a USB rs232 dongle...). If for any + * reason the configured console is not ready, we fallback to the default + * console. */ + + if (!vc || path_equal(vc, "/dev/console")) + return vc; + + r = stat(vc, &st); + if (r < 0) + goto fail; + + if (!S_ISCHR(st.st_mode)) { + errno = ENOTTY; + goto fail; + } + + last_errno = 0; + return vc; +fail: + if (last_errno != errno) { + last_errno = errno; + log_warning_errno(errno, "Failed to open %s: %m, using default console", vc); + } + return "/dev/console"; +} + void manager_set_first_boot(Manager *m, bool b) { assert(m); @@ -3194,6 +3243,17 @@ void manager_set_first_boot(Manager *m, bool b) { m->first_boot = b; } +void manager_disable_confirm_spawn(void) { + (void) touch("/run/systemd/confirm_spawn_disabled"); +} + +bool manager_is_confirm_spawn_disabled(Manager *m) { + if (!m->confirm_spawn) + return true; + + return access("/run/systemd/confirm_spawn_disabled", F_OK) >= 0; +} + void manager_status_printf(Manager *m, StatusType type, const char *status, const char *format, ...) { va_list ap; diff --git a/src/core/manager.h b/src/core/manager.h index d54ca54107..4a9a37caff 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -246,7 +246,7 @@ struct Manager { uint8_t return_value; ShowStatus show_status; - bool confirm_spawn; + char *confirm_spawn; bool no_console_output; ExecOutput default_std_output, default_std_error; @@ -403,3 +403,7 @@ void manager_deserialize_gid_refs_one(Manager *m, const char *value); const char *manager_state_to_string(ManagerState m) _const_; ManagerState manager_state_from_string(const char *s) _pure_; + +const char *manager_get_confirm_spawn(Manager *m); +bool manager_is_confirm_spawn_disabled(Manager *m); +void manager_disable_confirm_spawn(void); diff --git a/src/core/mount.c b/src/core/mount.c index 43e0f1c746..1c2be28d55 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -747,7 +747,7 @@ static int mount_spawn(Mount *m, ExecCommand *c, pid_t *_pid) { return r; exec_params.environment = UNIT(m)->manager->environment; - exec_params.flags |= UNIT(m)->manager->confirm_spawn ? EXEC_CONFIRM_SPAWN : 0; + exec_params.confirm_spawn = manager_get_confirm_spawn(UNIT(m)->manager); 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 7aa1fba572..9ad4cf5070 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1335,7 +1335,7 @@ static int service_spawn( exec_params.fds = fds; exec_params.fd_names = fd_names; exec_params.n_fds = n_fds; - exec_params.flags |= UNIT(s)->manager->confirm_spawn ? EXEC_CONFIRM_SPAWN : 0; + exec_params.confirm_spawn = manager_get_confirm_spawn(UNIT(s)->manager); exec_params.cgroup_supported = UNIT(s)->manager->cgroup_supported; exec_params.cgroup_path = path; exec_params.cgroup_delegate = s->cgroup_context.delegate; diff --git a/src/core/socket.c b/src/core/socket.c index ebacd74a47..1a53d47f21 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1778,7 +1778,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.flags |= UNIT(s)->manager->confirm_spawn ? EXEC_CONFIRM_SPAWN : 0; + exec_params.confirm_spawn = manager_get_confirm_spawn(UNIT(s)->manager); 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 b870ac88e3..bf404db8c3 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -636,7 +636,7 @@ static int swap_spawn(Swap *s, ExecCommand *c, pid_t *_pid) { goto fail; exec_params.environment = UNIT(s)->manager->environment; - exec_params.flags |= UNIT(s)->manager->confirm_spawn ? EXEC_CONFIRM_SPAWN : 0; + exec_params.confirm_spawn = manager_get_confirm_spawn(UNIT(s)->manager); 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/unit.c b/src/core/unit.c index fbb21e4985..cba6342eca 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1511,6 +1511,17 @@ int unit_start_limit_test(Unit *u) { return emergency_action(u->manager, u->start_limit_action, u->reboot_arg, "unit failed"); } +bool unit_shall_confirm_spawn(Unit *u) { + + if (manager_is_confirm_spawn_disabled(u->manager)) + return false; + + /* For some reasons units remaining in the same process group + * as PID 1 fail to acquire the console even if it's not used + * by any process. So skip the confirmation question for them. */ + return !unit_get_exec_context(u)->same_pgrp; +} + /* Errors: * -EBADR: This unit type does not support starting. * -EALREADY: Unit is already started. diff --git a/src/core/unit.h b/src/core/unit.h index 6d6885b487..8052c234fd 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -657,6 +657,8 @@ void unit_notify_user_lookup(Unit *u, uid_t uid, gid_t gid); int unit_set_invocation_id(Unit *u, sd_id128_t id); int unit_acquire_invocation_id(Unit *u); +bool unit_shall_confirm_spawn(Unit *u); + /* Macros which append UNIT= or USER_UNIT= to the message */ #define log_unit_full(unit, level, error, ...) \ diff --git a/src/shared/nsflags.c b/src/shared/nsflags.c index 911779a28f..aeb79b131e 100644 --- a/src/shared/nsflags.c +++ b/src/shared/nsflags.c @@ -22,7 +22,6 @@ #include "alloc-util.h" #include "extract-word.h" #include "nsflags.h" -#include "seccomp-util.h" #include "string-util.h" const struct namespace_flag_map namespace_flag_map[] = { |