[PATCH v2] strace: expand -D option
Fanda Uchytil
strace.t8xuewpmde at h4x.cz
Mon Oct 7 05:33:24 UTC 2019
2019-10-06 00:55:13, Eugene Syromiatnikov wrote:
> From: Fanda Uchytil <strace.t8xuewpmde at h4x.cz>
>
> As of now, despit 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, <unfinished ...>) = ?
> +++ 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 that way for quite
> some time, so it might be relied upon by some of the 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 setsid if daemonized_tracer is no less than
> DAEMONIZE_NEW_SESSION; call setpgid if daemonized_tracer is equal
> to DAEMONIZE_NEW_PGROUP.
> (init) <case 'D'>: Increase daemonized_tracer instead of setting to 1.
> (init): Bail out if to many -D's are given.
> * tests/options-syntax.test: Add checks for -D option usage.
>
> Co-authored-by: Eugene Syromyatnikov <evgsyr at gmail.com>
> ---
> strace.1.in | 25 ++++++++++++++++++++++---
> strace.c | 43 ++++++++++++++++++++++++++++++++++++++-----
> tests/options-syntax.test | 4 ++++
> 3 files changed, 64 insertions(+), 8 deletions(-)
>
> diff --git a/strace.1.in b/strace.1.in
> index 86058a8..eb872df 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 b52a3db..b7388d1 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 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\
> @@ -1131,6 +1142,23 @@ startup_attach(void)
> _exit(0); /* paranoia */
> }
> /* grandchild */
> + if (daemonized_tracer >= 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("Can't create "
> + "a new session");
> + } else if (daemonized_tracer == 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("Can't create "
> + "a new process group");
> + }
> /* We will be the tracer process. Remember our new pid: */
> strace_tracer_pid = getpid();
> }
> @@ -1662,7 +1690,7 @@ init(int argc, char *argv[])
> debug_flag = 1;
> break;
> case 'D':
> - daemonized_tracer = 1;
> + daemonized_tracer++;
> break;
> case 'e':
> qualify(optarg);
> @@ -1786,6 +1814,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) {
> if (nprocs && (!argc || debug_flag))
> error_msg("--seccomp-bpf is not enabled for processes"
> diff --git a/tests/options-syntax.test b/tests/options-syntax.test
> index 141689a..6a916f1 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.1.4
>
> --
> Strace-devel mailing list
> Strace-devel at lists.strace.io
> https://lists.strace.io/mailman/listinfo/strace-devel
Great solution, thank you!
More information about the Strace-devel
mailing list