From 8854d7950449911dafc2aad945ad97584dd5b9f1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 13 Feb 2018 21:24:37 +0100 Subject: terminal-util: rework acquire_terminal() This modernizes acquire_terminal() in a couple of ways: 1. The three boolean arguments are replaced by a flags parameter, that should be more descriptive in what it does. 2. We now properly handle inotify queue overruns 3. We use _cleanup_ for closing the fds now, to shorten the code quite a bit. Behaviour should not be altered by this. --- src/basic/terminal-util.c | 134 +++++++++------------ src/basic/terminal-util.h | 18 ++- src/core/execute.c | 8 +- .../tty-ask-password-agent.c | 2 +- 4 files changed, 81 insertions(+), 81 deletions(-) (limited to 'src') diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index 42336e8fdf..395adc1ea0 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -365,44 +365,36 @@ int open_terminal(const char *name, int mode) { int acquire_terminal( const char *name, - bool fail, - bool force, - bool ignore_tiocstty_eperm, + AcquireTerminalFlags flags, usec_t timeout) { - int fd = -1, notify = -1, r = 0, wd = -1; - usec_t ts = 0; + _cleanup_close_ int notify = -1, fd = -1; + usec_t ts = USEC_INFINITY; + int r, wd = -1; assert(name); + assert(IN_SET(flags & ~ACQUIRE_TERMINAL_PERMISSIVE, ACQUIRE_TERMINAL_TRY, ACQUIRE_TERMINAL_FORCE, ACQUIRE_TERMINAL_WAIT)); - /* 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. + /* 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. * - * Note: strictly speaking this actually watches for the - * device being closed, it does *not* really watch whether a - * tty loses its controlling process. However, unless some - * rogue process uses TIOCNOTTY on /dev/tty *after* closing - * its tty otherwise this will not become a problem. As long - * as the administrator makes sure not configure any service - * on the same tty as an untrusted user this should not be a - * problem. (Which he probably should not do anyway.) */ - - if (timeout != USEC_INFINITY) - ts = now(CLOCK_MONOTONIC); - - if (!fail && !force) { + * Note: strictly speaking this actually watches for the device being closed, it does *not* really watch + * whether a tty loses its controlling process. However, unless some rogue process uses TIOCNOTTY on /dev/tty + * *after* closing its tty otherwise this will not become a problem. As long as the administrator makes sure + * not configure any service on the same tty as an untrusted user this should not be a problem. (Which he + * 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 (notify < 0) { - r = -errno; - goto fail; - } + if (notify < 0) + return -errno; wd = inotify_add_watch(notify, name, IN_CLOSE); - if (wd < 0) { - r = -errno; - goto fail; - } + if (wd < 0) + return -errno; + + if (timeout != USEC_INFINITY) + ts = now(CLOCK_MONOTONIC); } for (;;) { @@ -414,41 +406,43 @@ int acquire_terminal( if (notify >= 0) { r = flush_fd(notify); if (r < 0) - goto fail; + return r; } - /* We pass here O_NOCTTY only so that we can check the return - * value TIOCSCTTY and have a reliable way to figure out if we - * successfully became the controlling process of the tty */ + /* We pass here O_NOCTTY only so that we can check the return value TIOCSCTTY and have a reliable way + * to figure out if we successfully became the controlling process of the tty */ fd = open_terminal(name, O_RDWR|O_NOCTTY|O_CLOEXEC); if (fd < 0) return fd; - /* Temporarily ignore SIGHUP, so that we don't get SIGHUP'ed - * if we already own the tty. */ + /* Temporarily ignore SIGHUP, so that we don't get SIGHUP'ed if we already own the tty. */ assert_se(sigaction(SIGHUP, &sa_new, &sa_old) == 0); /* First, try to get the tty */ - if (ioctl(fd, TIOCSCTTY, force) < 0) - r = -errno; + r = ioctl(fd, TIOCSCTTY, + (flags & ~ACQUIRE_TERMINAL_PERMISSIVE) == ACQUIRE_TERMINAL_FORCE) < 0 ? -errno : 0; + /* Reset signal handler to old value */ assert_se(sigaction(SIGHUP, &sa_old, NULL) == 0); - /* Sometimes, it makes sense to ignore TIOCSCTTY - * returning EPERM, i.e. when very likely we already - * are have this controlling terminal. */ - if (r < 0 && r == -EPERM && ignore_tiocstty_eperm) - r = 0; + /* Success? Exit the loop now! */ + if (r >= 0) + break; - if (r < 0 && (force || fail || r != -EPERM)) - goto fail; + /* Any failure besides -EPERM? Fail, regardless of the mode. */ + if (r != -EPERM) + return r; - if (r >= 0) + if (flags & ACQUIRE_TERMINAL_PERMISSIVE) /* If we are in permissive mode, then EPERM is fine, turn this + * into a success. Note that EPERM is also returned if we + * already are the owner of the TTY. */ break; - assert(!fail); - assert(!force); + if (flags != 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; @@ -458,20 +452,17 @@ int acquire_terminal( if (timeout != USEC_INFINITY) { usec_t n; + assert(ts != USEC_INFINITY); + n = now(CLOCK_MONOTONIC); - if (ts + timeout < n) { - r = -ETIMEDOUT; - goto fail; - } + if (ts + timeout < n) + return -ETIMEDOUT; r = fd_wait_for_event(notify, POLLIN, ts + timeout - n); if (r < 0) - goto fail; - - if (r == 0) { - r = -ETIMEDOUT; - goto fail; - } + return r; + if (r == 0) + return -ETIMEDOUT; } l = read(notify, &buffer, sizeof(buffer)); @@ -479,34 +470,27 @@ int acquire_terminal( if (IN_SET(errno, EINTR, EAGAIN)) continue; - r = -errno; - goto fail; + return -errno; } FOREACH_INOTIFY_EVENT(e, buffer, l) { - if (e->wd != wd || !(e->mask & IN_CLOSE)) { - r = -EIO; - goto fail; - } + if (e->mask & IN_Q_OVERFLOW) /* If we hit an inotify queue overflow, simply check if the terminal is up for grabs now. */ + break; + + if (e->wd != wd || !(e->mask & IN_CLOSE)) /* Safety checks */ + return -EIO; } break; } - /* We close the tty fd here since if the old session - * ended our handle will be dead. It's important that - * we do this after sleeping, so that we don't enter - * an endless loop. */ + /* We close the tty fd here since if the old session ended our handle will be dead. It's important that + * we do this after sleeping, so that we don't enter an endless loop. */ fd = safe_close(fd); } - safe_close(notify); - - return fd; - -fail: - safe_close(fd); - safe_close(notify); + r = fd; + fd = -1; return r; } @@ -630,7 +614,7 @@ int make_console_stdio(void) { /* Make /dev/console the controlling terminal and stdin/stdout/stderr */ - fd = acquire_terminal("/dev/console", false, true, true, USEC_INFINITY); + fd = acquire_terminal("/dev/console", ACQUIRE_TERMINAL_FORCE|ACQUIRE_TERMINAL_PERMISSIVE, USEC_INFINITY); if (fd < 0) return log_error_errno(fd, "Failed to acquire terminal: %m"); diff --git a/src/basic/terminal-util.h b/src/basic/terminal-util.h index e82719b11b..257dbe71da 100644 --- a/src/basic/terminal-util.h +++ b/src/basic/terminal-util.h @@ -52,7 +52,23 @@ int reset_terminal_fd(int fd, bool switch_to_text); int reset_terminal(const char *name); int open_terminal(const char *name, int mode); -int acquire_terminal(const char *name, bool fail, bool force, bool ignore_tiocstty_eperm, usec_t timeout); + +/* Flags for tweaking the way we become the controlling process of a terminal. */ +typedef enum AcquireTerminalFlags { + /* Try to become the controlling process of the TTY. If we can't return -EPERM. */ + ACQUIRE_TERMINAL_TRY = 0, + + /* Tell the kernel to forcibly make us the controlling process of the TTY. Returns -EPERM if the kernel doesn't allow that. */ + ACQUIRE_TERMINAL_FORCE = 1, + + /* If we can't become the controlling process of the TTY right-away, then wait until we can. */ + ACQUIRE_TERMINAL_WAIT = 2, + + /* 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 = 4, +} AcquireTerminalFlags; + +int acquire_terminal(const char *name, AcquireTerminalFlags flags, usec_t timeout); int release_terminal(void); int terminal_vhangup_fd(int fd); diff --git a/src/core/execute.c b/src/core/execute.c index be9ef20725..1381ab4091 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -509,9 +509,9 @@ static int setup_input( int fd; fd = acquire_terminal(exec_context_tty_path(context), - i == EXEC_INPUT_TTY_FAIL, - i == EXEC_INPUT_TTY_FORCE, - false, + i == EXEC_INPUT_TTY_FAIL ? ACQUIRE_TERMINAL_TRY : + i == EXEC_INPUT_TTY_FORCE ? ACQUIRE_TERMINAL_FORCE : + ACQUIRE_TERMINAL_WAIT, USEC_INFINITY); if (fd < 0) return fd; @@ -753,7 +753,7 @@ static int setup_confirm_stdio(const char *vc, int *_saved_stdin, int *_saved_st if (saved_stdout < 0) return -errno; - fd = acquire_terminal(vc, false, false, false, DEFAULT_CONFIRM_USEC); + fd = acquire_terminal(vc, ACQUIRE_TERMINAL_WAIT, DEFAULT_CONFIRM_USEC); if (fd < 0) return fd; 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 baed728bf9..33b7e6010f 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -365,7 +365,7 @@ static int parse_password(const char *filename, char **wall) { if (arg_console) { const char *con = arg_device ?: "/dev/console"; - tty_fd = acquire_terminal(con, false, false, false, USEC_INFINITY); + tty_fd = acquire_terminal(con, ACQUIRE_TERMINAL_WAIT, USEC_INFINITY); if (tty_fd < 0) return log_error_errno(tty_fd, "Failed to acquire /dev/console: %m"); -- cgit v1.2.1