From: Lennart Poettering Date: Fri, 28 Feb 2025 22:30:55 +0000 (+0100) Subject: tty-askpw-agent: react to SIGTERM while waiting for console X-Git-Tag: v258-rc1~1202^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=789f4f7ee09025f1814f049f40444ccf70c1fed7;p=thirdparty%2Fsystemd.git 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 6bcbef34136..d8b56d05955 100644 --- 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 11a8e282953..208e168317d 100644 --- 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 787e698738a..5303910b4c5 100644 --- 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 9decf42f221..943513db560 100644 --- 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 b3471e272f5..a9240551f09 100644 --- 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);