Merge pull request #1490 from poettering/machinectl-shell-race-fix

machinectl shell race fix

7 files changed, 53 insertions, 34 deletions

diff --git a/TODO b/TODO
index 066d0ae6b6..d399ae9477 100644
--- a/TODO
+++ b/TODO
@@ -26,9 +26,9 @@ External:
 
 Features:
 
-* add a concept of RemainAfterExit= to scope units
+* when creating transient services, support passing in a tty fd to use for stdin/stdout
 
-* add journal vacuum by max number of files
+* add a concept of RemainAfterExit= to scope units
 
 * add a new command "systemctl revert" or so, that removes all dropin
   snippets in /run and /etc, and all unit files with counterparts in
@@ -159,15 +159,9 @@ Features:
   that are not supported...
   http://lists.freedesktop.org/archives/systemd-devel/2015-February/028076.html
 
-* Introduce $LISTEN_NAMES to complement $LISTEN_FDS, containing a
-  colon separated list of identifiers for the fds passed.
-
 * maybe introduce WantsMountsFor=? Usecase:
   http://lists.freedesktop.org/archives/systemd-devel/2015-January/027729.html
 
-* rework kexec logic to use new kexec_file_load() syscall, so that we
-  don't have to call kexec tool anymore.
-
 * The udev blkid built-in should expose a property that reflects
   whether media was sensed in USB CF/SD card readers. This should then
   be used to control SYSTEMD_READY=1/0 so that USB card readers aren't
