[DTrace-devel] [PATCH 5/6] usdt: typed args and arg remapping

Nick Alcock nick.alcock at oracle.com
Fri Oct 18 19:58:07 UTC 2024


This change propagates the arg type and mapping info we just got into the
pid_probespec_t onward into the underlying dt_uprobe_t, shuffles the native
types before handing off a 1:1 mapping to the dt_probe_t, and calls
dt_cg_tramp_remap_args() to apply the requested remappings.  With this, USDT
probes should now have visible types which obey the :-notation translators
specified in their .d fils (this is now tested).

The probe info has been moved from the underlying probe to the overlying one
as part of this: underlying probes don't have any user-visible args to
get info for, and their probe_info functions are never called.

We make several assumptions for simplicity, all noted by TODOs in the
source:

 - overlying probes all have the same native types and remappings, so we
   don't need to look at more than one underlying probe to figure out what
   they are.  Given the structure of DOF it seems to me this must always be
   true, and even a malicious attacker could not produce DOF which said
   otherwise.

 - any given underlying probe locus is either a PID or a USDT probe, never
   both, so we can just remap the args at the start of the trampoline and
   then never touch them again.  This is obviously wrong, but the both-at
   the-same-locus case is not yet implemented, and significant changes need
   to be made to create_underlying() to fix this.  Right now we can't easily
   even tell if a given locus has both types of probe on it... fixing this
   is literally a matter of shuffling the pid probes after the usdt probes
   and then doing a single dt_cg_tramp_restore_args before all the pid
   probes fire.

   PID args are already broken before this commit (something to do with the
   wildcard USDT work?) so I cannot test if this commit broke them further,
   but it shouldn't have.

Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
---
 libdtrace/dt_prov_uprobe.c                    | 163 +++++++++++++++++-
 test/triggers/usdt-tst-argmap-prov.d          |   3 +-
 test/triggers/usdt-tst-argmap.c               |   3 +-
 .../dtrace-util/tst.ListProbesArgsUSDT.r      |  34 ++++
 .../dtrace-util/tst.ListProbesArgsUSDT.r.p    |   2 +
 .../dtrace-util/tst.ListProbesArgsUSDT.sh     |  83 +++++++++
 test/unittest/usdt/tst.argmap-typed.d         |  40 +++++
 test/unittest/usdt/tst.argmap.d               |   3 +-
 8 files changed, 320 insertions(+), 11 deletions(-)
 create mode 100644 test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r
 create mode 100755 test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r.p
 create mode 100755 test/unittest/dtrace-util/tst.ListProbesArgsUSDT.sh
 create mode 100644 test/unittest/usdt/tst.argmap-typed.d

diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
index 481159b01c24..767d9a80b5e1 100644
--- a/libdtrace/dt_prov_uprobe.c
+++ b/libdtrace/dt_prov_uprobe.c
@@ -55,7 +55,13 @@ typedef struct dt_uprobe {
 	uint64_t	off;
 	int		flags;
 	tp_probe_t	*tp;
-	dt_list_t	probes;		/* pid/USDT probes triggered by it */
+	int		nargc;		   /* number of native args */
+	int		xargc;		   /* number of xlated args */
+        int		remapargc;	   /* number of remapped args */
+	char		*nargv;		   /* array of native args */
+	char		*xargv;		   /* array of xlated args */
+	int8_t		*remapargs;	   /* remapped arg indexes */
+	dt_list_t	probes;		   /* pid/USDT probes triggered by it */
 } dt_uprobe_t;
 
 typedef struct list_probe {
@@ -112,6 +118,9 @@ static void probe_destroy_underlying(dtrace_hdl_t *dtp, void *datap)
 	dt_tp_destroy(dtp, tpp);
 	free_probe_list(dtp, dt_list_next(&upp->probes));
 	dt_free(dtp, upp->fn);
+	dt_free(dtp, upp->nargv);
+	dt_free(dtp, upp->xargv);
+	dt_free(dtp, upp->remapargs);
 	dt_free(dtp, upp);
 }
 
@@ -138,7 +147,7 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
 	char			prb[DTRACE_NAMELEN];
 	dtrace_probedesc_t	pd;
 	dt_probe_t		*uprp;
