From: Todd C. Miller Date: Wed, 5 Feb 2014 19:03:58 +0000 (-0700) Subject: In term_restore(), only restores the terminal if we are in the X-Git-Tag: SUDO_1_8_10^2~46 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=135c85e1528e9ba41cb8d26ceb390926b39b76c1;p=sudo In term_restore(), only restores the terminal if we are in the foregroup process group. Instead of calling tcgetpgrp(), which is racy, we set a temporary handler for SIGTTOU and check whether it was received after a failed call to tcsetattr(). --- diff --git a/common/term.c b/common/term.c index b534c2982..e03b29b06 100644 --- a/common/term.c +++ b/common/term.c @@ -36,7 +36,9 @@ # include #endif /* HAVE_STRINGS_H */ #include +#include #include +#include #include "missing.h" #include "sudo_debug.h" @@ -70,24 +72,47 @@ static int changed; int term_erase; int term_kill; +static volatile sig_atomic_t got_sigttou; + /* - * Like tcsetattr() but restarts on EINTR. + * SIGTTOU signal handler for term_restore that just sets a flag. + */ +static void sigttou(int signo) +{ + got_sigttou = 1; +} + +/* + * Like tcsetattr() but restarts on EINTR _except_ for SIGTTOU. * Returns 0 on success or -1 on failure, setting errno. + * Sets got_sigttou on failure if interrupted by SIGTTOU. */ static int -tcsetattr_nointr(int fd, int flags, struct termios *tp) +tcsetattr_nobg(int fd, int flags, struct termios *tp) { + sigaction_t sa, osa; int rc; + /* + * If we receive SIGTTOU from tcsetattr() it means we are + * not in the foreground process group. + * This should be less racy than using tcgetpgrp(). + */ + memset(&sa, 0, sizeof(sa)); + sigemptyset(&sa.sa_mask); + sa.sa_handler = sigttou; + got_sigttou = 0; + sigaction(SIGTTOU, &sa, &osa); do { rc = tcsetattr(fd, flags, tp); - } while (rc != 0 && errno == EINTR); + } while (rc != 0 && errno == EINTR && !got_sigttou); + sigaction(SIGTTOU, &osa, NULL); return rc; } /* - * Restore saved terminal settings. + * Restore saved terminal settings if we are in the foreground process group. * Returns true on success or false on failure. */ bool @@ -97,7 +122,7 @@ term_restore(int fd, bool flush) if (changed) { const int flags = flush ? (TCSASOFT|TCSAFLUSH) : (TCSASOFT|TCSADRAIN); - if (tcsetattr_nointr(fd, flags, &oterm) != 0) + if (tcsetattr_nobg(fd, flags, &oterm) != 0) debug_return_bool(false); changed = 0; } @@ -113,6 +138,7 @@ term_noecho(int fd) { debug_decl(term_noecho, SUDO_DEBUG_UTIL) +again: if (!changed && tcgetattr(fd, &oterm) != 0) debug_return_bool(false); (void) memcpy(&term, &oterm, sizeof(term)); @@ -120,10 +146,15 @@ term_noecho(int fd) #ifdef VSTATUS term.c_cc[VSTATUS] = _POSIX_VDISABLE; #endif - if (tcsetattr_nointr(fd, TCSADRAIN|TCSASOFT, &term) == 0) { + if (tcsetattr_nobg(fd, TCSADRAIN|TCSASOFT, &term) == 0) { changed = 1; debug_return_bool(true); } + if (got_sigttou) { + /* We were in the background, so oterm is probably bogus. */ + kill(getpid(), SIGTTOU); + goto again; + } debug_return_bool(false); } @@ -137,6 +168,7 @@ term_raw(int fd, int isig) struct termios term; debug_decl(term_raw, SUDO_DEBUG_UTIL) +again: if (!changed && tcgetattr(fd, &oterm) != 0) return 0; (void) memcpy(&term, &oterm, sizeof(term)); @@ -148,10 +180,15 @@ term_raw(int fd, int isig) CLR(term.c_lflag, ECHO | ICANON | ISIG | IEXTEN); if (isig) SET(term.c_lflag, ISIG); - if (tcsetattr_nointr(fd, TCSADRAIN|TCSASOFT, &term) == 0) { + if (tcsetattr_nobg(fd, TCSADRAIN|TCSASOFT, &term) == 0) { changed = 1; debug_return_bool(true); } + if (got_sigttou) { + /* We were in the background, so oterm is probably bogus. */ + kill(getpid(), SIGTTOU); + goto again; + } debug_return_bool(false); } @@ -164,6 +201,7 @@ term_cbreak(int fd) { debug_decl(term_cbreak, SUDO_DEBUG_UTIL) +again: if (!changed && tcgetattr(fd, &oterm) != 0) return 0; (void) memcpy(&term, &oterm, sizeof(term)); @@ -177,12 +215,17 @@ term_cbreak(int fd) #ifdef VSTATUS term.c_cc[VSTATUS] = _POSIX_VDISABLE; #endif - if (tcsetattr_nointr(fd, TCSADRAIN|TCSASOFT, &term) == 0) { + if (tcsetattr_nobg(fd, TCSADRAIN|TCSASOFT, &term) == 0) { term_erase = term.c_cc[VERASE]; term_kill = term.c_cc[VKILL]; changed = 1; debug_return_bool(true); } + if (got_sigttou) { + /* We were in the background, so oterm is probably bogus. */ + kill(getpid(), SIGTTOU); + goto again; + } debug_return_bool(false); } @@ -196,9 +239,15 @@ term_copy(int src, int dst) struct termios tt; debug_decl(term_copy, SUDO_DEBUG_UTIL) +again: if (tcgetattr(src, &tt) != 0) debug_return_bool(false); - if (tcsetattr_nointr(dst, TCSANOW|TCSASOFT, &tt) != 0) - debug_return_bool(false); - debug_return_bool(true); + if (tcsetattr_nobg(dst, TCSANOW|TCSASOFT, &tt) == 0) + debug_return_bool(true); + if (got_sigttou) { + /* We were in the background, so oterm is probably bogus. */ + kill(getpid(), SIGTTOU); + goto again; + } + debug_return_bool(false); } diff --git a/src/exec_pty.c b/src/exec_pty.c index 0287c5d1b..d2975d54a 100644 --- a/src/exec_pty.c +++ b/src/exec_pty.c @@ -115,11 +115,8 @@ pty_cleanup(void) { debug_decl(cleanup, SUDO_DEBUG_EXEC); - if (!TAILQ_EMPTY(&io_plugins) && io_fds[SFD_USERTTY] != -1) { - check_foreground(); - if (foreground) - term_restore(io_fds[SFD_USERTTY], 0); - } + if (!TAILQ_EMPTY(&io_plugins) && io_fds[SFD_USERTTY] != -1) + term_restore(io_fds[SFD_USERTTY], 0); #ifdef HAVE_SELINUX selinux_restore_tty(); #endif @@ -812,11 +809,8 @@ pty_close(struct command_status *cstat) } /* Restore terminal settings. */ - if (io_fds[SFD_USERTTY] != -1) { - check_foreground(); - if (foreground) - term_restore(io_fds[SFD_USERTTY], 0); - } + if (io_fds[SFD_USERTTY] != -1) + term_restore(io_fds[SFD_USERTTY], 0); /* If child was signalled, write the reason to stdout like the shell. */ if (cstat->type == CMD_WSTATUS && WIFSIGNALED(cstat->val)) {