| Message ID | 20220729194106.1207472-1-song@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | dc81f8d1e8ea3f5dfa88919cb834a135a6a536b8 |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] bpf: fix test_progs -j error with fentry/fexit tests | expand |
On Fri, 29 Jul 2022 12:41:06 -0700 Song Liu <song@kernel.org> wrote: > Then multiple threads are attaching/detaching fentry/fexit programs to "When multiple threads"? -- Steve > the same trampoline, we may call register_fentry on the same trampoline > twice: register_fentry(), unregister_fentry(), then register_fentry again. > This causes ftrace_set_filter_ip() for the same ip on tr->fops twice, > which leaves duplicated ip in tr->fops. The extra ip is not cleaned up > properly on unregister and thus causes failures with further register in > register_ftrace_direct_multi(): > > register_ftrace_direct_multi() > { > ... > for (i = 0; i < size; i++) { > hlist_for_each_entry(entry, &hash->buckets[i], hlist) { > if (ftrace_find_rec_direct(entry->ip)) > goto out_unlock; > } > } > ... > } > > This can be triggered with parallel fentry/fexit tests with test_progs: > > ./test_progs -t fentry,fexit -j > > Fix this by resetting tr->fops in ftrace_set_filter_ip(), so that there > will never be duplicated entries in tr->fops. >
Hello: This patch was applied to bpf/bpf-next.git (master) by Andrii Nakryiko <andrii@kernel.org>: On Fri, 29 Jul 2022 12:41:06 -0700 you wrote: > Then multiple threads are attaching/detaching fentry/fexit programs to > the same trampoline, we may call register_fentry on the same trampoline > twice: register_fentry(), unregister_fentry(), then register_fentry again. > This causes ftrace_set_filter_ip() for the same ip on tr->fops twice, > which leaves duplicated ip in tr->fops. The extra ip is not cleaned up > properly on unregister and thus causes failures with further register in > register_ftrace_direct_multi(): > > [...] Here is the summary with links: - [bpf-next] bpf: fix test_progs -j error with fentry/fexit tests https://git.kernel.org/bpf/bpf-next/c/dc81f8d1e8ea You are awesome, thank you!
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c index 42e387a12694..7ec7e23559ad 100644 --- a/kernel/bpf/trampoline.c +++ b/kernel/bpf/trampoline.c @@ -255,7 +255,7 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr) return -ENOENT; if (tr->func.ftrace_managed) { - ftrace_set_filter_ip(tr->fops, (unsigned long)ip, 0, 0); + ftrace_set_filter_ip(tr->fops, (unsigned long)ip, 0, 1); ret = register_ftrace_direct_multi(tr->fops, (long)new_addr); } else { ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, NULL, new_addr);
Then multiple threads are attaching/detaching fentry/fexit programs to the same trampoline, we may call register_fentry on the same trampoline twice: register_fentry(), unregister_fentry(), then register_fentry again. This causes ftrace_set_filter_ip() for the same ip on tr->fops twice, which leaves duplicated ip in tr->fops. The extra ip is not cleaned up properly on unregister and thus causes failures with further register in register_ftrace_direct_multi(): register_ftrace_direct_multi() { ... for (i = 0; i < size; i++) { hlist_for_each_entry(entry, &hash->buckets[i], hlist) { if (ftrace_find_rec_direct(entry->ip)) goto out_unlock; } } ... } This can be triggered with parallel fentry/fexit tests with test_progs: ./test_progs -t fentry,fexit -j Fix this by resetting tr->fops in ftrace_set_filter_ip(), so that there will never be duplicated entries in tr->fops. Fixes: 00963a2e75a8 ("bpf: Support bpf_trampoline on functions with IPMODIFY (e.g. livepatch)") Reported-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: Song Liu <song@kernel.org> --- kernel/bpf/trampoline.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)