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().
This commit is contained in:
Todd C. Miller
2014-02-05 12:03:58 -07:00
parent 85598f77b2
commit 135c85e152
2 changed files with 64 additions and 21 deletions

View File

@@ -36,7 +36,9 @@
# include <strings.h>
#endif /* HAVE_STRINGS_H */
#include <errno.h>
#include <signal.h>
#include <termios.h>
#include <unistd.h>
#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);
}

View File

@@ -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)) {