diff --git a/src/basic/util.c b/src/basic/util.c
index 2855993afe..f845e2dabb 100644
--- a/src/basic/util.c
+++ b/src/basic/util.c
@@ -6143,7 +6143,7 @@ int openpt_in_namespace(pid_t pid, int flags) {
                 if (r < 0)
                         _exit(EXIT_FAILURE);
 
-                master = posix_openpt(flags);
+                master = posix_openpt(flags|O_NOCTTY|O_CLOEXEC);
                 if (master < 0)
                         _exit(EXIT_FAILURE);
 
diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c
index 17a186eb0e..e75b183328 100644
--- a/src/machine/machinectl.c
+++ b/src/machine/machinectl.c
@@ -1173,7 +1173,7 @@ static int on_machine_removed(sd_bus_message *m, void *userdata, sd_bus_error *r
         return 0;
 }
 
-static int process_forward(sd_event *event, PTYForward **forward, int master, bool ignore_vhangup, const char *name) {
+static int process_forward(sd_event *event, PTYForward **forward, int master, PTYForwardFlags flags, const char *name) {
         char last_char = 0;
         bool machine_died;
         int ret = 0, r;
@@ -1192,7 +1192,7 @@ static int process_forward(sd_event *event, PTYForward **forward, int master, bo
         sd_event_add_signal(event, NULL, SIGINT, NULL, NULL);
         sd_event_add_signal(event, NULL, SIGTERM, NULL, NULL);
 
-        r = pty_forward_new(event, master, ignore_vhangup, false, forward);
+        r = pty_forward_new(event, master, flags, forward);
         if (r < 0)
                 return log_error_errno(r, "Failed to create PTY forwarder: %m");
 
@@ -1203,7 +1203,7 @@ static int process_forward(sd_event *event, PTYForward **forward, int master, bo
         pty_forward_get_last_char(*forward, &last_char);
 
         machine_died =
-                ignore_vhangup &&
+                (flags & PTY_FORWARD_IGNORE_VHANGUP) &&
                 pty_forward_get_ignore_vhangup(*forward) == 0;
 
         *forward = pty_forward_free(*forward);
@@ -1286,7 +1286,7 @@ static int login_machine(int argc, char *argv[], void *userdata) {
         if (r < 0)
                 return bus_log_parse_error(r);
 
-        return process_forward(event, &forward, master, true, machine);
+        return process_forward(event, &forward, master, PTY_FORWARD_IGNORE_VHANGUP, machine);
 }
 
 static int shell_machine(int argc, char *argv[], void *userdata) {
@@ -1390,7 +1390,7 @@ static int shell_machine(int argc, char *argv[], void *userdata) {
         if (r < 0)
                 return bus_log_parse_error(r);
 
-        return process_forward(event, &forward, master, false, machine);
+        return process_forward(event, &forward, master, PTY_FORWARD_IGNORE_INITIAL_VHANGUP, machine);
 }
 
 static int remove_image(int argc, char *argv[], void *userdata) {
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index f4a2e3d9ba..ab93f98df4 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -3512,7 +3512,7 @@ int main(int argc, char *argv[]) {
 
                 rtnl_socket_pair[0] = safe_close(rtnl_socket_pair[0]);
 
-                r = pty_forward_new(event, master, true, !interactive, &forward);
+                r = pty_forward_new(event, master, PTY_FORWARD_IGNORE_VHANGUP | (interactive ? 0 : PTY_FORWARD_READ_ONLY), &forward);
                 if (r < 0) {
                         log_error_errno(r, "Failed to create PTY forwarder: %m");
                         goto finish;
diff --git a/src/run/run.c b/src/run/run.c
index 62687591f4..bb18dd4383 100644
--- a/src/run/run.c
+++ b/src/run/run.c
@@ -828,7 +828,7 @@ static int start_transient_service(
                 if (!arg_quiet)
                         log_info("Running as unit %s.\nPress ^] three times within 1s to disconnect TTY.", service);
 
-                r = pty_forward_new(event, master, false, false, &forward);
+                r = pty_forward_new(event, master, PTY_FORWARD_IGNORE_INITIAL_VHANGUP, &forward);
                 if (r < 0)
                         return log_error_errno(r, "Failed to create PTY forwarder: %m");
 
diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c
index 789f217efc..7749f20540 100644
--- a/src/shared/ptyfwd.c
+++ b/src/shared/ptyfwd.c
@@ -32,6 +32,8 @@ struct PTYForward {
 
         int master;
 
+        PTYForwardFlags flags;
+
         sd_event_source *stdin_event_source;
         sd_event_source *stdout_event_source;
         sd_event_source *master_event_source;
@@ -41,8 +43,6 @@ struct PTYForward {
         struct termios saved_stdin_attr;
         struct termios saved_stdout_attr;
 
-        bool read_only:1;
-
         bool saved_stdin:1;
         bool saved_stdout:1;
 
@@ -54,8 +54,7 @@ struct PTYForward {
         bool master_writable:1;
         bool master_hangup:1;
 
-        /* Continue reading after hangup? */
-        bool ignore_vhangup:1;
+        bool read_from_master:1;
 
         bool last_char_set:1;
         char last_char;
@@ -100,6 +99,18 @@ static bool look_for_escape(PTYForward *f, const char *buffer, size_t n) {
         return false;
 }
 
+static bool ignore_vhangup(PTYForward *f) {
+        assert(f);
+
+        if (f->flags & PTY_FORWARD_IGNORE_VHANGUP)
+                return true;
+
+        if ((f->flags & PTY_FORWARD_IGNORE_INITIAL_VHANGUP) && !f->read_from_master)
+                return true;
+
+        return false;
+}
+
 static int shovel(PTYForward *f) {
         ssize_t k;
 
@@ -179,7 +190,7 @@ static int shovel(PTYForward *f) {
                                  * EAGAIN here and try again, unless
                                  * ignore_vhangup is off. */
 
-                                if (errno == EAGAIN || (errno == EIO && f->ignore_vhangup))
+                                if (errno == EAGAIN || (errno == EIO && ignore_vhangup(f)))
                                         f->master_readable = false;
                                 else if (errno == EPIPE || errno == ECONNRESET || errno == EIO) {
                                         f->master_readable = f->master_writable = false;
@@ -190,8 +201,10 @@ static int shovel(PTYForward *f) {
                                         log_error_errno(errno, "read(): %m");
                                         return sd_event_exit(f->event, EXIT_FAILURE);
                                 }
-                        }  else
+                        }  else {
+                                f->read_from_master = true;
                                 f->out_buffer_full += (size_t) k;
+                        }
                 }
 
                 if (f->stdout_writable && f->out_buffer_full > 0) {
@@ -302,8 +315,7 @@ static int on_sigwinch_event(sd_event_source *e, const struct signalfd_siginfo *
 int pty_forward_new(
                 sd_event *event,
                 int master,
-                bool ignore_vhangup,
-                bool read_only,
+                PTYForwardFlags flags,
                 PTYForward **ret) {
 
         _cleanup_(pty_forward_freep) PTYForward *f = NULL;
@@ -314,8 +326,7 @@ int pty_forward_new(
         if (!f)
                 return -ENOMEM;
 
-        f->read_only = read_only;
-        f->ignore_vhangup = ignore_vhangup;
+        f->flags = flags;
 
         if (event)
                 f->event = sd_event_ref(event);
@@ -325,7 +336,7 @@ int pty_forward_new(
                         return r;
         }
 
-        if (!read_only) {
+        if (!(flags & PTY_FORWARD_READ_ONLY)) {
                 r = fd_nonblock(STDIN_FILENO, true);
                 if (r < 0)
                         return r;
@@ -344,7 +355,7 @@ int pty_forward_new(
         if (ioctl(STDOUT_FILENO, TIOCGWINSZ, &ws) >= 0)
                 (void) ioctl(master, TIOCSWINSZ, &ws);
 
-        if (!read_only) {
+        if (!(flags & PTY_FORWARD_READ_ONLY)) {
                 if (tcgetattr(STDIN_FILENO, &f->saved_stdin_attr) >= 0) {
                         struct termios raw_stdin_attr;
 
@@ -429,16 +440,20 @@ int pty_forward_get_last_char(PTYForward *f, char *ch) {
         return 0;
 }
 
-int pty_forward_set_ignore_vhangup(PTYForward *f, bool ignore_vhangup) {
+int pty_forward_set_ignore_vhangup(PTYForward *f, bool b) {
         int r;
 
         assert(f);
 
-        if (f->ignore_vhangup == ignore_vhangup)
+        if (!!(f->flags & PTY_FORWARD_IGNORE_VHANGUP) == b)
                 return 0;
 
-        f->ignore_vhangup = ignore_vhangup;
-        if (!f->ignore_vhangup) {
+        if (b)
+                f->flags |= PTY_FORWARD_IGNORE_VHANGUP;
+        else
+                f->flags &= ~PTY_FORWARD_IGNORE_VHANGUP;
+
+        if (!ignore_vhangup(f)) {
 
                 /* We shall now react to vhangup()s? Let's check
                  * immediately if we might be in one */
@@ -455,5 +470,5 @@ int pty_forward_set_ignore_vhangup(PTYForward *f, bool ignore_vhangup) {
 int pty_forward_get_ignore_vhangup(PTYForward *f) {
         assert(f);
 
-        return f->ignore_vhangup;
+        return !!(f->flags & PTY_FORWARD_IGNORE_VHANGUP);
 }
diff --git a/src/shared/ptyfwd.h b/src/shared/ptyfwd.h
index 6f84e4036a..9b3214221b 100644
--- a/src/shared/ptyfwd.h
+++ b/src/shared/ptyfwd.h
@@ -27,7 +27,17 @@
 
 typedef struct PTYForward PTYForward;
 
-int pty_forward_new(sd_event *event, int master, bool ignore_vhangup, bool read_only, PTYForward **f);
+typedef enum PTYForwardFlags {
+        PTY_FORWARD_READ_ONLY = 1,
+
+        /* Continue reading after hangup? */
+        PTY_FORWARD_IGNORE_VHANGUP = 2,
+
+        /* Continue reading after hangup but only if we never read anything else? */
+        PTY_FORWARD_IGNORE_INITIAL_VHANGUP = 4,
+} PTYForwardFlags;
+
+int pty_forward_new(sd_event *event, int master, PTYForwardFlags flags, PTYForward **f);
 PTYForward *pty_forward_free(PTYForward *f);
 
 int pty_forward_get_last_char(PTYForward *f, char *ch);