Add fallback to handle EADDRNOTAVAIL by retrying with absolute kernel addresses from kallsyms when symbol names are ambiguous. When kprobe attachment fails with EADDRNOTAVAIL (or EINVAL in legacy mode) and a symbol name was used, the kernel encountered duplicate symbols in kallsyms. This patch: - Adds module_name parameter to kallsyms_cb_t callback and parser to distinguish vmlinux from module symbols - Implements find_kaddr_cb() to look up symbol addresses in kallsyms, rejecting ambiguous symbol names - Modifies add_kprobe_event_legacy() to support absolute address format - Implements retry logic in attach_kprobe_opts() that falls back to address-based attachment on failure - Updates avail_kallsyms_cb() callback signature Signed-off-by: Andrey Grodzovsky --- tools/lib/bpf/libbpf.c | 150 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 133 insertions(+), 17 deletions(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 0c8bf0b5cce4..684781d25859 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -8449,11 +8449,12 @@ 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); + const char *sym_name, const char *module_name, void *ctx); static int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *ctx) { char sym_type, sym_name[500]; + char module_name[128]; unsigned long long sym_addr; int ret, err = 0; FILE *f; @@ -8465,18 +8466,39 @@ static int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *ctx) return err; } - while (true) { - ret = fscanf(f, "%llx %c %499s%*[^\n]\n", - &sym_addr, &sym_type, sym_name); - if (ret == EOF && feof(f)) + while (!feof(f)) { + /* Position indicator - will be updated by %n if format fully matches */ + int n = 0; + /* + * The module name and symbol name are both without whitespace, + * but we cannot use %s to capture, as it consumes the closing ']' + * of the module format. Hence the %127[^] \t\n\r\v\f] which provides + * the equivalent effect. + */ + ret = fscanf(f, "%llx %c %499s [%127[^] \t\n\r\v\f]] %n", + &sym_addr, &sym_type, sym_name, module_name, &n); + + if (ret == 4 && n > 0) { + /* + * Module symbol: all 4 fields matched AND we reached %n. + * n > 0 means we successfully parsed up to the closing ']'. + */ + err = cb(sym_addr, sym_type, sym_name, module_name, ctx); + } else if (ret == 3) { + /* + * vmlinux symbol: 3 fields matched, next is matching failure against + * '[', but we don't care as we got what we needed. Also note that the + * trailing newline was consumed by the space after the symbol name. + */ + err = cb(sym_addr, sym_type, sym_name, NULL, ctx); + } else if (ret == EOF && feof(f)) { break; - if (ret != 3) { - pr_warn("failed to read kallsyms entry: %d\n", ret); + } else { + pr_warn("failed to read kallsyms entry: ret=%d n=%d\n", ret, n); err = -EINVAL; break; } - err = cb(sym_addr, sym_type, sym_name, ctx); if (err) break; } @@ -8486,7 +8508,7 @@ static int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *ctx) } static int kallsyms_cb(unsigned long long sym_addr, char sym_type, - const char *sym_name, void *ctx) + const char *sym_name, const char *module_name, void *ctx) { struct bpf_object *obj = ctx; const struct btf_type *t; @@ -11518,10 +11540,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) @@ -11642,6 +11674,47 @@ 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, skipping module symbols */ +static int find_kaddr_cb(unsigned long long sym_addr, char sym_type, + const char *sym_name, const char *module_name, void *ctx) +{ + struct find_kaddr_ctx *data = ctx; + + /* Skip module symbols - we only want vmlinux symbols */ + if (module_name != NULL) + return 0; + + /* Only match text section symbols ('T' or 't') */ + if (sym_type != 'T' && 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, @@ -11654,6 +11727,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); @@ -11684,21 +11759,22 @@ 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]; gen_probe_legacy_event_name(probe_name, sizeof(probe_name), - func_name, offset); + optional_func_name, offset); legacy_probe = strdup(probe_name); 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) { @@ -11707,6 +11783,46 @@ 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); + if (err) { + pr_warn("failed to get addr for %s\n", + func_name); + goto err_out; + } + + pr_debug("retrying %s using kaddr 0x%llx\n", + func_name, kaddr_ctx.kaddr); + + offset += kaddr_ctx.kaddr; + goto retry_create; + } + goto err_out; } link = bpf_program__attach_perf_event_opts(prog, pfd, &pe_opts); @@ -11822,7 +11938,7 @@ static int avail_func_cmp(const void *a, const void *b) } static int avail_kallsyms_cb(unsigned long long sym_addr, char sym_type, - const char *sym_name, void *ctx) + const char *sym_name, const char *module_name, void *ctx) { struct avail_kallsyms_data *data = ctx; struct kprobe_multi_resolve *res = data->res; -- 2.34.1 Add tests for kprobe attachment with duplicate symbols to validate the libbpf fallback using absolute kernel addresses from kallsyms. - Add bpf_testmod_dup_sym.ko test module creating a duplicate syscall wrapper symbol for vmlinux symbol preference testing - Register module in test and kmod Makefiles - Add test_attach_probe_dup_sym() with subtests covering all kprobe attachment modes (default, legacy, perf, link) - Validate kprobe attaches to vmlinux symbol by checking kprobe_res and kretprobe_res counters 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 | 45 +++++++++++++ 4 files changed, 110 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 c6bf4dfb1495..3d20a4d1d404 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..98b3e085ae90 --- /dev/null +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_dup_sym.c @@ -0,0 +1,45 @@ +// 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__ +noinline int __x64_sys_nanosleep(void) +#elif defined(__s390x__) +noinline int __s390x_sys_nanosleep(void) +#elif defined(__aarch64__) +noinline int __arm64_sys_nanosleep(void) +#elif defined(__riscv) +noinline int __riscv_sys_nanosleep(void) +#else +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