[DTrace-devel] [PATCH v3] dvar: ensure dynamic variables cannot overwrite eachother

[Kris Van Hees]

As Eugene discovered, it was possible for dynamic variables (elements
of associative arrays and TLS variables) to overwrite eachother.  The
problem turns out to be due to the implementation of the BPF helper
bpf_map_update_elem().  In order for the update to be seen as an atomic
operation, it does not update the value of the map element in-place but
instead allocates a new element and places it in front of the old one
before it removes the old one.  The result is that the address of the
map element changes as a result of the bpf_map_update_elem() call.

Fortunately, we can just assign the address of the map element in the
value that we obtained using bpf_map_lookup_elem() because that gives
us a pointer to the map value, and we can assign directly into it.

In other words, we do not need the 2nd bpf_map_update_elem() anyway,
and since that was the culprit, removing it resolves the issue.

Test included.

Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
---
 bpf/get_dvar.c                       |  5 +----
 test/unittest/assocs/tst.collision.d | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+), 4 deletions(-)
 create mode 100644 test/unittest/assocs/tst.collision.d

diff --git a/bpf/get_dvar.c b/bpf/get_dvar.c
index 1bb5eb002..073cca57c 100644
--- a/bpf/get_dvar.c
+++ b/bpf/get_dvar.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (c) 2019, 2024, Oracle and/or its affiliates.
+ * Copyright (c) 2019, 2025, Oracle and/or its affiliates.
  */
 #include <linux/bpf.h>
 #include <stdint.h>
@@ -150,9 +150,6 @@ noinline void *dt_get_assoc(uint32_t id, const char *tuple, uint64_t store,
 		if (valp == 0)
 			return dt_no_dvar();
 		*valp = (uint64_t)valp;
-		if (bpf_map_update_elem(&tuples, tuple, valp, BPF_ANY) < 0)
-			return dt_no_dvar();
-
 		val = *valp;
 	} else {
 		/*
diff --git a/test/unittest/assocs/tst.collision.d b/test/unittest/assocs/tst.collision.d
new file mode 100644
index 000000000..11a4363ca
--- /dev/null
+++ b/test/unittest/assocs/tst.collision.d
@@ -0,0 +1,22 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2025, 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: Dynamic variables do not overwrite eachother.
+ */
+
+BEGIN
+{
+	assoc[0x1234] = 1;
+	self->tls_assoc[1] = 0x1111;
+	self->tls_assoc[1] = 0;
+	self->tls_assoc[1] = 0x2222;
+
+	printf("%x / %x\n", assoc[0x1234], self->tls_assoc[1]);
+
+	exit(assoc[0x1234] == 1 && self->tls_assoc[1] == 0x2222 ? 0 : 1);
+}
-- 
2.45.2