From: Alexei Starovoitov check_dup_ids() rejects equal BTF ids, but different ids can resolve to the same address. Aliases: ffffffff8161b6b0 t __do_sys_munlockall ffffffff8161b6b0 T __ia32_sys_munlockall ffffffff8161b6b0 T __x64_sys_munlockall or several BTF_KIND_FUNC with the same name that vmlinux BTF has when CONFIG_DEBUG_INFO_BTF_INLINE is set. Each id gets its own trampoline and register_fentry_multi() adds them to data->reg with the same ip. update_ftrace_direct_add() checks that new entries are not in direct_functions, but doesn't compare them with each other, so the attach succeeds. At detach update_ftrace_direct_del() sees that 'direct' of one of the entries doesn't match and returns -EINVAL: WARNING: kernel/bpf/trampoline.c:1845 at bpf_trampoline_multi_detach+0x24c/0x280 Call Trace: bpf_link_free+0x51/0x110 bpf_link_release+0x18/0x20 __fput+0x103/0x2d0 __x64_sys_close+0x3d/0x80 The images are freed anyway, while ftrace still calls them: Oops: int3: 0000 [#1] SMP NOPTI RIP: 0010:0xffffffffc042b201 Call Trace: ? __audit_syscall_exit+0x9/0x100 ? do_syscall_64+0x5ab/0x5e0 entry_SYSCALL_64_after_hwframe+0x77/0x7f Kernel panic - not syncing: Fatal exception in interrupt Return -EINVAL from register_fentry_multi() when ip is already in data->reg. When such ids are attached via different links the second attach already fails with -EINVAL in update_ftrace_direct_add(). Move the declaration of ftrace_lookup_ip() to include/linux/ftrace.h. The lookup adds 60 msec to the attach of 64480 functions in tracing_multi_bench_attach: 1.34 -> 1.40 sec. Fixes: aef4dfa790b2 ("bpf: Add bpf_trampoline_multi_attach/detach functions") Signed-off-by: Alexei Starovoitov --- include/linux/ftrace.h | 1 + kernel/bpf/trampoline.c | 8 ++++++++ kernel/trace/trace.h | 3 --- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index bd76a16a63af..b041d9142d2b 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -416,6 +416,7 @@ void free_ftrace_hash(struct ftrace_hash *hash); struct ftrace_func_entry *add_ftrace_hash_entry_direct(struct ftrace_hash *hash, unsigned long ip, unsigned long direct); void add_ftrace_hash_entry(struct ftrace_hash *hash, struct ftrace_func_entry *entry); +struct ftrace_func_entry *ftrace_lookup_ip(struct ftrace_hash *hash, unsigned long ip); void ftrace_hash_remove(struct ftrace_hash *hash); /* The hash used to know what functions callbacks trace */ diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c index fc94d6062109..24edb59238f3 100644 --- a/kernel/bpf/trampoline.c +++ b/kernel/bpf/trampoline.c @@ -1638,6 +1638,14 @@ static int register_fentry_multi(struct bpf_trampoline *tr, struct bpf_tramp_ima unsigned long ip = ftrace_location(tr->ip); struct bpf_tracing_multi_data *data = ptr; + /* + * Different BTF ids get different trampolines, but they can still + * resolve to the same address (aliases, functions of the same name). + * There's only one call site to attach to. + */ + if (ftrace_lookup_ip(data->reg, ip)) + return -EINVAL; + if (bpf_trampoline_use_jmp(tr->flags)) addr = ftrace_jmp_set(addr); diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h index 3749485a7d85..9044f03fe2d9 100644 --- a/kernel/trace/trace.h +++ b/kernel/trace/trace.h @@ -1006,9 +1006,6 @@ struct ftrace_hash { struct rcu_head rcu; }; -struct ftrace_func_entry * -ftrace_lookup_ip(struct ftrace_hash *hash, unsigned long ip); - static __always_inline bool ftrace_hash_empty(struct ftrace_hash *hash) { return !hash || !(hash->count || (hash->flags & FTRACE_HASH_FL_MOD)); -- 2.55.0