tty-askpw-agent: react to SIGTERM while waiting for console

I noticed that systemd-tty-password-agent would time out when asked to
stop via SIGTERM, and eventually be killed, under some circumstances.
It took me a while but i figured out what was going on:

systemd-ask-pw-agent blocks SIGTERM because it wants async notifications
on SIGTERM via signalfd() to listen on. That mostly works great: except
for one case: if we actually get a pw query request, and hence need to
acquire the terminal: we issue open_terminal() in that case, but if the
terminal is used otherwsie we'll hang, and because SIGTERM is blocked
we'll hang and cannot exit cleanly.

Address that: optionally, in acquire_terminal() look for SIGTERM by
unblcking the signal mask via ppoll() while we wait.

diff --git a/src/basic/io-util.c b/src/basic/io-util.c
index 6bcbef3413623dd624ff0f98c44fe952cffbecf5..d8b56d05955b25b5026420616b9dfd0dbda3b968 100644 (file)
--- a/src/basic/io-util.c
+++ b/src/basic/io-util.c
@@ -188,7 +188,7 @@ int pipe_eof(int fd) {
         return !!(r & POLLHUP);
 }
 
-int ppoll_usec(struct pollfd *fds, size_t nfds, usec_t timeout) {
+int ppoll_usec_full(struct pollfd *fds, size_t nfds, usec_t timeout, const sigset_t *ss) {
         int r;
 
         assert(fds || nfds == 0);
@@ -211,7 +211,7 @@ int ppoll_usec(struct pollfd *fds, size_t nfds, usec_t timeout) {
         if (nfds == 0)
                 return 0;
 
-        r = ppoll(fds, nfds, timeout == USEC_INFINITY ? NULL : TIMESPEC_STORE(timeout), NULL);
+        r = ppoll(fds, nfds, timeout == USEC_INFINITY ? NULL : TIMESPEC_STORE(timeout), ss);
         if (r < 0)
                 return -errno;
         if (r == 0)

diff --git a/src/basic/io-util.h b/src/basic/io-util.h
index 11a8e282953d35a1459781e1a01ec3111f342dda..208e168317d6492bf6a91f3f51b368d00c425311 100644 (file)
--- a/src/basic/io-util.h
+++ b/src/basic/io-util.h
@@ -22,7 +22,11 @@ static inline int loop_write(int fd, const void *buf, size_t nbytes) {
 
 int pipe_eof(int fd);
 
-int ppoll_usec(struct pollfd *fds, size_t nfds, usec_t timeout) _nonnull_if_nonzero_(1, 2);
+int ppoll_usec_full(struct pollfd *fds, size_t nfds, usec_t timeout, const sigset_t *ss) _nonnull_if_nonzero_(1, 2);
+_nonnull_if_nonzero_(1, 2) static inline int ppoll_usec(struct pollfd *fds, size_t nfds, usec_t timeout) {
+        return ppoll_usec_full(fds, nfds, timeout, NULL);
+}
+
 int fd_wait_for_event(int fd, int event, usec_t timeout);
 
 ssize_t sparse_write(int fd, const void *p, size_t sz, size_t run_length);

diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c
index 787e698738a252210e38ee3595203d3f79ad0600..5303910b4c599c0a78415054f9df5a2f5375e338 100644 (file)
--- a/src/basic/terminal-util.c
+++ b/src/basic/terminal-util.c
@@ -643,7 +643,10 @@ int acquire_terminal(
         int r, wd = -1;
 
         assert(name);
-        assert(IN_SET(flags & ~ACQUIRE_TERMINAL_PERMISSIVE, ACQUIRE_TERMINAL_TRY, ACQUIRE_TERMINAL_FORCE, ACQUIRE_TERMINAL_WAIT));
+
+        AcquireTerminalFlags mode = flags & _ACQUIRE_TERMINAL_MODE_MASK;
+        assert(IN_SET(mode, ACQUIRE_TERMINAL_TRY, ACQUIRE_TERMINAL_FORCE, ACQUIRE_TERMINAL_WAIT));
+        assert(mode == ACQUIRE_TERMINAL_WAIT || !FLAGS_SET(flags, ACQUIRE_TERMINAL_WATCH_SIGTERM));
 
         /* We use inotify to be notified when the tty is closed. We create the watch before checking if we can actually
          * acquire it, so that we don't lose any event.
@@ -654,8 +657,8 @@ int acquire_terminal(
          * not configure any service on the same tty as an untrusted user this should not be a problem. (Which they
          * probably should not do anyway.) */
 
-        if ((flags & ~ACQUIRE_TERMINAL_PERMISSIVE) == ACQUIRE_TERMINAL_WAIT) {
-                notify = inotify_init1(IN_CLOEXEC | (timeout != USEC_INFINITY ? IN_NONBLOCK : 0));
+        if (mode == ACQUIRE_TERMINAL_WAIT) {
+                notify = inotify_init1(IN_CLOEXEC | IN_NONBLOCK);
                 if (notify < 0)
                         return -errno;
 
@@ -667,6 +670,14 @@ int acquire_terminal(
                         ts = now(CLOCK_MONOTONIC);
         }
 
+        /* If we are called with ACQUIRE_TERMINAL_WATCH_SIGTERM we'll unblock SIGTERM during ppoll() temporarily */
+        sigset_t poll_ss;
+        assert_se(sigprocmask(SIG_SETMASK, /* newset= */ NULL, &poll_ss) >= 0);
+        if (flags & ACQUIRE_TERMINAL_WATCH_SIGTERM) {
+                assert_se(sigismember(&poll_ss, SIGTERM) > 0);
+                assert_se(sigdelset(&poll_ss, SIGTERM) >= 0);
+        }
+
         for (;;) {
                 if (notify >= 0) {
                         r = flush_fd(notify);
@@ -685,7 +696,7 @@ int acquire_terminal(
                 assert_se(sigaction(SIGHUP, &sigaction_ignore, &sa_old) >= 0);
 
                 /* First, try to get the tty */
-                r = RET_NERRNO(ioctl(fd, TIOCSCTTY, (flags & ~ACQUIRE_TERMINAL_PERMISSIVE) == ACQUIRE_TERMINAL_FORCE));
+                r = RET_NERRNO(ioctl(fd, TIOCSCTTY, mode == ACQUIRE_TERMINAL_FORCE));
 
                 /* Reset signal handler to old value */
                 assert_se(sigaction(SIGHUP, &sa_old, NULL) >= 0);
@@ -703,32 +714,41 @@ int acquire_terminal(
                                                           * already are the owner of the TTY. */
                         break;
 
-                if (flags != ACQUIRE_TERMINAL_WAIT) /* If we are in TRY or FORCE mode, then propagate EPERM as EPERM */
+                if (mode != ACQUIRE_TERMINAL_WAIT) /* If we are in TRY or FORCE mode, then propagate EPERM as EPERM */
                         return r;
 
                 assert(notify >= 0);
                 assert(wd >= 0);
 
                 for (;;) {
-                        union inotify_event_buffer buffer;
-                        ssize_t l;
-
-                        if (timeout != USEC_INFINITY) {
-                                usec_t n;
-
+                        usec_t left;
+                        if (timeout == USEC_INFINITY)
+                                left = USEC_INFINITY;
+                        else {
                                 assert(ts != USEC_INFINITY);
 
-                                n = usec_sub_unsigned(now(CLOCK_MONOTONIC), ts);
+                                usec_t n = usec_sub_unsigned(now(CLOCK_MONOTONIC), ts);
                                 if (n >= timeout)
                                         return -ETIMEDOUT;
 
-                                r = fd_wait_for_event(notify, POLLIN, usec_sub_unsigned(timeout, n));
-                                if (r < 0)
-                                        return r;
-                                if (r == 0)
-                                        return -ETIMEDOUT;
+                                left = timeout - n;
                         }
 
+                        r = ppoll_usec_full(
+                                        &(struct pollfd) {
+                                                .fd = notify,
+                                                .events = POLLIN,
+                                        },
+                                        /* n_pollfds = */ 1,
+                                        left,
+                                        &poll_ss);
+                        if (r < 0)
+                                return r;
+                        if (r == 0)
+                                return -ETIMEDOUT;
+
+                        union inotify_event_buffer buffer;
+                        ssize_t l;
                         l = read(notify, &buffer, sizeof(buffer));
                         if (l < 0) {
                                 if (ERRNO_IS_TRANSIENT(errno))

diff --git a/src/basic/terminal-util.h b/src/basic/terminal-util.h
index 9decf42f2214fd6c84ea8d56fd088a401f784cc8..943513db560fead2c60515feac9b6fa6beed4adb 100644 (file)
--- a/src/basic/terminal-util.h
+++ b/src/basic/terminal-util.h
@@ -65,8 +65,14 @@ typedef enum AcquireTerminalFlags {
         /* If we can't become the controlling process of the TTY right-away, then wait until we can. */
         ACQUIRE_TERMINAL_WAIT       = 2,
 
+        /* The combined mask of the above */
+        _ACQUIRE_TERMINAL_MODE_MASK = ACQUIRE_TERMINAL_TRY | ACQUIRE_TERMINAL_FORCE | ACQUIRE_TERMINAL_WAIT,
+
         /* Pick one of the above, and then OR this flag in, in order to request permissive behaviour, if we can't become controlling process then don't mind */
         ACQUIRE_TERMINAL_PERMISSIVE = 1 << 2,
+
+        /* Check for pending SIGTERM while waiting for inotify (SIGTERM must be blocked by caller) */
+        ACQUIRE_TERMINAL_WATCH_SIGTERM = 1 << 3,
 } AcquireTerminalFlags;
 
 int acquire_terminal(const char *name, AcquireTerminalFlags flags, usec_t timeout);

diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c
index b3471e272f54afcb57dc84409b140b570c2c59ce..a9240551f09d9bc98d6cc1f813461aad473391fa 100644 (file)
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -141,7 +141,7 @@ static int agent_ask_password_tty(
         const char *con = arg_device ?: "/dev/console";
 
         if (arg_console) {
-                tty_fd = acquire_terminal(con, ACQUIRE_TERMINAL_WAIT, USEC_INFINITY);
+                tty_fd = acquire_terminal(con, ACQUIRE_TERMINAL_WAIT|ACQUIRE_TERMINAL_WATCH_SIGTERM, USEC_INFINITY);
                 if (tty_fd < 0)
                         return log_error_errno(tty_fd, "Failed to acquire %s: %m", con);