[DTrace-devel] [PATCH] cg, test: fix copyinstr() implementation to use a tstring

Kris Van Hees kris.van.hees at oracle.com
Thu Feb 16 21:31:31 UTC 2023


Since copyinstr() returns a string, it should be using a temporary
string like other string functions.  Also, semantics needed fixing to
apply strsize as an upper limit to the optional size rather than
triggering a fault if the size is greater than strsize.

Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
---
 libdtrace/dt_cg.c                             | 23 +++++++-------
 libdtrace/dt_open.c                           |  2 +-
 .../funcs/copyinstr/err.D_ALLOCA_SIZE.d       | 26 ----------------
 .../copyinstr/tst.copyinstr-high-maxsize.d    | 30 +++++++++++++++++++
 .../copyinstr/tst.copyinstr-high-maxsize.r    |  4 +++
 5 files changed, 47 insertions(+), 38 deletions(-)
 delete mode 100644 test/unittest/funcs/copyinstr/err.D_ALLOCA_SIZE.d
 create mode 100644 test/unittest/funcs/copyinstr/tst.copyinstr-high-maxsize.d
 create mode 100644 test/unittest/funcs/copyinstr/tst.copyinstr-high-maxsize.r

diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index cb284f07..8e767c2f 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -4517,7 +4517,7 @@ dt_cg_subr_copyinstr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
 	dt_node_t	*size = src->dn_list;
 	int		maxsize = dtp->dt_options[DTRACEOPT_STRSIZE];
 	uint_t		lbl_ok = dt_irlist_label(dlp);
-	uint_t		lbl_badsize = dt_irlist_label(dlp);
+	uint_t		lbl_oksize = dt_irlist_label(dlp);
 
 	TRACE_REGSET("    subr-copyinstr:Begin");
 
@@ -4529,15 +4529,19 @@ dt_cg_subr_copyinstr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
 
 	dt_cg_node(size, dlp, drp);
 
-	emit(dlp,  BPF_BRANCH_IMM(BPF_JSLT, size->dn_reg, 0, lbl_badsize));
-	emit(dlp,  BPF_BRANCH_IMM(BPF_JGT, size->dn_reg, maxsize, lbl_badsize));
+	emit(dlp,  BPF_BRANCH_IMM(BPF_JLE, size->dn_reg, maxsize, lbl_oksize));
+	emit(dlp,  BPF_MOV_IMM(size->dn_reg, maxsize));
+	emitl(dlp, lbl_oksize,
+		   BPF_NOP());
 
-	if ((dnp->dn_reg = dt_regset_alloc(drp)) == -1)
+	/* Allocate a temporary string for the destination (return value). */
+	dnp->dn_reg = dt_regset_alloc(drp);
+	if (dnp->dn_reg == -1)
 		longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
-
-	/* Allocate scratch space. */
-	dt_cg_subr_alloca_impl(dnp, size, dlp, drp);
-	dt_cg_alloca_ptr(dlp, drp, dnp->dn_reg, dnp->dn_reg);
+	dt_cg_tstring_alloc(yypcb, dnp);
+	emit(dlp,  BPF_LOAD(BPF_DW, dnp->dn_reg, BPF_REG_FP, DT_STK_DCTX));
+	emit(dlp,  BPF_LOAD(BPF_DW, dnp->dn_reg, dnp->dn_reg, DCTX_MEM));
+	emit(dlp,  BPF_ALU64_IMM(BPF_ADD, dnp->dn_reg, dnp->dn_tstring->dn_value));
 
 	dt_cg_node(src, dlp, drp);
 
@@ -4552,9 +4556,6 @@ dt_cg_subr_copyinstr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
 	dt_regset_free_args(drp);
 	emit(dlp,  BPF_BRANCH_IMM(BPF_JSGT, BPF_REG_0, 0, lbl_ok));
 	dt_cg_probe_error(yypcb, DTRACEFLT_BADADDR, DT_ISREG, src->dn_reg);
