diff options
author | Alexei Starovoitov <ast@kernel.org> | 2022-08-09 18:46:12 -0700 |
---|---|---|
committer | Alexei Starovoitov <ast@kernel.org> | 2022-08-09 18:46:12 -0700 |
commit | 46c8229c4317ba8576a206d285a34783390ba6ab (patch) | |
tree | 8b2a5de71b7b3f5cdbc9972a27724030ea1010d2 | |
parent | 19f68ed6dc90c93daf7e54d3350ea67fead7cbbf (diff) | |
parent | de7b9927105bd2afe940c6ad22de6938edd8b1c1 (diff) | |
download | lwn-46c8229c4317ba8576a206d285a34783390ba6ab.tar.gz lwn-46c8229c4317ba8576a206d285a34783390ba6ab.zip |
Merge branch 'Don't reinit map value in prealloc_lru_pop'
Kumar Kartikeya Dwivedi says:
====================
Fix for a bug in prealloc_lru_pop spotted while reading the code, then a test +
example that checks whether it is fixed.
Changelog:
----------
v2 -> v3:
v2: https://lore.kernel.org/bpf/20220809140615.21231-1-memxor@gmail.com
* Switch test to use kptr instead of kptr_ref to stabilize test runs
* Fix missing lru_bug__destroy (Yonghong)
* Collect Acks
v1 -> v2:
v1: https://lore.kernel.org/bpf/20220806014603.1771-1-memxor@gmail.com
* Expand commit log to include summary of the discussion with Yonghong
* Make lru_bug selftest serial to not mess up refcount for map_kptr test
====================
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
-rw-r--r-- | kernel/bpf/hashtab.c | 6 | ||||
-rw-r--r-- | net/bpf/test_run.c | 1 | ||||
-rw-r--r-- | tools/testing/selftests/bpf/prog_tests/lru_bug.c | 21 | ||||
-rw-r--r-- | tools/testing/selftests/bpf/progs/lru_bug.c | 49 |
4 files changed, 72 insertions, 5 deletions
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c index da7578426a46..4d793a92301b 100644 --- a/kernel/bpf/hashtab.c +++ b/kernel/bpf/hashtab.c @@ -311,12 +311,8 @@ static struct htab_elem *prealloc_lru_pop(struct bpf_htab *htab, void *key, struct htab_elem *l; if (node) { - u32 key_size = htab->map.key_size; - l = container_of(node, struct htab_elem, lru_node); - memcpy(l->key, key, key_size); - check_and_init_map_value(&htab->map, - l->key + round_up(key_size, 8)); + memcpy(l->key, key, htab->map.key_size); return l; } diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index cbc9cd5058cb..d11209367dd0 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -1628,6 +1628,7 @@ static int __init bpf_prog_test_run_init(void) int ret; ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &bpf_prog_test_kfunc_set); + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_prog_test_kfunc_set); return ret ?: register_btf_id_dtor_kfuncs(bpf_prog_test_dtor_kfunc, ARRAY_SIZE(bpf_prog_test_dtor_kfunc), THIS_MODULE); diff --git a/tools/testing/selftests/bpf/prog_tests/lru_bug.c b/tools/testing/selftests/bpf/prog_tests/lru_bug.c new file mode 100644 index 000000000000..3c7822390827 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/lru_bug.c @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <test_progs.h> + +#include "lru_bug.skel.h" + +void test_lru_bug(void) +{ + struct lru_bug *skel; + int ret; + + skel = lru_bug__open_and_load(); + if (!ASSERT_OK_PTR(skel, "lru_bug__open_and_load")) + return; + ret = lru_bug__attach(skel); + if (!ASSERT_OK(ret, "lru_bug__attach")) + goto end; + usleep(1); + ASSERT_OK(skel->data->result, "prealloc_lru_pop doesn't call check_and_init_map_value"); +end: + lru_bug__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/lru_bug.c b/tools/testing/selftests/bpf/progs/lru_bug.c new file mode 100644 index 000000000000..687081a724b3 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/lru_bug.c @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <vmlinux.h> +#include <bpf/bpf_tracing.h> +#include <bpf/bpf_helpers.h> + +struct map_value { + struct task_struct __kptr *ptr; +}; + +struct { + __uint(type, BPF_MAP_TYPE_LRU_HASH); + __uint(max_entries, 1); + __type(key, int); + __type(value, struct map_value); +} lru_map SEC(".maps"); + +int pid = 0; +int result = 1; + +SEC("fentry/bpf_ktime_get_ns") +int printk(void *ctx) +{ + struct map_value v = {}; + + if (pid == bpf_get_current_task_btf()->pid) + bpf_map_update_elem(&lru_map, &(int){0}, &v, 0); + return 0; +} + +SEC("fentry/do_nanosleep") +int nanosleep(void *ctx) +{ + struct map_value val = {}, *v; + struct task_struct *current; + + bpf_map_update_elem(&lru_map, &(int){0}, &val, 0); + v = bpf_map_lookup_elem(&lru_map, &(int){0}); + if (!v) + return 0; + bpf_map_delete_elem(&lru_map, &(int){0}); + current = bpf_get_current_task_btf(); + v->ptr = current; + pid = current->pid; + bpf_ktime_get_ns(); + result = !v->ptr; + return 0; +} + +char _license[] SEC("license") = "GPL"; |