From eb0f6de3a5efc57bba81992ad8463494a6f45c4a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Sun, 8 Apr 2018 20:46:32 +0100 Subject: [PATCH 1/3] Add '-p' flag to get signalled when parent dies Add a new flag '-p', which sets up the parent death signal to `SIGKILL`. This will cause the kernel to send us a `SIGKILL` as soon as the direct parent process dies. This is useful e.g. in combination with unshare(1) from util-linux when using PID namespaces. When unshare forks the child, which is about to become PID 1, killing the unshare parent will not cause the child to exit. When executing the command $ unshare --pid --fork tini -- then killing unshare will not cause tini to be killed. Since util-linux v2.32, unshare has an option "--kill-child=" that will set up the parent death signal for the forked process. This does not help though in case either SELinux or AppArmor are in use and credentials of the forked process change (e.g. by changing its UID), as these LSMs will clear the parent death signal again. The following example would trigger that situation: $ unshare --pid --fork setpriv --reuid user tini -s -- The parent death signal will get reset by the LSMs as soon as `setpriv` switchets its user ID to that of "user", and killing unshare will again not result in tini being killed. The new '-p' flag helps that exact scenario: $ unshare --pid --fork setpriv --reuid user tini -s -p SIGKILL -- As soon as unshare is getting killed, tini will get signalled SIGKILL and exit as well, tearing down with it. --- src/tini.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 2 deletions(-) diff --git a/src/tini.c b/src/tini.c index 1faf75c..3753867 100644 --- a/src/tini.c +++ b/src/tini.c @@ -50,17 +50,51 @@ typedef struct { struct sigaction* const sigttou_action_ptr; } signal_configuration_t; +static const struct { + char *const name; + int number; +} signal_names[] = { + { "SIGHUP", SIGHUP }, + { "SIGINT", SIGINT }, + { "SIGQUIT", SIGQUIT }, + { "SIGILL", SIGILL }, + { "SIGTRAP", SIGTRAP }, + { "SIGABRT", SIGABRT }, + { "SIGBUS", SIGBUS }, + { "SIGFPE", SIGFPE }, + { "SIGKILL", SIGKILL }, + { "SIGUSR1", SIGUSR1 }, + { "SIGSEGV", SIGSEGV }, + { "SIGUSR2", SIGUSR2 }, + { "SIGPIPE", SIGPIPE }, + { "SIGALRM", SIGALRM }, + { "SIGTERM", SIGTERM }, + { "SIGCHLD", SIGCHLD }, + { "SIGCONT", SIGCONT }, + { "SIGSTOP", SIGSTOP }, + { "SIGTSTP", SIGTSTP }, + { "SIGTTIN", SIGTTIN }, + { "SIGTTOU", SIGTTOU }, + { "SIGURG", SIGURG }, + { "SIGXCPU", SIGXCPU }, + { "SIGXFSZ", SIGXFSZ }, + { "SIGVTALRM", SIGVTALRM }, + { "SIGPROF", SIGPROF }, + { "SIGWINCH", SIGWINCH }, + { "SIGSYS", SIGSYS }, +}; + static unsigned int verbosity = DEFAULT_VERBOSITY; static int32_t expect_status[(STATUS_MAX - STATUS_MIN + 1) / 32]; #ifdef PR_SET_CHILD_SUBREAPER #define HAS_SUBREAPER 1 -#define OPT_STRING "hvwgle:s" +#define OPT_STRING "p:hvwgle:s" #define SUBREAPER_ENV_VAR "TINI_SUBREAPER" #else #define HAS_SUBREAPER 0 -#define OPT_STRING "hvwgle:" +#define OPT_STRING "p:hvwgle:" #endif #define VERBOSITY_ENV_VAR "TINI_VERBOSITY" @@ -71,6 +105,7 @@ static int32_t expect_status[(STATUS_MAX - STATUS_MIN + 1) / 32]; #if HAS_SUBREAPER static unsigned int subreaper = 0; #endif +static unsigned int parent_death_signal = 0; static unsigned int kill_process_group = 0; static unsigned int warn_on_reap = 0; @@ -207,6 +242,7 @@ void print_usage(char* const name, FILE* const file) { #if HAS_SUBREAPER fprintf(file, " -s: Register as a process subreaper (requires Linux >= 3.4).\n"); #endif + fprintf(file, " -p SIGNAL: Trigger SIGNAL when parent dies, e.g. \"-p SIGKILL\".\n"); fprintf(file, " -v: Generate more verbose output. Repeat up to 3 times.\n"); fprintf(file, " -w: Print a warning when processes are getting reaped.\n"); fprintf(file, " -g: Send signals to the child's process group.\n"); @@ -235,6 +271,20 @@ void print_license(FILE* const file) { } } +int set_pdeathsig(char* const arg) { + size_t i; + + for (i = 0; i < ARRAY_LEN(signal_names); i++) { + if (strcmp(signal_names[i].name, arg) == 0) { + /* Signals start at value "1" */ + parent_death_signal = signal_names[i].number; + return 0; + } + } + + return 1; +} + int add_expect_status(char* arg) { long status = 0; char* endptr = NULL; @@ -276,6 +326,14 @@ int parse_args(const int argc, char* const argv[], char* (**child_args_ptr_ptr)[ subreaper++; break; #endif + case 'p': + if (set_pdeathsig(optarg)) { + PRINT_FATAL("Not a valid option for -p: %s", optarg); + *parse_fail_exitcode_ptr = 1; + return 1; + } + break; + case 'v': verbosity++; break; @@ -575,6 +633,12 @@ int main(int argc, char *argv[]) { return 1; } + /* Trigger signal on this process when the parent process exits. */ + if (parent_death_signal && prctl(PR_SET_PDEATHSIG, parent_death_signal)) { + PRINT_FATAL("Failed to set up parent death signal"); + return 1; + } + #if HAS_SUBREAPER /* If available and requested, register as a subreaper */ if (register_subreaper()) { From 1487373aa2e870950b1a9f4c1ce748d91e1547a5 Mon Sep 17 00:00:00 2001 From: Thomas Orozco Date: Thu, 19 Apr 2018 16:58:00 +0200 Subject: [PATCH 2/3] Exercise pdeathsignal in tests See: https://github.com/krallin/tini/pull/114#issuecomment-382756277 --- test/pdeathsignal/stage_1.py | 30 ++++++++++++++++++++++++++++++ test/pdeathsignal/stage_2.py | 25 +++++++++++++++++++++++++ test/run_inner_tests.py | 18 +++++++++++++++++- 3 files changed, 72 insertions(+), 1 deletion(-) create mode 100755 test/pdeathsignal/stage_1.py create mode 100755 test/pdeathsignal/stage_2.py diff --git a/test/pdeathsignal/stage_1.py b/test/pdeathsignal/stage_1.py new file mode 100755 index 0000000..60dd463 --- /dev/null +++ b/test/pdeathsignal/stage_1.py @@ -0,0 +1,30 @@ +#!/usr/bin/env python +from __future__ import print_function + +import os +import sys +import subprocess + + +def main(): + pid = os.getpid() + + tini = sys.argv[1] + ret = sys.argv[2] + stage_2 = os.path.join(os.path.dirname(__file__), "stage_2.py") + + cmd = [ + tini, + "-vvv", + "-p", + "SIGUSR1", + "--", + stage_2, + str(pid), + ret + ] + + subprocess.Popen(cmd).wait() + +if __name__ == "__main__": + main() diff --git a/test/pdeathsignal/stage_2.py b/test/pdeathsignal/stage_2.py new file mode 100755 index 0000000..43eb537 --- /dev/null +++ b/test/pdeathsignal/stage_2.py @@ -0,0 +1,25 @@ +#!/usr/bin/env python +from __future__ import print_function + +import os +import sys +import signal +import time + + +def main(): + ret = sys.argv[2] + + def handler(*args): + with open(ret, "w") as f: + f.write("ok") + sys.exit(0) + + signal.signal(signal.SIGUSR1, handler) + pid = int(sys.argv[1]) + + os.kill(pid, signal.SIGKILL) + time.sleep(5) + +if __name__ == "__main__": + main() diff --git a/test/run_inner_tests.py b/test/run_inner_tests.py index 47e1a10..4468797 100755 --- a/test/run_inner_tests.py +++ b/test/run_inner_tests.py @@ -9,6 +9,7 @@ import psutil import bitmap import re import itertools +import tempfile DEVNULL = open(os.devnull, 'wb') @@ -133,7 +134,7 @@ def main(): assert ret == 1, "Reaping test succeeded (it should have failed)!" # Test that the signals are properly in place here. - print "running signal configuration test" + print "Running signal configuration test" p = subprocess.Popen([os.path.join(build, "sigconf-test"), tini, "cat", "/proc/self/status"], stdout=subprocess.PIPE, stderr=subprocess.PIPE) out, err = p.communicate() @@ -156,6 +157,21 @@ def main(): # Use signum - 1 because the bitmap is 0-indexed but represents signals strting at 1 assert (signum - 1) in props[signal_set_name].nonzero(), "{0} ({1}) is missing in {2}!".format(SIGNUM_TO_SIGNAME[signum], signum, signal_set_name) + # Test parent death signal handling. + if not args_disabled: + print "Running parent death signal test" + f = tempfile.NamedTemporaryFile() + try: + p = subprocess.Popen( + [os.path.join(src, "test", "pdeathsignal", "stage_1.py"), tini, f.name], + stdout=DEVNULL, stderr=DEVNULL + ) + p.wait() + + busy_wait(lambda: open(f.name).read() == "ok", 10) + finally: + f.close() + print "---------------------------" print "All done, tests as expected" print "---------------------------" From a500ee72c110367b5ccaa93640278ffd5cfd84e2 Mon Sep 17 00:00:00 2001 From: Thomas Orozco Date: Thu, 19 Apr 2018 19:37:25 +0200 Subject: [PATCH 3/3] Update README for parent death signal --- README.md | 17 +++++++++++++++++ tpl/README.md.in | 17 +++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/README.md b/README.md index ed2eed8..f2094da 100644 --- a/README.md +++ b/README.md @@ -175,6 +175,20 @@ closely to what happens when you do ctrl-C etc. in a terminal: The signal is sent to the foreground process group. +### Parent Death Signal ### + +Tini can set its parent death signal, which is the signal Tini should receive +when *its* parent exits. To set the parent death signal, use the `-p` flag with +the name of the signal Tini should receive when its parent exits: + +``` +tini -p SIGTERM -- ... +``` + +*NOTE: See [this PR discussion][12] to learn more about the parent death signal +and use cases.* + + More ---- @@ -257,6 +271,7 @@ Contributors: + [David Wragg][31] + [Michael Crosby][32] + [Wyatt Preul][33] + + [Patrick Steinhardt][34] Special thanks to: @@ -268,10 +283,12 @@ Special thanks to: [5]: https://docs.docker.com/engine/reference/commandline/run/ [10]: https://github.com/krallin/tini-images [11]: https://github.com/krallin/tini/releases + [12]: https://github.com/krallin/tini/pull/114 [20]: https://github.com/krallin/ [30]: https://github.com/tianon [31]: https://github.com/dpw [32]: https://github.com/crosbymichael [33]: https://github.com/geek + [34]: https://github.com/pks-t [40]: https://github.com/danilobuerger [41]: https://github.com/datakurre diff --git a/tpl/README.md.in b/tpl/README.md.in index 6e0ce2d..9f05383 100644 --- a/tpl/README.md.in +++ b/tpl/README.md.in @@ -175,6 +175,20 @@ closely to what happens when you do ctrl-C etc. in a terminal: The signal is sent to the foreground process group. +### Parent Death Signal ### + +Tini can set its parent death signal, which is the signal Tini should receive +when *its* parent exits. To set the parent death signal, use the `-p` flag with +the name of the signal Tini should receive when its parent exits: + +``` +tini -p SIGTERM -- ... +``` + +*NOTE: See [this PR discussion][12] to learn more about the parent death signal +and use cases.* + + More ---- @@ -257,6 +271,7 @@ Contributors: + [David Wragg][31] + [Michael Crosby][32] + [Wyatt Preul][33] + + [Patrick Steinhardt][34] Special thanks to: @@ -268,10 +283,12 @@ Special thanks to: [5]: https://docs.docker.com/engine/reference/commandline/run/ [10]: https://github.com/krallin/tini-images [11]: https://github.com/krallin/tini/releases + [12]: https://github.com/krallin/tini/pull/114 [20]: https://github.com/krallin/ [30]: https://github.com/tianon [31]: https://github.com/dpw [32]: https://github.com/crosbymichael [33]: https://github.com/geek + [34]: https://github.com/pks-t [40]: https://github.com/danilobuerger [41]: https://github.com/datakurre