Merge pull request #2463 from poettering/machined-tty-fix

Fixes for the service TTY reset logic and other stuff

8 files changed, 100 insertions, 75 deletions

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();
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);
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) {
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 73f5710b9c..12209dc99b 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -2410,19 +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 *n = NULL, *state = NULL;
-        const char *path;
+        _cleanup_free_ char *buf = NULL;
+        const char *path, *state;
         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. */
+        /* 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,
@@ -2430,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", n);
+                        "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)