When attaching a kprobe to a function that has duplicate symbols in kallsyms (e.g. same name in vmlinux and a module), the kernel returns EADDRNOTAVAIL (or EINVAL via the legacy path). Detect this and retry by resolving the vmlinux address through kallsyms, skipping module symbols, and using the absolute address for probe creation. Extend libbpf_kallsyms_parse() to support module name extraction and add a skip_if_module flag so callers can filter module symbols at the parse level without changing callback signatures. Signed-off-by: Andrey Grodzovsky --- tools/lib/bpf/libbpf.c | 153 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 135 insertions(+), 18 deletions(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 0be7017800fe..5907ab09c4dc 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -8460,9 +8460,11 @@ static int bpf_object__sanitize_maps(struct bpf_object *obj) typedef int (*kallsyms_cb_t)(unsigned long long sym_addr, char sym_type, const char *sym_name, void *ctx); -static int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *ctx) +static int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *ctx, + bool skip_if_module) { char sym_type, sym_name[500]; + char module_name[128]; unsigned long long sym_addr; int ret, err = 0; FILE *f; @@ -8475,21 +8477,39 @@ static int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *ctx) } while (true) { - ret = fscanf(f, "%llx %c %499s%*[^\n]\n", - &sym_addr, &sym_type, sym_name); - if (ret == EOF && feof(f)) + int n = 0; + + /* + * Format tries to match both vmlinux and module kallsyms + * entries in one call. Module entries have the form: + * addr type name [module] + * %n fires only if the closing ']' literal is matched. + */ + ret = fscanf(f, "%llx %c %499s [%127[^] \t\n\r\v\f]] %n", + &sym_addr, &sym_type, sym_name, module_name, &n); + + /* EOF or malformed line - handle after the loop */ + if (ret != 3 && ret != 4) break; - if (ret != 3) { - pr_warn("failed to read kallsyms entry: %d\n", ret); - err = -EINVAL; + + /* ret == 4 && n == 0: scanset matched but ']' didn't */ + if (ret == 4 && n == 0) break; - } + + if (ret == 4 && skip_if_module) + continue; err = cb(sym_addr, sym_type, sym_name, ctx); if (err) break; } + /* !err: don't overwrite callback errors with -EINVAL */ + if (!feof(f) && !err) { + pr_warn("failed to read kallsyms entry: ret=%d\n", ret); + err = -EINVAL; + } + fclose(f); return err; } @@ -8529,7 +8549,7 @@ static int kallsyms_cb(unsigned long long sym_addr, char sym_type, static int bpf_object__read_kallsyms_file(struct bpf_object *obj) { - return libbpf_kallsyms_parse(kallsyms_cb, obj); + return libbpf_kallsyms_parse(kallsyms_cb, obj, false); } static int find_ksym_btf_id(struct bpf_object *obj, const char *ksym_name, @@ -11527,10 +11547,20 @@ static void gen_probe_legacy_event_name(char *buf, size_t buf_sz, static int add_kprobe_event_legacy(const char *probe_name, bool retprobe, const char *kfunc_name, size_t offset) { - return append_to_file(tracefs_kprobe_events(), "%c:%s/%s %s+0x%zx", - retprobe ? 'r' : 'p', - retprobe ? "kretprobes" : "kprobes", - probe_name, kfunc_name, offset); + /* When kfunc_name is NULL, use absolute address format (0xADDR), + * otherwise use symbol+offset format (SYMBOL+0xOFF) + */ + if (kfunc_name) { + return append_to_file(tracefs_kprobe_events(), "%c:%s/%s %s+0x%zx", + retprobe ? 'r' : 'p', + retprobe ? "kretprobes" : "kprobes", + probe_name, kfunc_name, offset); + } else { + return append_to_file(tracefs_kprobe_events(), "%c:%s/%s 0x%zx", + retprobe ? 'r' : 'p', + retprobe ? "kretprobes" : "kprobes", + probe_name, offset); + } } static int remove_kprobe_event_legacy(const char *probe_name, bool retprobe) @@ -11559,7 +11589,7 @@ static int perf_event_kprobe_open_legacy(const char *probe_name, bool retprobe, err = add_kprobe_event_legacy(probe_name, retprobe, kfunc_name, offset); if (err < 0) { pr_warn("failed to add legacy kprobe event for '%s+0x%zx': %s\n", - kfunc_name, offset, + kfunc_name ? kfunc_name : "(abs_addr)", offset, errstr(err)); return err; } @@ -11567,7 +11597,7 @@ static int perf_event_kprobe_open_legacy(const char *probe_name, bool retprobe, if (type < 0) { err = type; pr_warn("failed to determine legacy kprobe event id for '%s+0x%zx': %s\n", - kfunc_name, offset, + kfunc_name ? kfunc_name : "(abs_addr)", offset, errstr(err)); goto err_clean_legacy; } @@ -11651,6 +11681,43 @@ int probe_kern_syscall_wrapper(int token_fd) } } +/* Context structure for finding vmlinux kernel symbol address */ +struct find_kaddr_ctx { + const char *func_name; + unsigned long long kaddr; +}; + +/* Callback to find vmlinux kernel text symbol address */ +static int find_kaddr_cb(unsigned long long sym_addr, char sym_type, + const char *sym_name, void *ctx) +{ + struct find_kaddr_ctx *data = ctx; + + /* Only match text section symbols ('T' or 't') */ + if (toupper(sym_type) != 'T') + return 0; + + /* Check if this is the symbol we're looking for */ + if (strcmp(sym_name, data->func_name) == 0) { + + /* If we already have an address, we've encountered a + * duplicate symbol name. Reject such symbols to avoid + * ambiguous resolution. + */ + if (data->kaddr && data->kaddr != sym_addr) { + pr_warn("kernel symbol '%s': resolution is ambiguous: 0x%llx or 0x%llx\n", + sym_name, data->kaddr, sym_addr); + return -EINVAL; + } + + data->kaddr = sym_addr; + pr_debug("found kernel symbol %s at address 0x%llx\n", + sym_name, data->kaddr); + } + + return 0; +} + struct bpf_link * bpf_program__attach_kprobe_opts(const struct bpf_program *prog, const char *func_name, @@ -11663,6 +11730,8 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog, size_t offset; bool retprobe, legacy; int pfd, err; + const char *optional_func_name = func_name; + struct find_kaddr_ctx kaddr_ctx = { .kaddr = 0 }; if (!OPTS_VALID(opts, bpf_kprobe_opts)) return libbpf_err_ptr(-EINVAL); @@ -11693,9 +11762,10 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog, return libbpf_err_ptr(-EINVAL); } +retry_create: if (!legacy) { pfd = perf_event_open_probe(false /* uprobe */, retprobe, - func_name, offset, + optional_func_name, offset, -1 /* pid */, 0 /* ref_ctr_off */); } else { char probe_name[MAX_EVENT_NAME_LEN]; @@ -11707,7 +11777,7 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog, if (!legacy_probe) return libbpf_err_ptr(-ENOMEM); - pfd = perf_event_kprobe_open_legacy(legacy_probe, retprobe, func_name, + pfd = perf_event_kprobe_open_legacy(legacy_probe, retprobe, optional_func_name, offset, -1 /* pid */); } if (pfd < 0) { @@ -11716,6 +11786,53 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog, prog->name, retprobe ? "kretprobe" : "kprobe", func_name, offset, errstr(err)); + + /* + * If attachment fails with EADDRNOTAVAIL (or EINVAL in + * legacy mode) and we used non-NULL func_name, try to + * find function address in /proc/kallsyms and retry + * using KADDR instead of ambiguous symbol. Legacy + * tracefs API converts EADDRNOTAVAIL to EINVAL, so + * we need to check for both. + */ + if ((err == -EADDRNOTAVAIL || (legacy && err == -EINVAL)) && optional_func_name) { + pr_debug("kallsyms lookup for %s\n", + func_name); + + kaddr_ctx.func_name = func_name; + kaddr_ctx.kaddr = 0; + + /* + * Drop the function symbol and override the + * offset to be the desired function KADDR. + * This avoids kallsyms validation for duplicate + * symbols in the kernel. See attr.config2 in + * perf_event_open_probe and kernel code in + * perf_kprobe_init/create_local_trace_kprobe. + */ + optional_func_name = NULL; + + err = libbpf_kallsyms_parse(find_kaddr_cb, &kaddr_ctx, true); + if (err) { + pr_warn("failed to get addr for %s\n", + func_name); + goto err_out; + } + + if (kaddr_ctx.kaddr) { + pr_debug("retrying %s using kaddr 0x%llx\n", + func_name, kaddr_ctx.kaddr); + + offset += kaddr_ctx.kaddr; + free(legacy_probe); + legacy_probe = NULL; + goto retry_create; + } + + pr_warn("symbol '%s' not found in vmlinux\n", + func_name); + } + goto err_out; } link = bpf_program__attach_perf_event_opts(prog, pfd, &pe_opts); @@ -11931,7 +12048,7 @@ static int libbpf_available_kallsyms_parse(struct kprobe_multi_resolve *res) data.syms = syms; data.res = res; data.cnt = cnt; - libbpf_kallsyms_parse(avail_kallsyms_cb, &data); + libbpf_kallsyms_parse(avail_kallsyms_cb, &data, false); if (res->cnt == 0) err = -ENOENT; -- 2.34.1 Add test_attach_probe_dup_sym() which loads bpf_testmod_dup_sym.ko to create a duplicate symbol scenario, then verifies kprobe and kretprobe attachment succeeds across default, legacy, perf_event, and link modes. Signed-off-by: Andrey Grodzovsky --- tools/testing/selftests/bpf/Makefile | 2 +- .../selftests/bpf/prog_tests/attach_probe.c | 63 +++++++++++++++++++ .../testing/selftests/bpf/test_kmods/Makefile | 2 +- .../bpf/test_kmods/bpf_testmod_dup_sym.c | 50 +++++++++++++++ 4 files changed, 115 insertions(+), 2 deletions(-) create mode 100644 tools/testing/selftests/bpf/test_kmods/bpf_testmod_dup_sym.c diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 6776158f1f3e..fe9671268c6f 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -117,7 +117,7 @@ TEST_PROGS_EXTENDED := \ test_bpftool.py TEST_KMODS := bpf_testmod.ko bpf_test_no_cfi.ko bpf_test_modorder_x.ko \ - bpf_test_modorder_y.ko bpf_test_rqspinlock.ko + bpf_test_modorder_y.ko bpf_test_rqspinlock.ko bpf_testmod_dup_sym.ko TEST_KMOD_TARGETS = $(addprefix $(OUTPUT)/,$(TEST_KMODS)) # Compile but not part of 'make run_tests' diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c index 9e77e5da7097..75c22af2f1b3 100644 --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c @@ -4,6 +4,7 @@ #include "test_attach_probe_manual.skel.h" #include "test_attach_probe.skel.h" #include "kprobe_write_ctx.skel.h" +#include "testing_helpers.h" /* this is how USDT semaphore is actually defined, except volatile modifier */ volatile unsigned short uprobe_ref_ctr __attribute__((unused)) __attribute((section(".probes"))); @@ -123,6 +124,59 @@ static void test_attach_probe_manual(enum probe_attach_mode attach_mode) test_attach_probe_manual__destroy(skel); } +/* Test kprobe attachment with duplicate symbols. + * This test loads bpf_testmod_dup_sym.ko which creates a duplicate + * __x64_sys_nanosleep symbol. The libbpf fix should prefer the vmlinux + * symbol over the module symbol when attaching kprobes. + */ +static void test_attach_probe_dup_sym(enum probe_attach_mode attach_mode) +{ + DECLARE_LIBBPF_OPTS(bpf_kprobe_opts, kprobe_opts); + struct bpf_link *kprobe_link, *kretprobe_link; + struct test_attach_probe_manual *skel; + int err; + + /* Load module with duplicate symbol */ + err = load_module("bpf_testmod_dup_sym.ko", false); + if (!ASSERT_OK(err, "load_bpf_testmod_dup_sym")) { + test__skip(); + return; + } + + skel = test_attach_probe_manual__open_and_load(); + if (!ASSERT_OK_PTR(skel, "skel_dup_sym_open_and_load")) + goto unload_module; + + /* manual-attach kprobe/kretprobe with duplicate symbol present */ + kprobe_opts.attach_mode = attach_mode; + kprobe_opts.retprobe = false; + kprobe_link = bpf_program__attach_kprobe_opts(skel->progs.handle_kprobe, + SYS_NANOSLEEP_KPROBE_NAME, + &kprobe_opts); + if (!ASSERT_OK_PTR(kprobe_link, "attach_kprobe_dup_sym")) + goto cleanup; + skel->links.handle_kprobe = kprobe_link; + + kprobe_opts.retprobe = true; + kretprobe_link = bpf_program__attach_kprobe_opts(skel->progs.handle_kretprobe, + SYS_NANOSLEEP_KPROBE_NAME, + &kprobe_opts); + if (!ASSERT_OK_PTR(kretprobe_link, "attach_kretprobe_dup_sym")) + goto cleanup; + skel->links.handle_kretprobe = kretprobe_link; + + /* trigger & validate kprobe && kretprobe */ + usleep(1); + + ASSERT_EQ(skel->bss->kprobe_res, 1, "check_kprobe_dup_sym_res"); + ASSERT_EQ(skel->bss->kretprobe_res, 2, "check_kretprobe_dup_sym_res"); + +cleanup: + test_attach_probe_manual__destroy(skel); +unload_module: + unload_module("bpf_testmod_dup_sym", false); +} + /* attach uprobe/uretprobe long event name testings */ static void test_attach_uprobe_long_event_name(void) { @@ -417,6 +471,15 @@ void test_attach_probe(void) if (test__start_subtest("manual-link")) test_attach_probe_manual(PROBE_ATTACH_MODE_LINK); + if (test__start_subtest("dup-sym-default")) + test_attach_probe_dup_sym(PROBE_ATTACH_MODE_DEFAULT); + if (test__start_subtest("dup-sym-legacy")) + test_attach_probe_dup_sym(PROBE_ATTACH_MODE_LEGACY); + if (test__start_subtest("dup-sym-perf")) + test_attach_probe_dup_sym(PROBE_ATTACH_MODE_PERF); + if (test__start_subtest("dup-sym-link")) + test_attach_probe_dup_sym(PROBE_ATTACH_MODE_LINK); + if (test__start_subtest("auto")) test_attach_probe_auto(skel); if (test__start_subtest("kprobe-sleepable")) diff --git a/tools/testing/selftests/bpf/test_kmods/Makefile b/tools/testing/selftests/bpf/test_kmods/Makefile index 63c4d3f6a12f..938c462a103b 100644 --- a/tools/testing/selftests/bpf/test_kmods/Makefile +++ b/tools/testing/selftests/bpf/test_kmods/Makefile @@ -8,7 +8,7 @@ Q = @ endif MODULES = bpf_testmod.ko bpf_test_no_cfi.ko bpf_test_modorder_x.ko \ - bpf_test_modorder_y.ko bpf_test_rqspinlock.ko + bpf_test_modorder_y.ko bpf_test_rqspinlock.ko bpf_testmod_dup_sym.ko $(foreach m,$(MODULES),$(eval obj-m += $(m:.ko=.o))) diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_dup_sym.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_dup_sym.c new file mode 100644 index 000000000000..c6a5a32799e9 --- /dev/null +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_dup_sym.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2025 CrowdStrike */ +/* Test module for duplicate kprobe symbol handling */ +#include +#include +#include + +/* Duplicate symbol to test kprobe attachment with duplicate symbols. + * This creates a duplicate of the syscall wrapper used in attach_probe tests. + * The libbpf fix should handle this by preferring the vmlinux symbol. + * This function should NEVER be called - kprobes should attach to vmlinux version. + */ +#ifdef __x86_64__ +int __x64_sys_nanosleep(void); +noinline int __x64_sys_nanosleep(void) +#elif defined(__s390x__) +int __s390x_sys_nanosleep(void); +noinline int __s390x_sys_nanosleep(void) +#elif defined(__aarch64__) +int __arm64_sys_nanosleep(void); +noinline int __arm64_sys_nanosleep(void) +#elif defined(__riscv) +int __riscv_sys_nanosleep(void); +noinline int __riscv_sys_nanosleep(void) +#else +int sys_nanosleep(void); +noinline int sys_nanosleep(void) +#endif +{ + WARN_ONCE(1, "bpf_testmod_dup_sym: dummy nanosleep symbol called - this should never execute!\n"); + return -EINVAL; +} + +static int __init bpf_testmod_dup_sym_init(void) +{ + pr_info("bpf_testmod_dup_sym: loaded (duplicate symbol test module)\n"); + return 0; +} + +static void __exit bpf_testmod_dup_sym_exit(void) +{ + pr_info("bpf_testmod_dup_sym: unloaded\n"); +} + +module_init(bpf_testmod_dup_sym_init); +module_exit(bpf_testmod_dup_sym_exit); + +MODULE_AUTHOR("Andrey Grodzovsky"); +MODULE_DESCRIPTION("BPF selftest duplicate symbol module"); +MODULE_LICENSE("GPL"); -- 2.34.1