pid1,vconsole-setup: lock /dev/console instead of the tty device

As requested in https://github.com/systemd/systemd/pull/27867#pullrequestreview-1567161854.
/dev/console, /dev/tty0, and /dev/ttyN are "different" device nodes
that may point to a single underlying device. We want to use a single
lock so that we don't get a race if different writers are using a different
device path, so let's just always lock around /dev/console.
This effectively makes the locking less granular.

Fixup for a0043bfa51281c2374878e2a98cf2a3ee10fd92c.
Fixes https://github.com/systemd/systemd/issues/28721.
Maybe fixes https://github.com/systemd/systemd/issues/28778 and
https://github.com/systemd/systemd/issues/28634.

diff --git a/src/core/execute.c b/src/core/execute.c
index 46fb8805b3aa856d3a33aca0f15352841f9d88c4..0e7e91370adf7e9ed4ef2c6dc00f32963674167b 100644 (file)
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -25,6 +25,7 @@
 #include "cgroup-setup.h"
 #include "constants.h"
 #include "cpu-set-util.h"
+#include "dev-setup.h"
 #include "env-file.h"
 #include "env-util.h"
 #include "errno-list.h"
@@ -106,29 +107,28 @@ int exec_context_tty_size(const ExecContext *context, unsigned *ret_rows, unsign
 }
 
 void exec_context_tty_reset(const ExecContext *context, const ExecParameters *p) {
-        _cleanup_close_ int fd = -EBADF;
+        _cleanup_close_ int _fd = -EBADF, lock_fd = -EBADF;
         const char *path = exec_context_tty_path(ASSERT_PTR(context));
+        int fd;
 
-        /* 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.
-         */
-
-        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 (p && p->stdin_fd >= 0)
+                fd = p->stdin_fd;
+        else if (path) {
+                fd = _fd = open_terminal(path, O_RDWR|O_NOCTTY|O_CLOEXEC|O_NONBLOCK);
                 if (fd < 0)
                         return;
-
-                if (lock_generic(fd, LOCK_BSD, LOCK_EX) < 0)
-                        return;
         } else
                 return;   /* nothing to do */
 
+        /* Take a synchronization lock 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.
+         */
+        lock_fd = lock_dev_console();
+        if (lock_fd < 0)
+                return (void) log_debug_errno(lock_fd,
+                                              "Failed to lock /dev/console: %m");
+
         if (context->tty_vhangup)
                 (void) terminal_vhangup_fd(fd);
 

diff --git a/src/shared/dev-setup.c b/src/shared/dev-setup.c
index 7dca6ad7d4e6eb49a86384bd603b898771e20492..f7ed16193f183881ebfc72f064d597f70bf555d4 100644 (file)
--- a/src/shared/dev-setup.c
+++ b/src/shared/dev-setup.c
@@ -6,14 +6,32 @@
 
 #include "alloc-util.h"
 #include "dev-setup.h"
+#include "fd-util.h"
 #include "label-util.h"
+#include "lock-util.h"
 #include "log.h"
 #include "mkdir-label.h"
 #include "nulstr-util.h"
 #include "path-util.h"
+#include "terminal-util.h"
 #include "umask-util.h"
 #include "user-util.h"
 
+int lock_dev_console(void) {
+        _cleanup_close_ int fd = -EBADF;
+        int r;
+
+        fd = open_terminal("/dev/console", O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW);
+        if (fd < 0)
+                return fd;
+
+        r = lock_generic(fd, LOCK_BSD, LOCK_EX);
+        if (r < 0)
+                return log_error_errno(r, "Failed to lock /dev/console: %m");
+
+        return TAKE_FD(fd);
+}
+
 int dev_setup(const char *prefix, uid_t uid, gid_t gid) {
         static const char symlinks[] =
                 "-/proc/kcore\0"     "/dev/core\0"

diff --git a/src/shared/dev-setup.h b/src/shared/dev-setup.h
index 92ba6cf76403346dc55a8da9c50a158f4fb6791f..5339bc4e5e9075605283076ffc338740a3ab1a6b 100644 (file)
--- a/src/shared/dev-setup.h
+++ b/src/shared/dev-setup.h
@@ -3,6 +3,8 @@
 
 #include <sys/types.h>
 
+int lock_dev_console(void);
+
 int dev_setup(const char *prefix, uid_t uid, gid_t gid);
 
 int make_inaccessible_nodes(const char *parent_dir, uid_t uid, gid_t gid);

diff --git a/src/vconsole/vconsole-setup.c b/src/vconsole/vconsole-setup.c
index 2901881218377efc6e7e3dfdc4157346f7bf33b5..564f14228458382b69628b979e009b60d7f6ecbf 100644 (file)
--- a/src/vconsole/vconsole-setup.c
+++ b/src/vconsole/vconsole-setup.c
@@ -21,13 +21,13 @@
 
 #include "alloc-util.h"
 #include "creds-util.h"
+#include "dev-setup.h"
 #include "env-file.h"
 #include "errno-util.h"
 #include "fd-util.h"
 #include "fileio.h"
 #include "io-util.h"
 #include "locale-util.h"
-#include "lock-util.h"
 #include "log.h"
 #include "main-func.h"
 #include "proc-cmdline.h"
@@ -574,7 +574,7 @@ static int verify_source_vc(char **ret_path, const char *src_vc) {
 static int run(int argc, char **argv) {
         _cleanup_(context_done) Context c = {};
         _cleanup_free_ char *vc = NULL;
-        _cleanup_close_ int fd = -EBADF;
+        _cleanup_close_ int fd = -EBADF, lock_fd = -EBADF;
         bool utf8, keyboard_ok;
         unsigned idx = 0;
         int r;
@@ -596,9 +596,15 @@ static int run(int argc, char **argv) {
 
         /* 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)
-                return log_error_errno(r, "Failed to lock console: %m");
+        lock_fd = lock_dev_console();
+        if (lock_fd < 0) {
+                log_full_errno(lock_fd == -ENOENT ? LOG_DEBUG : LOG_ERR,
+                               lock_fd,
+                               "Failed to lock /dev/console%s: %m",
+                               lock_fd == -ENOENT ? ", ignoring" : "");
+                if (lock_fd != -ENOENT)
+                        return lock_fd;
+        }
 
         (void) toggle_utf8_sysfs(utf8);
         (void) toggle_utf8_vc(vc, fd, utf8);