-	emitl(dlp, lbl_badsize,
-		   BPF_NOP());
-	dt_cg_probe_error(yypcb, DTRACEFLT_BADSIZE, DT_ISREG, size->dn_reg);
 	emitl(dlp, lbl_ok,
 		   BPF_NOP());
 	dt_regset_free(drp, BPF_REG_0);
diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
index a3754a32..8cd861e9 100644
--- a/libdtrace/dt_open.c
+++ b/libdtrace/dt_open.c
@@ -131,7 +131,7 @@ static const dt_ident_t _dtrace_globals[] = {
 	&dt_idops_func, "void(int)" },
 { "copyin", DT_IDENT_FUNC, 0, DIF_SUBR_COPYIN, DT_ATTR_STABCMN, DT_VERS_1_0,
 	&dt_idops_func, "void *(uintptr_t, size_t)" },
-{ "copyinstr", DT_IDENT_FUNC, 0, DIF_SUBR_COPYINSTR,
+{ "copyinstr", DT_IDENT_FUNC, DT_IDFLG_DPTR, DIF_SUBR_COPYINSTR,
 	DT_ATTR_STABCMN, DT_VERS_1_0,
 	&dt_idops_func, "string(uintptr_t, [size_t])" },
 { "copyinto", DT_IDENT_FUNC, 0, DIF_SUBR_COPYINTO, DT_ATTR_STABCMN,
diff --git a/test/unittest/funcs/copyinstr/err.D_ALLOCA_SIZE.d b/test/unittest/funcs/copyinstr/err.D_ALLOCA_SIZE.d
deleted file mode 100644
index c9f25721..00000000
--- a/test/unittest/funcs/copyinstr/err.D_ALLOCA_SIZE.d
+++ /dev/null
@@ -1,26 +0,0 @@
-/*
- * Oracle Linux DTrace.
- * Copyright (c) 2022, 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: The copyinstr() size must not exceed the (adjusted) scratchsize.
- *
- * SECTION: Actions and Subroutines/copyinstr()
- *	    User Process Tracing/copyin() and copyinstr()
- */
-
-#pragma D option scratchsize=64
-
-BEGIN
-{
-	copyinstr(0, 65);
-	exit(0);
-}
-
-ERROR
-{
-	exit(1);
-}
diff --git a/test/unittest/funcs/copyinstr/tst.copyinstr-high-maxsize.d b/test/unittest/funcs/copyinstr/tst.copyinstr-high-maxsize.d
new file mode 100644
index 00000000..7a341640
--- /dev/null
+++ b/test/unittest/funcs/copyinstr/tst.copyinstr-high-maxsize.d
@@ -0,0 +1,30 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2006, 2023, 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: It is possible to read a string from userspace addresses.
+ *
+ * SECTION: Actions and Subroutines/copyinstr()
+ *	    User Process Tracing/copyin() and copyinstr() Subroutines
+ */
+/* @@trigger: delaydie */
+
+#pragma D option quiet
+#pragma D option strsize=12
+
+syscall::write:entry
+/pid == $target/
+{
+	printf("%s char match\n", (s = copyinstr(arg1, 96))[4] == 'y' ? "good" : "BAD");
+	printf("'%s'", s);
+	exit(0);
+}
+
+ERROR
+{
+	exit(1);
+}
diff --git a/test/unittest/funcs/copyinstr/tst.copyinstr-high-maxsize.r b/test/unittest/funcs/copyinstr/tst.copyinstr-high-maxsize.r
new file mode 100644
index 00000000..e6d97c74
--- /dev/null
+++ b/test/unittest/funcs/copyinstr/tst.copyinstr-high-maxsize.r
@@ -0,0 +1,4 @@
+good char match
+'Delay in ns'
+-- @@stderr --
+Delay in ns needed in delay env var.
-- 
2.39.1




More information about the DTrace-devel mailing list