[PATCH v3] dvar: ensure dynamic variables cannot overwrite eachother
Kris Van Hees <[email protected]>
| Newsgroups | dev.linux.lists.dtrace |
|---|---|
| Message-ID | <SJ0PR10MB567287FE5AFCB29C1950F1A6C228A@SJ0PR10MB5672.namprd10.prod.outlook.com> |
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 <[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); +} -- 2.45.2