pid1,vconsole-setup: take a lock for the console device

When systemd-firstboot (or any other unit which uses the tty) is started,
systemd will reset the terminal. If systemd-vconsole-setup happens to be
running at that time, it'll error out when it tries to use the vconsole fd and
gets an EIO from ioctl.

e019ea738d63d5f7803f378f8bd3e074d66be08f was the first fix. It added an
implicit ordering between units using the tty and systemd-vconsole-setup.
(The commit title is wrong. The approach was generalized, but the commit title
wasn't updated.)
Then cea32691c313b2dab91cef986d08f309edeb4a40 was added to restart
systemd-vconsole-setup.service from systemd-firstboot.service. This was OK,
with the ordering in place, systemd-vconsole-setup.service would wait until
systemd-firstboot.service exited. But this wasn't enough, because we want the
key mappings to be loaded immediately after systemd-firstboot writes the
config. 8eb668b9ab2f7627a89c95ffd61350ee9d416da1 implemented that, but actually
reintroduced the original issue. I had to drop the ordering between the two
units because otherwise we'd deadlock, waiting from firstboot for
vconsole-setup which wouldn't start while firstboot was running.
Restarting vconsole-setup.service from systemd-firstboot.service works just
fine, but when vconsole-setup.service is started earlier, it may be interrupted
by systemd-firstboot.service.

To resolve the issue, let's take a lock around the tty device. The reset is
performed after fork, so the (short) delay should not matter too much.

In xopenat_lock() the assert on <path> is dropped so that we can call
xopenat(fd, NULL) to get a copy of the original fd.

Fixes #26908.

diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c
index 94c0dfd3de5b23f31613830a31e6d97bc6befce2..804440ef2a5b1f90a5d2da021f0b839759ec60e6 100644 (file)
--- a/src/basic/fs-util.c
+++ b/src/basic/fs-util.c
@@ -1172,7 +1172,6 @@ int xopenat_lock(
         int r;
 
         assert(dir_fd >= 0 || dir_fd == AT_FDCWD);
-        assert(path);
         assert(IN_SET(operation & ~LOCK_NB, LOCK_EX, LOCK_SH));
 
         /* POSIX/UNPOSIX locks don't work on directories (errno is set to -EBADF so let's return early with

diff --git a/src/core/execute.c b/src/core/execute.c
index 3a0d289fa551c58c4eb7b9fcc7b53a46be7a03eb..246c151ebce10c24f28108dce555f9d66a449bb8 100644 (file)
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -268,25 +268,34 @@ static int exec_context_tty_size(const ExecContext *context, unsigned *ret_rows,
 }
 
 static void exec_context_tty_reset(const ExecContext *context, const ExecParameters *p) {
-        const char *path;
+        _cleanup_close_ int fd = -EBADF;
+        const char *path = exec_context_tty_path(ASSERT_PTR(context));
 
-        assert(context);
+        /* Take a lock around the device for the duration of the setup that we do here.
+         * systemd-vconsole-setup.service also takes the lock to avoid being interrupted.
+         * We open a new fd that will be closed automatically, and operate on it for convenience.
+         */
 
-        path = exec_context_tty_path(context);
+        if (p && p->stdin_fd >= 0) {
+                fd = xopenat_lock(p->stdin_fd, NULL,
+                                  O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY, 0, 0, LOCK_BSD, LOCK_EX);
+                if (fd < 0)
+                        return;
+        } else if (path) {
+                fd = open_terminal(path, O_RDWR|O_NOCTTY|O_CLOEXEC|O_NONBLOCK);
+                if (fd < 0)
+                        return;
 
-        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 (lock_generic(fd, LOCK_BSD, LOCK_EX) < 0)
+                        return;
+        } else
+                return;   /* nothing to do */
 
-        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_vhangup)
+                (void) terminal_vhangup_fd(fd);
+
+        if (context->tty_reset)
+                (void) reset_terminal_fd(fd, true);
 
         if (p && p->stdin_fd >= 0) {
                 unsigned rows = context->tty_rows, cols = context->tty_cols;

diff --git a/src/vconsole/vconsole-setup.c b/src/vconsole/vconsole-setup.c
index 4d9915cbbaaab8b4578113a670400665e77407fa..f2987412b992042e486ad7429b732092e34da9dd 100644 (file)
--- a/src/vconsole/vconsole-setup.c
+++ b/src/vconsole/vconsole-setup.c
@@ -26,6 +26,7 @@
 #include "fileio.h"
 #include "io-util.h"
 #include "locale-util.h"
+#include "lock-util.h"
 #include "log.h"
 #include "proc-cmdline.h"
 #include "process-util.h"
@@ -589,6 +590,14 @@ int main(int argc, char **argv) {
 
         context_load_config(&c);
 
+        /* Take lock around the remaining operation to avoid being interrupted by a tty reset operation
+         * performed for services with TTYVHangup=yes. */
+        r = lock_generic(fd, LOCK_BSD, LOCK_EX);
+        if (r < 0) {
+                log_error_errno(r, "Failed to lock console: %m");
+                return EXIT_FAILURE;
+        }
+
         (void) toggle_utf8_sysfs(utf8);
         (void) toggle_utf8_vc(vc, fd, utf8);