From 1f58235b13a3e6dda0bbff9b66ac583a71b69583 Mon Sep 17 00:00:00 2001 From: Fanda Uchytil Date: Sun, 6 Oct 2019 00:55:13 +0200 Subject: [PATCH] strace: expand -D option As of now, despite of being stated that -D option runs strace as a "detached" grandchild (and the option name being named after "daemon"), strace still runs in the same process group and session, thus not being "detached" in a common sense and being subjected to process group kill and session termination kill. Quoting[1]: I stumble upon unexpected behavior: if strace is used with option '-D' (tracer as a detached grandchild) and process (leader) kills whole process group, it will kill strace too. It can be easily reproduced by `timeout` from "coreutils": # timeout -s KILL 2 strace -D -o ./strace-inside.log /bin/sleep 10 & Here we can see, that `strace` didn't finished its output (because it was killed): # tail -n 1 ./strace-inside.log nanosleep({tv_sec=10, tv_nsec=0}, If `timeout` is not run in '--foreground' mode, it changes process group and after "timeout" it sends two kills: setpgid(0, 0) = 0 kill(37337, SIGKILL) = 0 kill(0, SIGKILL) = ? The first kill is for the `sleep` and the second one is for the process group (which is `strace` part of). PIDs and their relations are: timeout pid=30595 [ppid=476 bash ] pgrp=30595 sleep pid=37337 [ppid=30595 timeout] pgrp=30595 strace pid=30603 [ppid=1 systemd] pgrp=30595 Here is "strace log" of `strace` inside `timeout`: strace: Process 30603 attached wait4(-1, ) = ? +++ killed by SIGKILL +++ I think that detached `strace` should not be killed like that -- it should not be part of former grandparents' "job pipeline". While this behaviour is not exactly intuitive, it is implemented this way for quite some time, so it might be relied upon by some of strace users. In order to address this issue, two new levels of "daemonisation" are added, that put strace in a separate process group and session, respectively. [1] https://lists.strace.io/pipermail/strace-devel/2019-October/009160.html * strace.1.in (.SH SYNOPSIS): Update. (.SS Tracing): Document -DD and -DDD. * strace.c (DAEMONIZE_NONE, DAEMONIZE_GRANDCHILD, DAEMONIZE_NEW_PGROUP, * DAEMONIZE_NEW_SESSION, DAEMONIZE_OPTS_GUARD__, MAX_DAEMONIZE_OPTS): * New enumeration entities. (daemonized_tracer): Change type to unsigned int. (usage): Document -DD and -DDD. (startup_attach) : Call setpgid. : Call setsid. (init) : Increase daemonized_tracer instead of setting to 1. (init): Bail out if too many -D's are given. * NEWS: Mention this improvement. * tests/options-syntax.test: Add checks for -D option usage. Co-authored-by: Eugene Syromyatnikov Co-authored-by: Dmitry V. Levin --- NEWS | 4 ++++ strace.1.in | 25 ++++++++++++++++++--- strace.c | 47 ++++++++++++++++++++++++++++++++++----- tests/options-syntax.test | 4 ++++ 4 files changed, 72 insertions(+), 8 deletions(-) diff --git a/NEWS b/NEWS index e0dc7939..a2dbfbfd 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,10 @@ Noteworthy changes in release ?.? (????-??-??) ============================================== +* Improvements + * Implemented -DD and -DDD options that move strace into a separate + process group and session, respectively. + * Bug fixes * Fixed -b execve when --seccomp-bpf option is specified. * Fixed build on no-MMU architectures. diff --git a/strace.1.in b/strace.1.in index 86058a84..eb872df1 100644 --- a/strace.1.in +++ b/strace.1.in @@ -53,7 +53,7 @@ strace \- trace system calls and signals .BR "" { .OR \-p pid .BR "" | -.OP \-D +.OP \-DDD .OM \-E var\fR[=\fIval\fR] .OP \-u username .IR command " [" args ] @@ -73,7 +73,7 @@ strace \- trace system calls and signals .BR "" { .OR \-p pid .BR "" | -.OP \-D +.OP \-DDD .OM \-E var\fR[=\fIval\fR] .OP -u username .IR command " [" args ] @@ -338,11 +338,30 @@ multi-threaded process and therefore require but don't want to trace its (potentially very complex) children. .TP .B \-D -Run tracer process as a detached grandchild, not as parent of the +Run tracer process as a grandchild, not as the parent of the tracee. This reduces the visible effect of .B strace by keeping the tracee a direct child of the calling process. .TP +.B \-DD +Run tracer process as tracee's grandchild in a separate process group. +In addition to reduction of the visible effect of +.BR strace , +it also avoids killing of +.B strace +with +.BR kill (2) +issued to the whole process group. +.TP +.B \-DDD +Run tracer process as tracee's grandchild in a separate session +("true daemonisation"). +In addition to reduction of the visible effect of +.BR strace , +it also avoids killing of +.B strace +upon session termination. +.TP .B \-f Trace child processes as they are created by currently traced processes as a result of the diff --git a/strace.c b/strace.c index a4de6ae3..1ca39a2c 100644 --- a/strace.c +++ b/strace.c @@ -93,6 +93,15 @@ static int opt_intr; /* We play with signal mask only if this mode is active: */ #define interactive (opt_intr == INTR_WHILE_WAIT) +enum { + DAEMONIZE_NONE = 0, + DAEMONIZE_GRANDCHILD = 1, + DAEMONIZE_NEW_PGROUP = 2, + DAEMONIZE_NEW_SESSION = 3, + + DAEMONIZE_OPTS_GUARD__, + MAX_DAEMONIZE_OPTS = DAEMONIZE_OPTS_GUARD__ - 1 +}; /* * daemonized_tracer supports -D option. * With this option, strace forks twice. @@ -105,7 +114,7 @@ static int opt_intr; * wait() etc. Without -D, strace process gets lodged in between, * disrupting parent<->child link. */ -static bool daemonized_tracer; +static unsigned int daemonized_tracer; static int post_attach_sigstop = TCB_IGNORE_ONE_SIGSTOP; #define use_seize (post_attach_sigstop == 0) @@ -241,10 +250,10 @@ usage(void) usage: strace [-ACdffhi" K_OPT "qqrtttTvVwxxyyzZ] [-I n] [-b execve] [-e expr]...\n\ [-a column] [-o file] [-s strsize] [-X format] [-P path]...\n\ [-p pid]... [--seccomp-bpf]\n\ - { -p pid | [-D] [-E var=val]... [-u username] PROG [ARGS] }\n\ + { -p pid | [-DDD] [-E var=val]... [-u username] PROG [ARGS] }\n\ or: strace -c[dfwzZ] [-I n] [-b execve] [-e expr]... [-O overhead]\n\ [-S sortby] [-P path]... [-p pid]... [--seccomp-bpf]\n\ - { -p pid | [-D] [-E var=val]... [-u username] PROG [ARGS] }\n\ + { -p pid | [-DDD] [-E var=val]... [-u username] PROG [ARGS] }\n\ \n\ Output format:\n\ -A open the file provided in the -o option in append mode\n\ @@ -292,7 +301,9 @@ Filtering:\n\ \n\ Tracing:\n\ -b execve detach on execve syscall\n\ - -D run tracer process as a detached grandchild, not as parent\n\ + -D run tracer process as a grandchild, not as a parent\n\ + -DD run tracer process in a separate process group\n\ + -DDD run tracer process in a separate session\n\ -f follow forks\n\ -ff follow forks with output into separate files\n\ -I interruptible\n\ @@ -1133,6 +1144,27 @@ startup_attach(void) /* grandchild */ /* We will be the tracer process. Remember our new pid: */ strace_tracer_pid = getpid(); + + switch (daemonized_tracer) { + case DAEMONIZE_NEW_PGROUP: + /* + * If -D is passed twice, create a new process group, + * so we won't be killed by kill(0, ...). + */ + if (setpgid(0, 0) < 0) + perror_msg_and_die("Cannot create a new" + " process group"); + break; + case DAEMONIZE_NEW_SESSION: + /* + * If -D is passed thrice, create a new session, + * so we won't be killed upon session termination. + */ + if (setsid() < 0) + perror_msg_and_die("Cannot create a new" + " session"); + break; + } } for (tcbi = 0; tcbi < tcbtabsize; tcbi++) { @@ -1662,7 +1694,7 @@ init(int argc, char *argv[]) debug_flag = 1; break; case 'D': - daemonized_tracer = 1; + daemonized_tracer++; break; case 'e': qualify(optarg); @@ -1786,6 +1818,11 @@ init(int argc, char *argv[]) error_msg_and_help("PROG [ARGS] must be specified with -D"); } + if (daemonized_tracer > (unsigned int) MAX_DAEMONIZE_OPTS) + error_msg_and_help("Too many -D's (%u), maximum supported -D " + "count is %d", + daemonized_tracer, MAX_DAEMONIZE_OPTS); + if (seccomp_filtering && detach_on_execve) { error_msg("--seccomp-bpf is not enabled because" " it is not compatible with -b"); diff --git a/tests/options-syntax.test b/tests/options-syntax.test index d98d0f72..54083e82 100755 --- a/tests/options-syntax.test +++ b/tests/options-syntax.test @@ -24,6 +24,10 @@ check_e_using_grep "$ff_name: File *name too long" -ff -o "$ff_name" true check_h 'must have PROG [ARGS] or -p PID' check_h 'PROG [ARGS] must be specified with -D' -D -p $$ +check_h 'PROG [ARGS] must be specified with -D' -DD -p $$ +check_h 'PROG [ARGS] must be specified with -D' -DDD -p $$ +check_h 'PROG [ARGS] must be specified with -D' -DDDD -p $$ +check_h 'Too many -D'\''s (4), maximum supported -D count is 3' -DDDD /bin/true check_h '-c and -C are mutually exclusive' -c -C true check_h '-c and -C are mutually exclusive' -C -c true check_h '(-c or -C) and -ff are mutually exclusive' -c -ff true -- 2.40.0