-	dt_uprobe_t		*upp;
+	dt_uprobe_t		*upp = NULL;
 	int			is_enabled = 0;
 
 	/*
@@ -201,7 +210,48 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
 		if (upp->tp == NULL)
 			goto fail;
 
-		uprp = dt_probe_insert(dtp, pvp, pd.prv, pd.mod, pd.fun, pd.prb,
+		/*
+		 * Arg info.  This really relates to the overlying probe (all
+		 * underlying probes associated with a given underlying probe
+		 * must have the same args), but the overlying probe doesn't
+		 * exist at this point so we must put it here and pull it out
+		 * when the overlying probe info is called for.
+		 *
+		 * TODO: is this true? can distinct underlying probes have
+		 * different native args, as long as their xlated types are the
+		 * same?
+		 */
+
+                if (psp->pps_nargv) {
+			upp->nargc = psp->pps_nargc;
+			upp->nargv = dt_alloc(dtp, psp->pps_nargvlen);
+			if(upp->nargv == NULL)
+				goto fail;
+			memcpy(upp->nargv, psp->pps_nargv, psp->pps_nargvlen);
+		}
+
+                if (psp->pps_xargv) {
+			upp->xargc = psp->pps_xargc;
+			upp->xargv = dt_alloc(dtp, psp->pps_xargvlen);
+			if(upp->xargv == NULL)
+				goto fail;
+			memcpy(upp->xargv, psp->pps_xargv, psp->pps_xargvlen);
+		}
+
+                if (psp->pps_remapargs) {
+			upp->remapargc = psp->pps_remapargc;
+			upp->remapargs = dt_alloc(dtp, psp->pps_remapargslen);
+			if(upp->remapargs == NULL)
+				goto fail;
+			memcpy(upp->remapargs, psp->pps_remapargs, psp->pps_remapargslen);
+		}
+
+		/*
+		 * TODO: validate that all underlying probes for a given
+		 * overlying probe have the same args.
+		 */
+
+                uprp = dt_probe_insert(dtp, pvp, pd.prv, pd.mod, pd.fun, pd.prb,
 				      upp);
 		if (uprp == NULL)
 			goto fail;
@@ -413,6 +463,9 @@ static void enable_usdt(dtrace_hdl_t *dtp, dt_probe_t *prp)
  *
  * The trampoline will first populate a dt_dctx_t struct.  It will then emulate
  * the firing of all dependent pid* probes and their clauses.
