From b1154c4ece2192345efe1a3fb835ae28c8f92154 Mon Sep 17 00:00:00 2001 From: Karel Zak Date: Wed, 9 Oct 2019 16:43:50 +0200 Subject: [PATCH] script: fix signalfd use It's necessary to create signal-fd before fork() to get SIGCHLD, because child could be faster than our code. Signed-off-by: Karel Zak --- lib/pty-session.c | 74 +++++++++++++++++++++------------- term-utils/script.c | 8 ++-- tests/ts/script/buffering-race | 3 +- 3 files changed, 52 insertions(+), 33 deletions(-) diff --git a/lib/pty-session.c b/lib/pty-session.c index 10eb462d28..5486fd0688 100644 --- a/lib/pty-session.c +++ b/lib/pty-session.c @@ -128,11 +128,24 @@ void ul_pty_set_mainloop_time(struct ul_pty *pty, struct timeval *tv) } } +static void pty_signals_cleanup(struct ul_pty *pty) +{ + if (pty->sigfd != -1) + close(pty->sigfd); + pty->sigfd = -1; + + /* restore original setting */ + sigprocmask(SIG_SETMASK, &pty->orgsig, NULL); +} + /* call me before fork() */ int ul_pty_setup(struct ul_pty *pty) { struct termios slave_attrs; - int rc; + sigset_t ourset; + int rc = 0; + + assert(pty->sigfd == -1); /* save the current signals setting */ sigprocmask(0, NULL, &pty->orgsig); @@ -141,11 +154,15 @@ int ul_pty_setup(struct ul_pty *pty) DBG(SETUP, ul_debugobj(pty, "create for terminal")); /* original setting of the current terminal */ - if (tcgetattr(STDIN_FILENO, &pty->stdin_attrs) != 0) - return -errno; + if (tcgetattr(STDIN_FILENO, &pty->stdin_attrs) != 0) { + rc = -errno; + goto done; + } ioctl(STDIN_FILENO, TIOCGWINSZ, (char *)&pty->win); /* create master+slave */ rc = openpty(&pty->master, &pty->slave, NULL, &pty->stdin_attrs, &pty->win); + if (rc) + goto done; /* set the current terminal to raw mode; pty_cleanup() reverses this change on exit */ slave_attrs = pty->stdin_attrs; @@ -160,9 +177,30 @@ int ul_pty_setup(struct ul_pty *pty) tcgetattr(pty->slave, &slave_attrs); slave_attrs.c_lflag &= ~ECHO; tcsetattr(pty->slave, TCSANOW, &slave_attrs); - } + } else + goto done; + } + + sigfillset(&ourset); + if (sigprocmask(SIG_BLOCK, &ourset, NULL)) { + rc = -errno; + goto done; } + sigemptyset(&ourset); + sigaddset(&ourset, SIGCHLD); + sigaddset(&ourset, SIGWINCH); + sigaddset(&ourset, SIGALRM); + sigaddset(&ourset, SIGTERM); + sigaddset(&ourset, SIGINT); + sigaddset(&ourset, SIGQUIT); + + if ((pty->sigfd = signalfd(-1, &ourset, SFD_CLOEXEC)) < 0) + rc = -errno; +done: + if (rc) + ul_pty_cleanup(pty); + DBG(SETUP, ul_debugobj(pty, "pty setup done [master=%d, slave=%d, rc=%d]", pty->master, pty->slave, rc)); return rc; @@ -173,6 +211,8 @@ void ul_pty_cleanup(struct ul_pty *pty) { struct termios rtt; + pty_signals_cleanup(pty); + if (pty->master == -1 || !pty->isterm) return; @@ -434,7 +474,6 @@ static int handle_signal(struct ul_pty *pty, int fd) /* loop in parent */ int ul_pty_proxy_master(struct ul_pty *pty) { - sigset_t ourset; int rc = 0, ret, eof = 0; enum { POLLFD_SIGNAL = 0, @@ -451,22 +490,7 @@ int ul_pty_proxy_master(struct ul_pty *pty) /* We use signalfd and standard signals by handlers are blocked * at all */ - sigfillset(&ourset); - if (sigprocmask(SIG_BLOCK, &ourset, NULL)) - return -errno; - - sigemptyset(&ourset); - sigaddset(&ourset, SIGCHLD); - sigaddset(&ourset, SIGWINCH); - sigaddset(&ourset, SIGALRM); - sigaddset(&ourset, SIGTERM); - sigaddset(&ourset, SIGINT); - sigaddset(&ourset, SIGQUIT); - - if ((pty->sigfd = signalfd(-1, &ourset, SFD_CLOEXEC)) < 0) { - rc = -errno; - goto done; - } + assert(pty->sigfd >= 0); pfd[POLLFD_SIGNAL].fd = pty->sigfd; pty->poll_timeout = -1; @@ -569,13 +593,7 @@ int ul_pty_proxy_master(struct ul_pty *pty) } } -done: - if (pty->sigfd != -1) - close(pty->sigfd); - pty->sigfd = -1; - - /* restore original setting */ - sigprocmask(SIG_SETMASK, &pty->orgsig, NULL); + pty_signals_cleanup(pty); DBG(IO, ul_debug("poll() done [signal=%d, rc=%d]", pty->delivered_signal, rc)); return rc; diff --git a/term-utils/script.c b/term-utils/script.c index d1f326d074..8d0b45f646 100644 --- a/term-utils/script.c +++ b/term-utils/script.c @@ -893,6 +893,10 @@ int main(int argc, char **argv) printf(_(".\n")); } +#ifdef HAVE_LIBUTEMPTER + utempter_add_record(ul_pty_get_childfd(ctl.pty), NULL); +#endif + if (ul_pty_setup(ctl.pty)) err(EXIT_FAILURE, _("failed to create pseudo-terminal")); @@ -902,10 +906,6 @@ int main(int argc, char **argv) * We have terminal, do not use err() from now, use "goto done" */ -#ifdef HAVE_LIBUTEMPTER - utempter_add_record(ul_pty_get_childfd(ctl.pty), NULL); -#endif - switch ((int) (ctl.child = fork())) { case -1: /* error */ warn(_("cannot create child process")); diff --git a/tests/ts/script/buffering-race b/tests/ts/script/buffering-race index 51421faeb6..b7203837e4 100755 --- a/tests/ts/script/buffering-race +++ b/tests/ts/script/buffering-race @@ -20,7 +20,8 @@ ts_init "$*" ts_check_test_command "$TS_CMD_SCRIPT" -SCRIPT_DEBUG=all ULPTY_DEBUG=all $TS_CMD_SCRIPT -c "echo Hallo World" /dev/null $TS_OUTPUT +#SCRIPT_DEBUG=all ULPTY_DEBUG=all +$TS_CMD_SCRIPT -c "echo Hallo World" /dev/null $TS_OUTPUT ts_finalize -- 2.39.2