[DTrace-devel] [PATCH 11/14] map_value_or_null: add check for NULL pointer in load_var()

eugene.loh at oracle.com eugene.loh at oracle.com
Tue May 2 03:47:19 UTC 2023


From: Eugene Loh <eugene.loh at oracle.com>

When we look up a dynamic pointer (e.g., associative array or thread-local
variable), the BPF verifier sees that we get back a "map_value_or_null".
Ultimately, we will check the validity of the value.  If we first add an
offset to it, however, the BPF verifier objects.  This is a little strange,
since the verifier is fine adding an integer to 0 or to a map value.
Nevertheless, that's the situation.

Check the value.  If 0, assign 0.  This seems like it doesn't do anything,
but it allows the BPF verifier to branch and handle map_value_or_null as
two separate cases.

Add tests.  While this fix addresses dynamic variables (associative arrays
and thread-local variables), add tests for global and local variables also.

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
---
 libdtrace/dt_cg.c                             | 11 +++++++-
 .../codegen/err.str_NULL_plus_offset-assoc.d  | 25 ++++++++++++++++++
 .../codegen/err.str_NULL_plus_offset-assoc.r  |  3 +++
 .../codegen/err.str_NULL_plus_offset-lvar.d   | 25 ++++++++++++++++++
 .../codegen/err.str_NULL_plus_offset-lvar.r   |  3 +++
 .../codegen/err.str_NULL_plus_offset-tvar.d   | 25 ++++++++++++++++++
 .../codegen/err.str_NULL_plus_offset-tvar.r   |  3 +++
 .../codegen/err.str_NULL_plus_offset.d        | 26 +++++++++++++++++++
 .../codegen/err.str_NULL_plus_offset.r        |  3 +++
 .../unittest/codegen/tst.deref_string-assoc.d | 24 +++++++++++++++++
 .../unittest/codegen/tst.deref_string-assoc.r |  1 +
 test/unittest/codegen/tst.deref_string-gvar.d | 24 +++++++++++++++++
 test/unittest/codegen/tst.deref_string-gvar.r |  1 +
 test/unittest/codegen/tst.deref_string-lvar.d | 24 +++++++++++++++++
 test/unittest/codegen/tst.deref_string-lvar.r |  1 +
 test/unittest/codegen/tst.deref_string-tvar.d | 24 +++++++++++++++++
 test/unittest/codegen/tst.deref_string-tvar.r |  1 +
 17 files changed, 223 insertions(+), 1 deletion(-)
 create mode 100644 test/unittest/codegen/err.str_NULL_plus_offset-assoc.d
 create mode 100644 test/unittest/codegen/err.str_NULL_plus_offset-assoc.r
 create mode 100644 test/unittest/codegen/err.str_NULL_plus_offset-lvar.d
 create mode 100644 test/unittest/codegen/err.str_NULL_plus_offset-lvar.r
 create mode 100644 test/unittest/codegen/err.str_NULL_plus_offset-tvar.d
 create mode 100644 test/unittest/codegen/err.str_NULL_plus_offset-tvar.r
 create mode 100644 test/unittest/codegen/err.str_NULL_plus_offset.d
 create mode 100644 test/unittest/codegen/err.str_NULL_plus_offset.r
 create mode 100644 test/unittest/codegen/tst.deref_string-assoc.d
 create mode 100644 test/unittest/codegen/tst.deref_string-assoc.r
 create mode 100644 test/unittest/codegen/tst.deref_string-gvar.d
 create mode 100644 test/unittest/codegen/tst.deref_string-gvar.r
 create mode 100644 test/unittest/codegen/tst.deref_string-lvar.d
 create mode 100644 test/unittest/codegen/tst.deref_string-lvar.r
 create mode 100644 test/unittest/codegen/tst.deref_string-tvar.d
 create mode 100644 test/unittest/codegen/tst.deref_string-tvar.r

diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 7b98af5c..76614747 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -2566,7 +2566,16 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
 			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
 
 		if (dst->dn_flags & DT_NF_REF) {
-			emit(dlp,  BPF_MOV_REG(dst->dn_reg, BPF_REG_0));
+			uint_t	lbl_notnull = dt_irlist_label(dlp);
+			uint_t	lbl_done = dt_irlist_label(dlp);
+
+			emit(dlp,  BPF_BRANCH_IMM(BPF_JNE, BPF_REG_0, 0, lbl_notnull));
+			emit(dlp,  BPF_MOV_IMM(dst->dn_reg, 0));
+			emit(dlp,  BPF_JUMP(lbl_done));
+			emitl(dlp, lbl_notnull,
+				   BPF_MOV_REG(dst->dn_reg, BPF_REG_0));
+			emitl(dlp, lbl_done,
+				   BPF_NOP());
 		} else {
 			size_t	size = dt_node_type_size(dst);
 			uint_t	lbl_notnull = dt_irlist_label(dlp);
diff --git a/test/unittest/codegen/err.str_NULL_plus_offset-assoc.d b/test/unittest/codegen/err.str_NULL_plus_offset-assoc.d
new file mode 100644
index 00000000..20988c7d
--- /dev/null
+++ b/test/unittest/codegen/err.str_NULL_plus_offset-assoc.d
@@ -0,0 +1,25 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 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.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+	s[1234] = "hello world";
+	s[1234] = NULL;
+	printf("%x\n", s[1234][1]);
+}
+
+BEGIN
+{
+	exit(0);
+}
+
+ERROR
+{
+	exit(1);
+}
diff --git a/test/unittest/codegen/err.str_NULL_plus_offset-assoc.r b/test/unittest/codegen/err.str_NULL_plus_offset-assoc.r
new file mode 100644
index 00000000..187543b6
--- /dev/null
+++ b/test/unittest/codegen/err.str_NULL_plus_offset-assoc.r
@@ -0,0 +1,3 @@
+
+-- @@stderr --
+dtrace: error on enabled probe ID 3 (ID 1: dtrace:::BEGIN): invalid address ({ptr}) in action #1 at BPF pc NNN
diff --git a/test/unittest/codegen/err.str_NULL_plus_offset-lvar.d b/test/unittest/codegen/err.str_NULL_plus_offset-lvar.d
new file mode 100644
index 00000000..e529d611
--- /dev/null
+++ b/test/unittest/codegen/err.str_NULL_plus_offset-lvar.d
@@ -0,0 +1,25 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 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.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+	this->s = "hello world";
+	this->s = NULL;
+	printf("%x\n", this->s[1]);
+}
+
+BEGIN
+{
+	exit(0);
+}
+
+ERROR
+{
+	exit(1);
+}
diff --git a/test/unittest/codegen/err.str_NULL_plus_offset-lvar.r b/test/unittest/codegen/err.str_NULL_plus_offset-lvar.r
new file mode 100644
index 00000000..187543b6
--- /dev/null
+++ b/test/unittest/codegen/err.str_NULL_plus_offset-lvar.r
@@ -0,0 +1,3 @@
+
+-- @@stderr --
+dtrace: error on enabled probe ID 3 (ID 1: dtrace:::BEGIN): invalid address ({ptr}) in action #1 at BPF pc NNN
diff --git a/test/unittest/codegen/err.str_NULL_plus_offset-tvar.d b/test/unittest/codegen/err.str_NULL_plus_offset-tvar.d
new file mode 100644
index 00000000..898e895d
--- /dev/null
+++ b/test/unittest/codegen/err.str_NULL_plus_offset-tvar.d
@@ -0,0 +1,25 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 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.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+	self->s = "hello world";
+	self->s = NULL;
+	printf("%x\n", self->s[1]);
+}
+
+BEGIN
+{
+	exit(0);
+}
+
+ERROR
+{
+	exit(1);
+}
diff --git a/test/unittest/codegen/err.str_NULL_plus_offset-tvar.r b/test/unittest/codegen/err.str_NULL_plus_offset-tvar.r
new file mode 100644
index 00000000..187543b6
--- /dev/null
+++ b/test/unittest/codegen/err.str_NULL_plus_offset-tvar.r
@@ -0,0 +1,3 @@
+
+-- @@stderr --
+dtrace: error on enabled probe ID 3 (ID 1: dtrace:::BEGIN): invalid address ({ptr}) in action #1 at BPF pc NNN
diff --git a/test/unittest/codegen/err.str_NULL_plus_offset.d b/test/unittest/codegen/err.str_NULL_plus_offset.d
new file mode 100644
index 00000000..5a526428
--- /dev/null
+++ b/test/unittest/codegen/err.str_NULL_plus_offset.d
@@ -0,0 +1,26 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 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.
+ */
+
+#pragma D option quiet
+
+string s;
+
+BEGIN
+{
+	s = NULL;
+	printf("%x\n", s[1]);
+}
+
+BEGIN
+{
+	exit(0);
+}
+
+ERROR
+{
+	exit(1);
+}
diff --git a/test/unittest/codegen/err.str_NULL_plus_offset.r b/test/unittest/codegen/err.str_NULL_plus_offset.r
new file mode 100644
index 00000000..187543b6
--- /dev/null
+++ b/test/unittest/codegen/err.str_NULL_plus_offset.r
@@ -0,0 +1,3 @@
+
+-- @@stderr --
+dtrace: error on enabled probe ID 3 (ID 1: dtrace:::BEGIN): invalid address ({ptr}) in action #1 at BPF pc NNN
diff --git a/test/unittest/codegen/tst.deref_string-assoc.d b/test/unittest/codegen/tst.deref_string-assoc.d
new file mode 100644
index 00000000..ff7135dc
--- /dev/null
+++ b/test/unittest/codegen/tst.deref_string-assoc.d
@@ -0,0 +1,24 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 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.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+	s[1234] = "ABCDEFG";
+	trace(s[1234][1]);
+}
+
+BEGIN
+{
+	exit(0);
+}
+
+ERROR
+{
+	exit(1);
+}
diff --git a/test/unittest/codegen/tst.deref_string-assoc.r b/test/unittest/codegen/tst.deref_string-assoc.r
new file mode 100644
index 00000000..69a893aa
--- /dev/null
+++ b/test/unittest/codegen/tst.deref_string-assoc.r
@@ -0,0 +1 @@
+66
diff --git a/test/unittest/codegen/tst.deref_string-gvar.d b/test/unittest/codegen/tst.deref_string-gvar.d
new file mode 100644
index 00000000..15925510
--- /dev/null
+++ b/test/unittest/codegen/tst.deref_string-gvar.d
@@ -0,0 +1,24 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 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.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+	s = "ABCDEFG";
+	trace(s[1]);
+}
+
+BEGIN
+{
+	exit(0);
+}
+
+ERROR
+{
+	exit(1);
+}
diff --git a/test/unittest/codegen/tst.deref_string-gvar.r b/test/unittest/codegen/tst.deref_string-gvar.r
new file mode 100644
index 00000000..69a893aa
--- /dev/null
+++ b/test/unittest/codegen/tst.deref_string-gvar.r
@@ -0,0 +1 @@
+66
diff --git a/test/unittest/codegen/tst.deref_string-lvar.d b/test/unittest/codegen/tst.deref_string-lvar.d
new file mode 100644
index 00000000..eee6c806
--- /dev/null
+++ b/test/unittest/codegen/tst.deref_string-lvar.d
@@ -0,0 +1,24 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 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.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+	this->s = "ABCDEFG";
+	trace(this->s[1]);
+}
+
+BEGIN
+{
+	exit(0);
+}
+
+ERROR
+{
+	exit(1);
+}
diff --git a/test/unittest/codegen/tst.deref_string-lvar.r b/test/unittest/codegen/tst.deref_string-lvar.r
new file mode 100644
index 00000000..69a893aa
--- /dev/null
+++ b/test/unittest/codegen/tst.deref_string-lvar.r
@@ -0,0 +1 @@
+66
diff --git a/test/unittest/codegen/tst.deref_string-tvar.d b/test/unittest/codegen/tst.deref_string-tvar.d
new file mode 100644
index 00000000..9a697d48
--- /dev/null
+++ b/test/unittest/codegen/tst.deref_string-tvar.d
@@ -0,0 +1,24 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 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.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+	self->s = "ABCDEFG";
+	trace(self->s[1]);
+}
+
+BEGIN
+{
+	exit(0);
+}
+
+ERROR
+{
+	exit(1);
+}
diff --git a/test/unittest/codegen/tst.deref_string-tvar.r b/test/unittest/codegen/tst.deref_string-tvar.r
new file mode 100644
index 00000000..69a893aa
--- /dev/null
+++ b/test/unittest/codegen/tst.deref_string-tvar.r
@@ -0,0 +1 @@
+66
-- 
2.18.4




More information about the DTrace-devel mailing list