[DTrace-devel] [PATCH 2/2] usdt: introduce DOF_VERSION_3 and use it when is-enabled probes are present

[Nick Alcock]

This guardrail prevents DTrace v2 from picking up v1-style is-enabled
probes, by suppressing is-enabled probes when DOF_VERSION <= 3, setting
the DOF_VERSION to 3 for programs containing is-enabled probes, and
refusing to -G-link object files containing DOF_VERSION_2 and is-enabled
probes. This is all necessary because v1-style probes have no function
arguments, so it would be dangerous to try to pick up the first
argument, treat it as a pointer, and write a 1 down it.

Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
---
 include/dtrace/dof_defines.h |  5 +++--
 libcommon/dof_parser.c       | 15 +++++++++++----
 libdtrace/dt_link.c          | 21 +++++++++++++++++----
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/include/dtrace/dof_defines.h b/include/dtrace/dof_defines.h
index 4a7c4185fa4b5..20ef88436b683 100644
--- a/include/dtrace/dof_defines.h
+++ b/include/dtrace/dof_defines.h
@@ -2,7 +2,7 @@
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * http://oss.oracle.com/licenses/upl.
  *
- * Copyright (c) 2009, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 2023, Oracle and/or its affiliates. All rights reserved.
  */
 
 /*
@@ -111,7 +111,8 @@
 
 #define DOF_VERSION_1	1
 #define DOF_VERSION_2	2
-#define DOF_VERSION	DOF_VERSION_2
+#define DOF_VERSION_3	3
+#define DOF_VERSION	DOF_VERSION_3
 
 #define DOF_FL_VALID	0	/* mask of all valid dofh_flags bits */
 
diff --git a/libcommon/dof_parser.c b/libcommon/dof_parser.c
index 52ad0263c4b1f..d6631a1afdcb9 100644
--- a/libcommon/dof_parser.c
+++ b/libcommon/dof_parser.c
@@ -1,6 +1,6 @@
 /*
  * Oracle Linux DTrace; DOF parser.
- * Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 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.
  */
@@ -403,7 +403,8 @@ dof_slurp(int out, dof_hdr_t *dof, uint64_t ubase)
 	}
 
 	if (dof->dofh_ident[DOF_ID_VERSION] != DOF_VERSION_1 &&
-	    dof->dofh_ident[DOF_ID_VERSION] != DOF_VERSION_2) {
+	    dof->dofh_ident[DOF_ID_VERSION] != DOF_VERSION_2 &&
+	    dof->dofh_ident[DOF_ID_VERSION] != DOF_VERSION_3) {
 		dof_error(out, EINVAL, "DOF version mismatch: %i",
 			  dof->dofh_ident[DOF_ID_VERSION]);
 		return -1;
@@ -990,9 +991,15 @@ emit_provider(int out, dof_helper_t *dhp,
 	enoff = NULL;
 
 	/*
-	 * See validate_probes().
+	 * See validate_probe().  Ignore is-enabled probes for DOF version 2:
+	 * these probes rely on modifying the return value of the is-enabled
+	 * probe, and using the modern approach of writing a 1 to the first
+	 * argument will write to a nonexistent argument and cause, at best, a
+	 * crash, at worst, memory corruption.  Avoiding parsing them ultimately
+	 * avoids creating the system-level probes at all, avoiding the
+	 * corruption at the cost of causing the probes to always return 0.
 	 */
-	if (dof->dofh_ident[DOF_ID_VERSION] != DOF_VERSION_1 &&
+	if (dof->dofh_ident[DOF_ID_VERSION] >= DOF_VERSION_3 &&
 	    prov->dofpv_prenoffs != DOF_SECT_NONE) {
 		enoff_sec = (dof_sec_t *)(uintptr_t)
 		  (daddr + dof->dofh_secoff +
diff --git a/libdtrace/dt_link.c b/libdtrace/dt_link.c
index e6059e9840cde..3346720fcf62e 100644
--- a/libdtrace/dt_link.c
+++ b/libdtrace/dt_link.c
@@ -1539,16 +1539,29 @@ dtrace_program_link(dtrace_hdl_t *dtp, dtrace_prog_t *pgp, uint_t dflags,
 		return 0;
 	}
 
-	for (i = 0; i < objc; i++)
+	for (i = 0; i < objc; i++) {
 		if (process_obj(dtp, objv[i], &eprobes) != 0)
 			return -1; /* errno is set for us */
 
+		/*
+		 * If there are is-enabled probes then we need to error out if
+		 * DOF version 2 was used, preventing linkage of pre-existing
+		 * objects that contain return-value-driven is-enabled probes
+		 * with a DTrace that implements argument-driven is-enabled
+		 * probes.  (New objects will at this stage have DOF version 1.)
+		 */
+		if (eprobes && pgp->dp_dofversion == DOF_VERSION_2)
+			return dt_link_error(dtp, NULL, -1, NULL,
+			    "DOF in %s is too old: regenerate with dtrace -G",
+			    objv[i]);
+	}
+
 	/*
 	 * If there are is-enabled probes then we need to force use of DOF
-	 * version 2.
+	 * version 3.
 	 */
-	if (eprobes && pgp->dp_dofversion < DOF_VERSION_2)
-		pgp->dp_dofversion = DOF_VERSION_2;
+	if (eprobes && pgp->dp_dofversion < DOF_VERSION_3)
+		pgp->dp_dofversion = DOF_VERSION_3;
 
 	if ((dof = dtrace_dof_create(dtp, pgp, dflags)) == NULL)
 		return -1; /* errno is set for us */
-- 
2.39.1.268.g9de2f9a303