nspawn: don't become TTY controller just to undo it later again

Instead of first becoming a controlling process of the payload pty
as side effect of opening it (without O_NOCTTY), and then possibly
dropping it again, let's do it cleanly an reverse the logic: let's open
the pty without becoming its controller first. Only after everything
went the way we wanted it to go become the controller explicitly.

This has the benefit that the PID 1 stub process we run (as effect of
--as-pid2) doesn't have to lose the tty explicitly, but can just
continue running with things. And we explicitly make the tty controlling
right before invoking actual payload.

In order to make sure everything works as expected validate that the
stub PID 1 in the container really has no conrolling tty by issuing the
TIOCNOTTY tty and expecting ENOTTY, and log about it.

This shouldn't change behaviour much, it just makes thins a bit cleaner,
in particular as we'll not trigger SIGHUP on ourselves (since we are
controller and session leader) due to TIOCNOTTY which we then have to
explicitly ignore.

diff --git a/src/nspawn/nspawn-stub-pid1.c b/src/nspawn/nspawn-stub-pid1.c
index f785a3b2483f5821ba7a94789fac4bf46419b030..60d7439fb17b02bbb0c30008a17c4f04a2cde5cd 100644 (file)
--- a/src/nspawn/nspawn-stub-pid1.c
+++ b/src/nspawn/nspawn-stub-pid1.c
@@ -53,12 +53,6 @@ int stub_pid1(sd_id128_t uuid) {
         assert_se(sigfillset(&fullmask) >= 0);
         assert_se(sigprocmask(SIG_BLOCK, &fullmask, &oldmask) >= 0);
 
-        /* Surrender the terminal this stub may control so that child processes can have a controlling terminal
-         * without resorting to setsid hacks. */
-        r = ioctl(STDIN_FILENO, TIOCNOTTY);
-        if (r < 0 && errno != ENOTTY)
-                return log_error_errno(errno, "Failed to surrender controlling terminal: %m");
-
         pid = fork();
         if (pid < 0)
                 return log_error_errno(errno, "Failed to fork child pid: %m");
@@ -79,6 +73,12 @@ int stub_pid1(sd_id128_t uuid) {
         (void) close_all_fds(NULL, 0);
         log_open();
 
+        if (ioctl(STDIN_FILENO, TIOCNOTTY) < 0) {
+                if (errno != ENOTTY)
+                        log_warning_errno(errno, "Unexpected error from TIOCNOTTY ioctl in init stub process, ignoring: %m");
+        } else
+                log_warning("Expected TIOCNOTTY to fail, but it succeeded in init stub process, ignoring.");
+
         /* Flush out /proc/self/environ, so that we don't leak the environment from the host into the container. Also,
          * set $container= and $container_uuid= so that clients in the container that query it from /proc/1/environ
          * find them set. */

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 1b7578581de564b94daeea91e05ec2871c6eb561..282f12d9c141ba019949cb36660a32da6a47360b 100644 (file)
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -11,10 +11,12 @@
 #endif
 #include <stdlib.h>
 #include <sys/file.h>
+#include <sys/ioctl.h>
 #include <sys/personality.h>
 #include <sys/prctl.h>
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <termios.h>
 #include <unistd.h>
 
 #include "sd-bus.h"
@@ -2278,7 +2280,9 @@ static int setup_stdio_as_dev_console(void) {
         _cleanup_close_ int terminal = -1;
         int r;
 
-        terminal = open_terminal("/dev/console", O_RDWR);
+        /* We open the TTY in O_NOCTTY mode, so that we do not become controller yet. We'll do that later
+         * explicitly, if we are configured to. */
+        terminal = open_terminal("/dev/console", O_RDWR|O_NOCTTY);
         if (terminal < 0)
                 return log_error_errno(terminal, "Failed to open console: %m");
 
@@ -3373,8 +3377,7 @@ static int inner_child(
          * wait until the parent is ready with the
          * setup, too... */
         if (!barrier_place_and_sync(barrier)) /* #5 */
-                return log_error_errno(SYNTHETIC_ERRNO(ESRCH),
-                                       "Parent died too early");
+                return log_error_errno(SYNTHETIC_ERRNO(ESRCH), "Parent died too early");
 
         if (arg_chdir)
                 if (chdir(arg_chdir) < 0)
@@ -3386,6 +3389,13 @@ static int inner_child(
                         return r;
         }
 
+        if (arg_console_mode != CONSOLE_PIPE) {
+                /* So far our pty wasn't controlled by any process. Finally, it's time to change that, if we
+                 * are configured for that. Acquire it as controlling tty. */
+                if (ioctl(STDIN_FILENO, TIOCSCTTY) < 0)
+                        return log_error_errno(errno, "Failed to acquire controlling TTY: %m");
+        }
+
         log_debug("Inner child completed, invoking payload.");
 
         /* Now, explicitly close the log, so that we then can close all remaining fds. Closing the log explicitly first