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

Reviewed-by: Eugene Loh <[email protected]>

On 8/11/25 16:22, Kris Van Hees via DTrace-devel wrote:
> As Eugene discovered, it was possible for dynamic variables (elements
> of associative arrays and TLS variables) to overwrite eachother.  The

eachother

> 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 <[email protected]>
> Signed-off-by: Eugene Loh <[email protected]>
> ---
>   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);
> +}