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,  <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 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) <daemonized_tracer == DAEMONIZE_NEW_PGROUP>: Call
setpgid.
<daemonized_tracer == DAEMONIZE_NEW_SESSION>: Call setsid.
(init) <case 'D'>: 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 <evgsyr@gmail.com>
Co-authored-by: Dmitry V. Levin <ldv@altlinux.org>

diff --git a/NEWS b/NEWS
index e0dc79394809c76547811e622528fd8465fefd62..a2dbfbfd35108a5c3f180045fa0ecc2a5b1f3463 100644 (file)
--- 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 86058a841e1bab0f279a307651f0a36e3c1a0ccc..eb872df1870d5f6a5d67d749bc9a18303a36022e 100644 (file)
--- 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 a4de6ae3979ea216db8ec50a302130afb0319cec..1ca39a2ccc6d36fa199c48a4eb4ce3a4ee47b884 100644 (file)
--- 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 d98d0f7205aed52ee3a844127ec270c36e0af541..54083e822e75f5115da0d1a07a43cb871c4b60d5 100755 (executable)
--- 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