+ *
+ * uprobe trampolines will also arrange for argument shuffling if the remapping
+ * array calls for it.
  */
 static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
 {
@@ -437,6 +490,34 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
 		dt_cg_tramp_copy_args_from_regs(pcb,
 						!(upp->flags & PP_IS_FUNCALL));
 
+	/*
+	 * Figure out if we need to remap args, and remap them if needed.
+	 */
+	if (upp->remapargc) {
+		int	needs_remapping = 0;
+		size_t	i;
+
+                for (i = 0; i < upp->remapargc; i++) {
+			if (upp->remapargs[i] != i) {
+				needs_remapping = 1;
+				break;
+			}
+		}
+
+		/*
+		 * TODO: pid probes are never remapped, so a remapped set of
+		 * args for UDST probes need saving/restoring to reverse the
+		 * remapping for pid probes at the same locus -- but right now
+		 * pid and usdt probes hitting the same locus simply end up as
+		 * whichever registered first because create_underlying cannot
+		 * handle this case, so we can avoid it here too.
+		 * dt_cg_tramp_remap_args already saves regs: all we need to do
+		 * is restore them before any pid probe is hit.
+		 */
+		if (needs_remapping)
+			dt_cg_tramp_remap_args(pcb, upp->remapargs, upp->remapargc);
+	}
+
 	/*
 	 * Retrieve the PID of the process that caused the probe to fire.
 	 */
@@ -710,8 +791,77 @@ attach_bpf:
 static int probe_info(dtrace_hdl_t *dtp, const dt_probe_t *prp,
 		      int *argcp, dt_argdesc_t **argvp)
 {
-	*argcp = 0;			/* no known arguments */
-	*argvp = NULL;
+	size_t		i;
+	list_probe_t	*pup = prp->prv_data;
+	dt_uprobe_t	*upp;
+	char		*nptr;
+	char		*xptr;
+	int		argc = 0;
+	dt_argdesc_t	*argv = NULL;
+	char		**nargv = NULL;
+
+	/* No underlying probes?  No args.  */
+        if (!pup)
+		goto done;
+
+	upp = pup->probe->prv_data;
+	nptr = upp->nargv;
+	xptr = upp->xargv;
+
+        argc = upp->xargc;
+	if (argc == 0)
+		argc = upp->nargc;
+	if (argc == 0)
+		goto done;
+	if (!upp->nargv || upp->nargc == 0)
+		goto done;
+
+	/*
+	 * Construct an array to allow accessing native args by index, if we
+	 * need to (which we do whenever we have remapped args).  We don't need
+	 * to worry that any of the indexes might be too large, etc: this has
+	 * been checked already by validate_provider() before parsing and
+	 * writing to the DOF stash, and anyone who could inject invalid records
+	 * into the DOF stash is already root.
+	 *
+	 * Shuffle the native args according to the mapping, then set up a 1:1
+	 * mapping in its place: we do the remapping ourselves, driven directly
+	 * from upp->remapargs, in the trampoline, so array ops etc should not
+	 * do it.
+	 */
+	if (upp->xargv || upp->remapargs) {
+		if ((nargv = dt_zalloc(dtp, upp->nargc * sizeof(char *))) == NULL)
+			return dt_set_errno(dtp, ENOMEM);
+
+		for (i = 0; i < upp->nargc; i++, nptr += strlen(nptr) + 1)
+			nargv[i] = nptr;
+	}
+
+        argv = dt_zalloc(dtp, argc * sizeof(dt_argdesc_t));
+	if (argv == NULL) {
+		dt_free(dtp, nargv);
+		return dt_set_errno(dtp, EDT_NOMEM);
+	}
+
+	for (i = 0; i < argc; i++) {
+		int remap_arg = i;
+
+                if (upp->remapargc > i)
+			remap_arg = upp->remapargs[i];
+
+		if (xptr) {
+			argv[i].native = strdup(nargv[remap_arg]);
+			argv[i].xlate = strdup(xptr);
+		}
+		argv[i].mapping = i;
+
+		/* Guaranteed safe: see libcommon/dof_parser.c:strings_len() */
+                if (xptr)
+			xptr += strlen(xptr) + 1;
+	}
+done:
+	*argcp = argc;
+	*argvp = argv;
 
 	return 0;
 }
@@ -779,7 +929,6 @@ dt_provimpl_t	dt_uprobe = {
 	.load_prog	= &dt_bpf_prog_load,
 	.trampoline	= &trampoline,
 	.attach		= &attach,
-	.probe_info	= &probe_info,
 	.detach		= &detach,
 	.probe_destroy	= &probe_destroy_underlying,
 };
@@ -794,7 +943,6 @@ dt_provimpl_t	dt_uprobe_is_enabled = {
 	.load_prog	= &dt_bpf_prog_load,
 	.trampoline	= &trampoline_is_enabled,
 	.attach		= &attach,
-	.probe_info	= &probe_info,
 	.detach		= &detach,
 	.probe_destroy	= &probe_destroy_underlying,
 };
@@ -818,5 +966,6 @@ dt_provimpl_t	dt_usdt = {
 	.prog_type	= BPF_PROG_TYPE_UNSPEC,
 	.provide_probe	= &provide_usdt_probe,
 	.enable		= &enable_usdt,
+	.probe_info	= &probe_info,
 	.probe_destroy	= &probe_destroy,
 };
diff --git a/test/triggers/usdt-tst-argmap-prov.d b/test/triggers/usdt-tst-argmap-prov.d
index 28134baa6fa3..7cec525c908a 100644
--- a/test/triggers/usdt-tst-argmap-prov.d
+++ b/test/triggers/usdt-tst-argmap-prov.d
@@ -1,10 +1,11 @@
 /*
  * Oracle Linux DTrace.
- * Copyright (c) 2006, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2006, 2024, Oracle and/or its affiliates. All rights reserved.
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * http://oss.oracle.com/licenses/upl.
  */
 
 provider test_prov {
 	probe place(int i, int j) : (int j, int i, int i, int j);
+	probe place2(int i, char *j) : (char *j, int i, int i, char *j);
 };
diff --git a/test/triggers/usdt-tst-argmap.c b/test/triggers/usdt-tst-argmap.c
index 89a0a53fc1d5..cbdcaf1f0b61 100644
--- a/test/triggers/usdt-tst-argmap.c
+++ b/test/triggers/usdt-tst-argmap.c
@@ -1,6 +1,6 @@
 /*
  * Oracle Linux DTrace.
- * Copyright (c) 2006, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2006, 2024, Oracle and/or its affiliates. All rights reserved.
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * http://oss.oracle.com/licenses/upl.
  */
@@ -12,6 +12,7 @@ main(int argc, char **argv)
 {
 	for (;;) {
 		DTRACE_PROBE2(test_prov, place, 10, 4);
+		DTRACE_PROBE(test_prov, place2, 255, "foo");
 	}
 
 	return 0;
diff --git a/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r b/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r
new file mode 100644
index 000000000000..8bcfa92ff6b0
--- /dev/null
+++ b/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r
@@ -0,0 +1,34 @@
+   ID   PROVIDER            MODULE                          FUNCTION NAME
+ XX test_provXXXX              test                              main go
+
+	Probe Description Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Types
+		args[0]: char *
+		args[1]: int
+
+   ID   PROVIDER            MODULE                          FUNCTION NAME
+ XX test_provXXXX              test                              main go
+
+	Probe Description Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Types
+		args[0]: char *
+		args[1]: int
+
diff --git a/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r.p b/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r.p
new file mode 100755
index 000000000000..c575983adf65
--- /dev/null
+++ b/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r.p
@@ -0,0 +1,2 @@
+#!/bin/sed -rf
+s,test_prov[0-9]*,test_provXXXX,g; s,^ *[0-9]+, XX,g;
diff --git a/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.sh b/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.sh
new file mode 100755
index 000000000000..ad8dc2e7c267
--- /dev/null
+++ b/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.sh
@@ -0,0 +1,83 @@
+#!/bin/bash
+#
+# Oracle Linux DTrace.
+# Copyright (c) 2013, 2024, Oracle and/or its affiliates. All rights reserved.
+# Licensed under the Universal Permissive License v 1.0 as shown at
+# http://oss.oracle.com/licenses/upl.
+
+##
+#
+# ASSERTION:
+# Testing -lvn option with USDT probes with a valid probe name.
+#
+# SECTION: dtrace Utility/-ln Option
+#
+##
+
+if [ $# != 1 ]; then
+	echo expected one argument: '<'dtrace-path'>'
+	exit 2
+fi
+
+dtrace=$1
+CC=/usr/bin/gcc
+CFLAGS=
+
+DIRNAME="$tmpdir/list-probes-args-usdt.$$.$RANDOM"
+mkdir -p $DIRNAME
+cd $DIRNAME
+
+cat > prov.d <<EOF
+provider test_prov {
+	probe go(int a, char *b) : (char *b, int a);
+};
+EOF
+
+$dtrace -h -s prov.d
+if [ $? -ne 0 ]; then
+	echo "failed to generate header file" >& 2
+	exit 1
+fi
+
+cat > test.c <<EOF
+#include <sys/types.h>
+#include "prov.h"
+
+int
+main(int argc, char **argv)
+{
+	TEST_PROV_GO(666, "foo");
+	sleep(10);
+}
+EOF
+
+${CC} ${CFLAGS} -c test.c
+if [ $? -ne 0 ]; then
+	echo "failed to compile test.c" >& 2
+	exit 1
+fi
+$dtrace -G -s prov.d test.o
+if [ $? -ne 0 ]; then
+	echo "failed to create DOF" >& 2
+	exit 1
+fi
+${CC} ${CFLAGS} -o test test.o prov.o
+if [ $? -ne 0 ]; then
+	echo "failed to link final executable" >& 2
+	exit 1
+fi
+
+script()
+{
+	$dtrace -c ./test -lvn 'test_prov$target:::go'
+	./test &
+	PID=$!
+	disown %+
+	$dtrace -p $PID -lvn 'test_prov$target:::go'
+	kill -9 $PID
+}
+
+script
+status=$?
+
+exit $status
diff --git a/test/unittest/usdt/tst.argmap-typed.d b/test/unittest/usdt/tst.argmap-typed.d
new file mode 100644
index 000000000000..0e0c923e1736
--- /dev/null
+++ b/test/unittest/usdt/tst.argmap-typed.d
@@ -0,0 +1,40 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2007, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Licensed under the Universal Permissive License v 1.0 as shown at
+ * http://oss.oracle.com/licenses/upl.
+ */
+
+/* @@trigger: usdt-tst-argmap */
+/* @@trigger-timing: before */
+/* @@runtest-opts: $_pid */
+
+/*
+ * ASSERTION: Verify that args[N] variables are properly typed when remapped.
+ */
+
+BEGIN
+{
+	/* Timeout after 5 seconds */
+	timeout = timestamp + 5000000000;
+}
+
+test_prov$1:::place2
+/stringof(args[0]) != "foo" || args[1] != 255 || args[2] != 255 || stringof(args[3]) != "foo"/
+{
+	printf("args are %s, %d, %d, %s; should be \"foo\", 255, 255, \"foo\"",
+	    stringof(args[0]), args[1], args[2], stringof(args[3]));
+	exit(1);
+}
+
+test_prov$1:::place2
+{
+	exit(0);
+}
+
+profile:::tick-1
+/timestamp > timeout/
+{
+	trace("test timed out");
+	exit(1);
+}
diff --git a/test/unittest/usdt/tst.argmap.d b/test/unittest/usdt/tst.argmap.d
index 7896dc07e0e2..9cd3425bf8e0 100644
--- a/test/unittest/usdt/tst.argmap.d
+++ b/test/unittest/usdt/tst.argmap.d
@@ -1,11 +1,10 @@
 /*
  * Oracle Linux DTrace.
- * Copyright (c) 2007, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2007, 2024, Oracle and/or its affiliates. All rights reserved.
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * http://oss.oracle.com/licenses/upl.
  */
 
-/* @@xfail: dtv2 */
 /* @@trigger: usdt-tst-argmap */
 /* @@trigger-timing: before */
 /* @@runtest-opts: $_pid */
-- 
2.46.0.278.g36e3a12567




More information about the DTrace-devel mailing list