From 1e22b5cda04b6d5e0dd83ab8e6ecb452cf34851f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Jan 2016 16:25:39 +0100 Subject: core: don't reset /dev/console if stdin/stdout/stderr as passed as fd in a transient service Otherwise we might end resetting /dev/console all the time when a transient service starts or stops. Fixes #2377 Fixes #2198 Fixes #2061 --- src/basic/terminal-util.c | 4 +-- src/core/dbus-service.c | 2 ++ src/core/execute.c | 64 +++++++++++++++++++++++++++++++---------------- src/core/execute.h | 2 ++ src/core/service.c | 3 +++ 5 files changed, 50 insertions(+), 25 deletions(-) (limited to 'src') diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index fedfc8a5df..0a9d2bbdef 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -726,9 +726,7 @@ bool tty_is_vc_resolve(const char *tty) { } const char *default_term_for_tty(const char *tty) { - assert(tty); - - return tty_is_vc_resolve(tty) ? "TERM=linux" : "TERM=vt220"; + return tty && tty_is_vc_resolve(tty) ? "TERM=linux" : "TERM=vt220"; } int fd_columns(int fd) { diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c index 24f611a593..2529689f5a 100644 --- a/src/core/dbus-service.c +++ b/src/core/dbus-service.c @@ -153,6 +153,8 @@ static int bus_service_set_transient_property( asynchronous_close(s->stderr_fd); s->stderr_fd = copy; } + + s->exec_context.stdio_as_fds = true; } return 1; diff --git a/src/core/execute.c b/src/core/execute.c index b9de2617a9..80db62131c 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -183,26 +183,41 @@ static int flags_fds(const int fds[], unsigned n_fds, bool nonblock) { return 0; } -_pure_ static const char *tty_path(const ExecContext *context) { +static const char *exec_context_tty_path(const ExecContext *context) { assert(context); + if (context->stdio_as_fds) + return NULL; + if (context->tty_path) return context->tty_path; return "/dev/console"; } -static void exec_context_tty_reset(const ExecContext *context) { +static void exec_context_tty_reset(const ExecContext *context, const ExecParameters *p) { + const char *path; + assert(context); - if (context->tty_vhangup) - terminal_vhangup(tty_path(context)); + path = exec_context_tty_path(context); + + if (context->tty_vhangup) { + if (p && p->stdin_fd >= 0) + (void) terminal_vhangup_fd(p->stdin_fd); + else if (path) + (void) terminal_vhangup(path); + } - if (context->tty_reset) - reset_terminal(tty_path(context)); + if (context->tty_reset) { + if (p && p->stdin_fd >= 0) + (void) reset_terminal_fd(p->stdin_fd, true); + else if (path) + (void) reset_terminal(path); + } - if (context->tty_vt_disallocate && context->tty_path) - vt_disallocate(context->tty_path); + if (context->tty_vt_disallocate && path) + (void) vt_disallocate(path); } static bool is_terminal_output(ExecOutput o) { @@ -400,7 +415,7 @@ static int setup_input( case EXEC_INPUT_TTY_FAIL: { int fd, r; - fd = acquire_terminal(tty_path(context), + fd = acquire_terminal(exec_context_tty_path(context), i == EXEC_INPUT_TTY_FAIL, i == EXEC_INPUT_TTY_FORCE, false, @@ -485,7 +500,7 @@ static int setup_output( } else if (o == EXEC_OUTPUT_INHERIT) { /* If input got downgraded, inherit the original value */ if (i == EXEC_INPUT_NULL && is_terminal_input(context->std_input)) - return open_terminal_as(tty_path(context), O_WRONLY, fileno); + return open_terminal_as(exec_context_tty_path(context), O_WRONLY, fileno); /* If the input is connected to anything that's not a /dev/null, inherit that... */ if (i != EXEC_INPUT_NULL) @@ -509,7 +524,7 @@ static int setup_output( return dup2(STDIN_FILENO, fileno) < 0 ? -errno : fileno; /* We don't reset the terminal if this is just about output */ - return open_terminal_as(tty_path(context), O_WRONLY, fileno); + return open_terminal_as(exec_context_tty_path(context), O_WRONLY, fileno); case EXEC_OUTPUT_SYSLOG: case EXEC_OUTPUT_SYSLOG_AND_CONSOLE: @@ -1232,9 +1247,8 @@ static void do_idle_pipe_dance(int idle_pipe[4]) { static int build_environment( const ExecContext *c, + const ExecParameters *p, unsigned n_fds, - char ** fd_names, - usec_t watchdog_usec, const char *home, const char *username, const char *shell, @@ -1262,7 +1276,7 @@ static int build_environment( return -ENOMEM; our_env[n_env++] = x; - joined = strv_join(fd_names, ":"); + joined = strv_join(p->fd_names, ":"); if (!joined) return -ENOMEM; @@ -1272,12 +1286,12 @@ static int build_environment( our_env[n_env++] = x; } - if (watchdog_usec > 0) { + if (p->watchdog_usec > 0) { if (asprintf(&x, "WATCHDOG_PID="PID_FMT, getpid()) < 0) return -ENOMEM; our_env[n_env++] = x; - if (asprintf(&x, "WATCHDOG_USEC="USEC_FMT, watchdog_usec) < 0) + if (asprintf(&x, "WATCHDOG_USEC="USEC_FMT, p->watchdog_usec) < 0) return -ENOMEM; our_env[n_env++] = x; } @@ -1313,7 +1327,7 @@ static int build_environment( c->std_error == EXEC_OUTPUT_TTY || c->tty_path) { - x = strdup(default_term_for_tty(tty_path(c))); + x = strdup(default_term_for_tty(exec_context_tty_path(c))); if (!x) return -ENOMEM; our_env[n_env++] = x; @@ -1490,7 +1504,7 @@ static int exec_child( return -errno; } - exec_context_tty_reset(context); + exec_context_tty_reset(context, params); if (params->confirm_spawn) { char response; @@ -1991,7 +2005,7 @@ static int exec_child( #endif } - r = build_environment(context, n_fds, params->fd_names, params->watchdog_usec, home, username, shell, &our_env); + r = build_environment(context, params, n_fds, home, username, shell, &our_env); if (r < 0) { *exit_status = EXIT_MEMORY; return r; @@ -2366,6 +2380,9 @@ static bool tty_may_match_dev_console(const char *tty) { _cleanup_free_ char *active = NULL; char *console; + if (!tty) + return true; + if (startswith(tty, "/dev/")) tty += 5; @@ -2383,11 +2400,14 @@ static bool tty_may_match_dev_console(const char *tty) { } bool exec_context_may_touch_console(ExecContext *ec) { - return (ec->tty_reset || ec->tty_vhangup || ec->tty_vt_disallocate || + + return (ec->tty_reset || + ec->tty_vhangup || + ec->tty_vt_disallocate || is_terminal_input(ec->std_input) || is_terminal_output(ec->std_output) || is_terminal_output(ec->std_error)) && - tty_may_match_dev_console(tty_path(ec)); + tty_may_match_dev_console(exec_context_tty_path(ec)); } static void strv_fprintf(FILE *f, char **l) { @@ -2726,7 +2746,7 @@ void exec_status_exit(ExecStatus *s, ExecContext *context, pid_t pid, int code, if (context->utmp_id) utmp_put_dead_process(context->utmp_id, pid, code, status); - exec_context_tty_reset(context); + exec_context_tty_reset(context, NULL); } } diff --git a/src/core/execute.h b/src/core/execute.h index 8649620830..e4b93b603d 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -122,6 +122,8 @@ struct ExecContext { nsec_t timer_slack_nsec; + bool stdio_as_fds; + char *tty_path; bool tty_reset; diff --git a/src/core/service.c b/src/core/service.c index ae84cccbc8..355de3e15d 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -2363,6 +2363,7 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value, else { asynchronous_close(s->stdin_fd); s->stdin_fd = fdset_remove(fds, fd); + s->exec_context.stdio_as_fds = true; } } else if (streq(key, "stdout-fd")) { int fd; @@ -2372,6 +2373,7 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value, else { asynchronous_close(s->stdout_fd); s->stdout_fd = fdset_remove(fds, fd); + s->exec_context.stdio_as_fds = true; } } else if (streq(key, "stderr-fd")) { int fd; @@ -2381,6 +2383,7 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value, else { asynchronous_close(s->stderr_fd); s->stderr_fd = fdset_remove(fds, fd); + s->exec_context.stdio_as_fds = true; } } else log_unit_debug(u, "Unknown serialization key: %s", key); -- cgit v1.2.3-54-g00ecf From d813d7a37211680b3110e36b3bcaff7510a14378 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Jan 2016 16:28:01 +0100 Subject: shared: meanor clean-ups for logs-show.c Some minor simplifications. Shouldn't change codepaths. --- src/shared/logs-show.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) (limited to 'src') diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index a1f65d1a88..111f0225d9 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -82,12 +82,11 @@ static int print_catalog(FILE *f, sd_journal *j) { static int parse_field(const void *data, size_t length, const char *field, char **target, size_t *target_size) { size_t fl, nl; - void *buf; + char *buf; assert(data); assert(field); assert(target); - assert(target_size); fl = strlen(field); if (length < fl) @@ -97,16 +96,18 @@ static int parse_field(const void *data, size_t length, const char *field, char return 0; nl = length - fl; - buf = malloc(nl+1); + buf = new(char, nl+1); if (!buf) return log_oom(); memcpy(buf, (const char*) data + fl, nl); - ((char*)buf)[nl] = 0; + buf[nl] = 0; free(*target); *target = buf; - *target_size = nl; + + if (target_size) + *target_size = nl; return 1; } @@ -415,7 +416,7 @@ static int output_verbose( const void *data; size_t length; _cleanup_free_ char *cursor = NULL; - uint64_t realtime; + uint64_t realtime = 0; char ts[FORMAT_TIMESTAMP_MAX + 7]; int r; @@ -431,17 +432,15 @@ static int output_verbose( return log_full_errno(r == -EADDRNOTAVAIL ? LOG_DEBUG : LOG_ERR, r, "Failed to get source realtime timestamp: %m"); else { _cleanup_free_ char *value = NULL; - size_t size; - r = parse_field(data, length, "_SOURCE_REALTIME_TIMESTAMP=", &value, &size); + r = parse_field(data, length, "_SOURCE_REALTIME_TIMESTAMP=", &value, NULL); if (r < 0) return r; - else { - assert(r > 0); - r = safe_atou64(value, &realtime); - if (r < 0) - log_debug_errno(r, "Failed to parse realtime timestamp: %m"); - } + assert(r > 0); + + r = safe_atou64(value, &realtime); + if (r < 0) + log_debug_errno(r, "Failed to parse realtime timestamp: %m"); } if (r < 0) { -- cgit v1.2.3-54-g00ecf From 6233b9023b18148b2553b4d4fba246701b55916e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Jan 2016 16:33:31 +0100 Subject: systemctl: don't mangle unit names in check_unit_generic() As it turns out all callers of check_unit_generic() already mangle unit names, or get the unit names directly from PID 1 (and hence arein normalized form anyway), hence there's no point in mangling then... --- src/systemctl/systemctl.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 73f5710b9c..8a43d0174f 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2411,16 +2411,12 @@ static int unit_find_paths( static int check_one_unit(sd_bus *bus, const char *name, const char *good_states, bool quiet) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - _cleanup_free_ char *n = NULL, *state = NULL; + _cleanup_free_ char *state = NULL; const char *path; int r; assert(name); - r = unit_name_mangle(name, UNIT_NAME_NOGLOB, &n); - if (r < 0) - return log_error_errno(r, "Failed to mangle unit name: %m"); - /* We don't use unit_dbus_path_from_name() directly since we * don't want to load the unit if it isn't loaded. */ @@ -2432,7 +2428,7 @@ static int check_one_unit(sd_bus *bus, const char *name, const char *good_states "GetUnit", NULL, &reply, - "s", n); + "s", name); if (r < 0) { if (!quiet) puts("unknown"); -- cgit v1.2.3-54-g00ecf From b430f85f22ac23720b30263bd6cb252bb85ecc4b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Jan 2016 16:43:23 +0100 Subject: systemctl: don't make up unit states, and don't eat up errors to eagerly When checking a unit's state, don't ignore errors too eagerly, but generate proper error messages. Also, don't synthesize an "unknown" state on error, but let the operation file. If a unit file isn't loaded treat this as "inactive" as that's effectively what it means. --- src/systemctl/systemctl.c | 52 +++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 24 deletions(-) (limited to 'src') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 8a43d0174f..12209dc99b 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2410,15 +2410,16 @@ static int unit_find_paths( } static int check_one_unit(sd_bus *bus, const char *name, const char *good_states, bool quiet) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - _cleanup_free_ char *state = NULL; - const char *path; + _cleanup_free_ char *buf = NULL; + const char *path, *state; int r; assert(name); - /* We don't use unit_dbus_path_from_name() directly since we - * don't want to load the unit if it isn't loaded. */ + /* We don't use unit_dbus_path_from_name() directly since we don't want to load the unit unnecessarily, if it + * isn't loaded. */ r = sd_bus_call_method( bus, @@ -2426,31 +2427,34 @@ static int check_one_unit(sd_bus *bus, const char *name, const char *good_states "/org/freedesktop/systemd1", "org.freedesktop.systemd1.Manager", "GetUnit", - NULL, + &error, &reply, "s", name); if (r < 0) { - if (!quiet) - puts("unknown"); - return 0; - } + if (!sd_bus_error_has_name(&error, BUS_ERROR_NO_SUCH_UNIT)) + return log_error_errno(r, "Failed to retrieve unit: %s", bus_error_message(&error, r)); - r = sd_bus_message_read(reply, "o", &path); - if (r < 0) - return bus_log_parse_error(r); + /* The unit is currently not loaded, hence say it's "inactive", since all units that aren't loaded are + * considered inactive. */ + state = "inactive"; - r = sd_bus_get_property_string( - bus, - "org.freedesktop.systemd1", - path, - "org.freedesktop.systemd1.Unit", - "ActiveState", - NULL, - &state); - if (r < 0) { - if (!quiet) - puts("unknown"); - return 0; + } else { + r = sd_bus_message_read(reply, "o", &path); + if (r < 0) + return bus_log_parse_error(r); + + r = sd_bus_get_property_string( + bus, + "org.freedesktop.systemd1", + path, + "org.freedesktop.systemd1.Unit", + "ActiveState", + &error, + &buf); + if (r < 0) + return log_error_errno(r, "Failed to retrieve unit state: %s", bus_error_message(&error, r)); + + state = buf; } if (!quiet) -- cgit v1.2.3-54-g00ecf From ad6b1fa287c1d007fd85aa3e85b7e4a6bc7f515f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Jan 2016 17:00:38 +0100 Subject: basic: getauxval(AT_RANDOM) is apparently not necessarily aligned Let's make sure we read it in a way compatible with non-aligned memory. Fixes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=812928 --- src/basic/random-util.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/basic/random-util.c b/src/basic/random-util.c index e1543da5a3..2f468db770 100644 --- a/src/basic/random-util.c +++ b/src/basic/random-util.c @@ -95,17 +95,18 @@ void initialize_srand(void) { if (srand_called) return; - x = 0; - #ifdef HAVE_SYS_AUXV_H - /* The kernel provides us with a bit of entropy in auxv, so - * let's try to make use of that to seed the pseudo-random - * generator. It's better than nothing... */ + /* The kernel provides us with 16 bytes of entropy in auxv, so let's try to make use of that to seed the + * pseudo-random generator. It's better than nothing... */ auxv = (void*) getauxval(AT_RANDOM); - if (auxv) - x ^= *(unsigned*) auxv; + if (auxv) { + assert_cc(sizeof(x) < 16); + memcpy(&x, auxv, sizeof(x)); + } else #endif + x = 0; + x ^= (unsigned) now(CLOCK_REALTIME); x ^= (unsigned) gettid(); -- cgit v1.2.3-54-g00ecf