From 7d3aa2fc2b9fb67e8427612471559a498722b1a6 Mon Sep 17 00:00:00 2001 From: Chris Kuehl Date: Mon, 4 Jun 2018 12:39:14 -0700 Subject: [PATCH 1/2] Fix SIGHUP/SIGCONT race condition --- dumb-init.c | 46 ++++++++++++++++++++++++++++++++++++++-------- tests/tty_test.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/dumb-init.c b/dumb-init.c index f437b10..f13d4cb 100644 --- a/dumb-init.c +++ b/dumb-init.c @@ -37,7 +37,10 @@ #define MAXSIG 31 // Indices are one-indexed (signal 1 is at index 1). Index zero is unused. +// User-specified signal rewriting. int signal_rewrite[MAXSIG + 1] = {[0 ... MAXSIG] = -1}; +// One-time ignores due to TTY quirks. 0 = no skip, 1 = skip the next-received signal. +char signal_temporary_ignores[MAXSIG + 1] = {[0 ... MAXSIG] = 0}; pid_t child_pid = -1; char debug = 0; @@ -89,7 +92,11 @@ void forward_signal(int signum) { */ void handle_signal(int signum) { DEBUG("Received signal %d.\n", signum); - if (signum == SIGCHLD) { + + if (signal_temporary_ignores[signum] == 1) { + DEBUG("Ignoring tty hand-off signal %d.\n", signum); + signal_temporary_ignores[signum] = 0; + } else if (signum == SIGCHLD) { int status, exit_status; pid_t killed_pid; while ((killed_pid = waitpid(-1, &status, WNOHANG)) > 0) { @@ -251,13 +258,36 @@ int main(int argc, char *argv[]) { for (i = 1; i <= MAXSIG; i++) signal(i, dummy); - /* detach dumb-init from controlling tty */ - if (use_setsid && ioctl(STDIN_FILENO, TIOCNOTTY) == -1) { - DEBUG( - "Unable to detach from controlling tty (errno=%d %s).\n", - errno, - strerror(errno) - ); + /* + * Detach dumb-init from controlling tty, so that the child's session can + * attach to it instead. + * + * We want the child to be able to be the session leader of the TTY so that + * it can do normal job control. + */ + if (use_setsid) { + if (ioctl(STDIN_FILENO, TIOCNOTTY) == -1) { + DEBUG( + "Unable to detach from controlling tty (errno=%d %s).\n", + errno, + strerror(errno) + ); + } else { + /* + * When the session leader detaches from its controlling tty via + * TIOCNOTTY, the kernel sends SIGHUP and SIGCONT to the process + * group. We need to be careful not to forward these on to the + * dumb-init child so that it doesn't receive a SIGHUP and + * terminate itself (#136). + */ + if (getsid(0) == getpid()) { + DEBUG("Detached from controlling tty, ignoring the first SIGHUP and SIGCONT we receive.\n"); + signal_temporary_ignores[SIGHUP] = 1; + signal_temporary_ignores[SIGCONT] = 1; + } else { + DEBUG("Detached from controlling tty, but was not session leader.\n"); + } + } } child_pid = fork(); diff --git a/tests/tty_test.py b/tests/tty_test.py index 0c9acc6..3e8693d 100644 --- a/tests/tty_test.py +++ b/tests/tty_test.py @@ -1,7 +1,9 @@ import os import pty import re +import signal import termios +import time import pytest @@ -81,3 +83,36 @@ def test_child_gets_controlling_tty_if_we_had_one(): # "m" is job control flags = m.group(1) assert b'm' in flags + + +def test_sighup_sigcont_ignored_if_was_session_leader(): + """The first SIGHUP/SIGCONT should be ignored if dumb-init is the session leader. + + Due to TTY quirks (#136), when dumb-init is the session leader and forks, + it needs to avoid forwarding the first SIGHUP and SIGCONT to the child. + Otherwise, the child might receive the SIGHUP post-exec and terminate + itself. + + You can "force" this race by adding a `sleep(1)` before the signal handling + loop in dumb-init's code, but it's hard to reproduce the race reliably in a + test otherwise. Because of this, we're stuck just asserting debug messages. + """ + pid, fd = pty.fork() + if pid == 0: + # child + os.execvp('dumb-init', ('dumb-init', '-v', 'sleep', '20')) + else: + # parent + ttyflags(fd) + + # send another SIGCONT to make sure only the first is ignored + time.sleep(0.5) + os.kill(pid, signal.SIGHUP) + + output = readall(fd).decode('UTF-8') + + assert 'Ignoring tty hand-off signal {}.'.format(signal.SIGHUP) in output + assert 'Ignoring tty hand-off signal {}.'.format(signal.SIGCONT) in output + + assert '[dumb-init] Forwarded signal {} to children.'.format(signal.SIGHUP) in output + assert '[dumb-init] Forwarded signal {} to children.'.format(signal.SIGCONT) not in output From 53ca2265d1225aa2734532011b49de2d4993be47 Mon Sep 17 00:00:00 2001 From: Chris Kuehl Date: Wed, 1 Aug 2018 16:26:27 -0700 Subject: [PATCH 2/2] Temporarily allow the linux-ppc64le tests to fail This is due to #175. --- .travis.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.travis.yml b/.travis.yml index 0eb8065..6aa987b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,6 +11,10 @@ matrix: - env: ITEST_TARGET=itest_tox - os: linux-ppc64le env: ITEST_TARGET=itest_stretch + allow_failures: + - os: linux-ppc64le + env: ITEST_TARGET=itest_stretch + script: - make "$ITEST_TARGET"