Skip to contentSkip to conversation

bpf: fix test_progs -j error with fentry/fexit tests

PATCH Applied Song Liu 3 participants 3 messages Jul 29, 2022

Subsystems (3)
BPF BPF core Live patching
lore
Share & tools
lore

Song Liu

author maintainer
#0 lore raw

PATCH 1/1 bpf: fix test_progs -j error with fentry/fexit tests

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 <[email protected]>
Signed-off-by Song Liu <[email protected]>
---
 kernel/bpf/trampoline.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/bpf/trampoline.c +1 -1
index 42e387a12694..7ec7e23559ad 100644
@@ -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);
Signature · 1 line
2.30.2
#1 lore raw
Quoted 1 line · On Fri, 29 Jul 2022 12:41:06 -0700 Song Liu <[email protected]> wrote:

Then multiple threads are attaching/detaching fentry/fexit programs to

"When multiple threads"?

-- Steve

Quoted 26 lines

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.

CI Applied
#2 lore raw
In reply to Song Liu
This patch was applied to bpf/bpf-next.git (master) by Andrii Nakryiko <[email protected]>:

Hello:

This patch was applied to bpf/bpf-next.git (master) by Andrii Nakryiko <[email protected]>:

Quoted 9 lines · 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!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html