[DTrace-devel] [PATCH v2 11/23] proc: don't ptrace shortlived-grabbed processes if someone else is ptracing them

[Nick Alcock]

Shortlived grabs, by definition, are grabs that won't last long and where we
don't expect things like dlopen monitoring to be useful.  We ptrace for
this, and to extract dynamic loader scope info and take things like symbol
interposition into accout when doing symbol lookup; but if someone else is
already tracing the process we should probably just try not ptracing it,
since the worst that will happen if we do is that some name lookups of
interposed symbols or symbols in alternate dlmopen namespaces might yield
the wrong address (probably leading to a probe not firing when it
should).

Interposed symbols are rare enough that this failure mode is probably not
justification for refusing to trace entirely (perhaps a warning message
might be desirable, but honestly I think it's rare enough that not even that
is justifiable: most ptrace failures will have no visible negative
consequences at all, since not only must an interposed symbol exist but we
must also be tracing it *and* tracing the process with multiple debuggers at
once, which seems likely to be very rare: nearly all warnings will be
warning when nothing will go wrong at all).

It is even rarer in the modern toolchain world in which many ELF objects
cannot be interposed at all, whether due to -z now or
-fno-semantic-interposition or whatever.

Doing this makes it practical to have multiple dtraces USDT-probing the same
process at the same time: one of them will ptrace it, the other one won't,
and both will get the mapping info they need for a
reliable-enough-to-be-useful trace (and if we're lucky and can ptrace,
symbol resolution quality goes up a bit).

test/unittest/usdt/tst.multitrace.sh tests this (though there are other
things stopping it working well, it cannot work at all without something
like this).

Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
---
 libdtrace/dt_proc.c | 16 ++++++++++++----
 libproc/Pcontrol.c  | 18 ++++++++++++++++++
 libproc/libproc.h   |  1 +
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/libdtrace/dt_proc.c b/libdtrace/dt_proc.c
index dd5f69a60e17..73f35135a7b6 100644
--- a/libdtrace/dt_proc.c
+++ b/libdtrace/dt_proc.c
@@ -931,16 +931,24 @@ dt_proc_control(void *arg)
 		 * be worth ptracing it so that we get better symbol resolution,
 		 * but if the process is a crucial system daemon, avoid ptracing
 		 * it entirely, to avoid rtld_db dropping breakpoints in crucial
-		 * system daemons unless specifically demanded.  No death
-		 * notification is ever sent.
+		 * system daemons unless specifically demanded.  Also avoid
+		 * ptracing if the process is already being traced by someone
+		 * else (like another DTrace instance).  No death notification
+		 * is ever sent.
 		 *
 		 * Also, obviously enough, never drop breakpoints in ourself!
 		 */
 		if (datap->dpcd_flags & DTRACE_PROC_SHORTLIVED) {
-			noninvasive = 1;
+			pid_t tracer_pid;
+
+                        noninvasive = 1;
 			dpr->dpr_notifiable = 0;
-			if ((Psystem_daemon(dpr->dpr_pid, dtp->dt_useruid,
+			tracer_pid = Ptracer_pid(dpr->dpr_pid);
+
+                        if ((Psystem_daemon(dpr->dpr_pid, dtp->dt_useruid,
 				    dtp->dt_sysslice) > 0) ||
+			    ((tracer_pid != 0) &&
+			     (tracer_pid != getpid())) ||
 			    (dpr->dpr_pid == getpid()))
 				noninvasive = 2;
 		}
diff --git a/libproc/Pcontrol.c b/libproc/Pcontrol.c
index eb450bdc2981..68b372da62eb 100644
--- a/libproc/Pcontrol.c
+++ b/libproc/Pcontrol.c
@@ -2811,6 +2811,24 @@ Puid(pid_t pid)
 	return uid;
 }
 
+/*
+ * Determine if this process is already being ptraced, and if so, the pid of the
+ * ptracer.  The caller should eschew invasive tracing in this case.
+ */
+pid_t
+Ptracer_pid(pid_t pid)
+{
+	char *traced;
+	pid_t tracer_pid;
+
+	if ((traced = Pget_proc_status(pid, "TracerPid")) == NULL)
+		return 0;
+
+	tracer_pid = strtol(traced, NULL, 10);
+	free(traced);
+	return tracer_pid;
+}
+
 /*
  * Return 1 if this process is a system daemon, 0 if it is not, and -1 on error.
  *
diff --git a/libproc/libproc.h b/libproc/libproc.h
index 22ff54d58d3f..9e5c25990b4f 100644
--- a/libproc/libproc.h
+++ b/libproc/libproc.h
@@ -101,6 +101,7 @@ extern	void	Pset_procfs_path(const char *);
 extern	int	Pdynamically_linked(struct ps_prochandle *);
 extern	int	Ptraceable(struct ps_prochandle *);
 extern	int	Pelf64(struct ps_prochandle *);
+extern	pid_t	Ptracer_pid(pid_t);
 
 /*
  * Calls that do not take a process structure.  These are used to determine
-- 
2.42.0.271.g85384428f1