[DTrace-devel] [PATCH v2] error: ERROR probe firing should not corrupt probe arguments

[Kris Van Hees]

When an ERROR probe fires, it overwrites the first 6 probe arguments.
If other clauses are then executed for the original probe, the probe
arguments will have been corrupted.

Save arg0 through arg5 prior to calling the ERROR probe, and restore
them after the ERROR probe finishes.

Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
---
 bpf/probe_error.c                             |  2 +
 libdtrace/dt_cg.c                             |  4 +-
 libdtrace/dt_dctx.h                           |  5 +-
 .../error/tst.argv-corruption-by-error.d      | 48 +++++++++++++++++++
 4 files changed, 55 insertions(+), 4 deletions(-)
 create mode 100644 test/unittest/error/tst.argv-corruption-by-error.d

diff --git a/bpf/probe_error.c b/bpf/probe_error.c
index c8ddcdfa..cad161fd 100644
--- a/bpf/probe_error.c
+++ b/bpf/probe_error.c
@@ -27,6 +27,7 @@ noinline void dt_probe_error(const dt_dctx_t *dctx, uint64_t pc, uint64_t fault,
 {
 	dt_mstate_t	*mst = dctx->mst;
 
+	__builtin_memcpy(mst->saved_argv, mst->argv, sizeof(mst->saved_argv));
 	mst->argv[0] = 0;
 	mst->argv[1] = mst->epid;
 	mst->argv[2] = mst->clid;
@@ -36,5 +37,6 @@ noinline void dt_probe_error(const dt_dctx_t *dctx, uint64_t pc, uint64_t fault,
 
 	dt_error(dctx);
 
+	__builtin_memcpy(mst->argv, mst->saved_argv, sizeof(mst->saved_argv));
 	mst->fault = fault;
 }
diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 4fab8eed..166627fb 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -818,7 +818,7 @@ dt_cg_tramp_save_args(dt_pcb_t *pcb)
 
 	for (i = 0; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++) {
 		emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_7, DMST_ARG(i)));
-		emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_SAVED_ARG(i), BPF_REG_0));
+		emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ORIG_ARG(i), BPF_REG_0));
 	}
 }
 
@@ -832,7 +832,7 @@ dt_cg_tramp_restore_args(dt_pcb_t *pcb)
 	int		i;
 
 	for (i = 0; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++) {
-		emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_7, DMST_SAVED_ARG(i)));
+		emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_7, DMST_ORIG_ARG(i)));
 		emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(i), BPF_REG_0));
 	}
 }
diff --git a/libdtrace/dt_dctx.h b/libdtrace/dt_dctx.h
index 1422ad24..633c529f 100644
--- a/libdtrace/dt_dctx.h
+++ b/libdtrace/dt_dctx.h
@@ -30,7 +30,8 @@ typedef struct dt_mstate {
 	uint64_t	tstamp;		/* cached timestamp value */
 	dt_pt_regs	regs;		/* CPU registers */
 	uint64_t	argv[10];	/* Probe arguments */
-	uint64_t	saved_argv[10];	/* Saved probe arguments */
+	uint64_t	orig_argv[10];	/* Original (underlying) probe args */
+	uint64_t	saved_argv[6];	/* Saved arguments */
 } dt_mstate_t;
 
 #define DMST_EPID		offsetof(dt_mstate_t, epid)
@@ -45,7 +46,7 @@ typedef struct dt_mstate {
 #define DMST_TSTAMP		offsetof(dt_mstate_t, tstamp)
 #define DMST_REGS		offsetof(dt_mstate_t, regs)
 #define DMST_ARG(n)		offsetof(dt_mstate_t, argv[n])
-#define DMST_SAVED_ARG(n)	offsetof(dt_mstate_t, saved_argv[n])
+#define DMST_ORIG_ARG(n)	offsetof(dt_mstate_t, orig_argv[n])
 
 /*
  * The DTrace context.
diff --git a/test/unittest/error/tst.argv-corruption-by-error.d b/test/unittest/error/tst.argv-corruption-by-error.d
new file mode 100644
index 00000000..6fd1834e
--- /dev/null
+++ b/test/unittest/error/tst.argv-corruption-by-error.d
@@ -0,0 +1,48 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 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: A fault that triggers the ERROR probe terminates the execution of
+ *            the current clause, while other clauses for the same probe should
+ *            still be executed.  This tests that the ERROR probe invocation
+ *            does not corrupt the arguments of the original probe.
+ *
+ * SECTION: dtrace Provider
+ */
+
+#pragma D option quiet
+
+syscall::write*:entry
+{
+	self->arg0 = arg0;
+	self->arg1 = arg1;
+	self->arg2 = arg2;
+	self->arg3 = arg3;
+	self->arg4 = arg4;
+	self->arg5 = arg5;
+
+	printf("%d / %d / %d / %d / %d / %d\n",
+	       arg0, arg1, arg2, arg3, arg4, arg5);
+}
+
+syscall::write*:entry
+{
+	trace(*(int *)0);
+}
+
+syscall::write*:entry,
+ERROR {
+	printf("%d / %d / %d / %d / %d / %d\n",
+	       arg0, arg1, arg2, arg3, arg4, arg5);
+}
+
+syscall::write*:entry
+{
+	exit(self->arg0 != arg0 || self->arg1 != arg1 || self->arg2 != arg2 ||
+	     self->arg3 != arg3 || self->arg4 != arg4 || self->arg5 != arg5
+		? 1 : 0);
+}
-- 
2.45.2