public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH bpf-next 0/2] libbpf: Handle duplicate kprobe symbols
@ 2026-02-18 22:56 Andrey Grodzovsky
  2026-02-18 22:56 ` [RFC PATCH bpf-next 1/2] libbpf: Handle duplicate kprobe symbols in attach_kprobe_opts Andrey Grodzovsky
  2026-02-18 22:56 ` [RFC PATCH bpf-next 2/2] selftests/bpf: Add tests for duplicate kprobe symbol handling Andrey Grodzovsky
  0 siblings, 2 replies; 10+ messages in thread
From: Andrey Grodzovsky @ 2026-02-18 22:56 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, jolsa, linux-open-source

When a kernel module exports a symbol with the same name as an existing
vmlinux symbol, kprobe attachment via bpf_program__attach_kprobe_opts()
fails with EADDRNOTAVAIL (perf_event_open path) or EINVAL (legacy tracefs
path). The kernel rejects the ambiguous symbol name because it cannot
determine which address to probe.

This is a practical problem: out-of-tree modules may inadvertently
duplicate vmlinux symbol names, and the current libbpf behavior provides
no recovery path — the attachment simply fails.

This series adds a fallback mechanism in libbpf that retries attachment
using the absolute kernel address instead of the symbol name when the
initial attempt fails due to duplicate symbols.

The approach:
  1. Enhance libbpf_kallsyms_parse() to extract module names from
     /proc/kallsyms entries (format: "addr type name [module]"),
     passing them to callbacks via a new module_name parameter.
  2. On EADDRNOTAVAIL/EINVAL failure, look up the function address
     in /proc/kallsyms, filtering to vmlinux-only text symbols.
     If the vmlinux symbol itself is ambiguous (same name, different
     addresses), reject with an error.
  3. Retry the probe attachment with func_name=NULL and the resolved
     address as the offset, bypassing the kernel's symbol lookup
     entirely.

This works because perf_event_open with config1=0 (no symbol name)
and config2=address performs a direct address-based probe without
any kallsyms validation. The legacy tracefs path uses the "0xADDR"
format which similarly avoids symbol resolution.

Patch 1 implements the core libbpf changes.
Patch 2 adds selftests with a test module (bpf_testmod_dup_sym.ko)
that creates a duplicate syscall wrapper symbol, validating all four
kprobe attachment modes (default, legacy, perf_event_open, link).

Andrey Grodzovsky (2):
  libbpf: Handle duplicate kprobe symbols in attach_kprobe_opts
  selftests/bpf: Add tests for duplicate kprobe symbol handling

 tools/lib/bpf/libbpf.c                        | 150 ++++++++++++++++--
 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 ++++++
 5 files changed, 243 insertions(+), 19 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/test_kmods/bpf_testmod_dup_sym.c

-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC PATCH bpf-next 1/2] libbpf: Handle duplicate kprobe symbols in attach_kprobe_opts
  2026-02-18 22:56 [RFC PATCH bpf-next 0/2] libbpf: Handle duplicate kprobe symbols Andrey Grodzovsky
@ 2026-02-18 22:56 ` Andrey Grodzovsky
  2026-02-18 23:31   ` bot+bpf-ci
  2026-02-19  0:28   ` Emil Tsalapatis
  2026-02-18 22:56 ` [RFC PATCH bpf-next 2/2] selftests/bpf: Add tests for duplicate kprobe symbol handling Andrey Grodzovsky
  1 sibling, 2 replies; 10+ messages in thread
From: Andrey Grodzovsky @ 2026-02-18 22:56 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, jolsa, linux-open-source

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 <andrey.grodzovsky@crowdstrike.com>
---
 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC PATCH bpf-next 2/2] selftests/bpf: Add tests for duplicate kprobe symbol handling
  2026-02-18 22:56 [RFC PATCH bpf-next 0/2] libbpf: Handle duplicate kprobe symbols Andrey Grodzovsky
  2026-02-18 22:56 ` [RFC PATCH bpf-next 1/2] libbpf: Handle duplicate kprobe symbols in attach_kprobe_opts Andrey Grodzovsky
@ 2026-02-18 22:56 ` Andrey Grodzovsky
  2026-02-23  9:19   ` Jiri Olsa
  1 sibling, 1 reply; 10+ messages in thread
From: Andrey Grodzovsky @ 2026-02-18 22:56 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, jolsa, linux-open-source

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 <andrey.grodzovsky@crowdstrike.com>
---
 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 <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+
+/* 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH bpf-next 1/2] libbpf: Handle duplicate kprobe symbols in attach_kprobe_opts
  2026-02-18 22:56 ` [RFC PATCH bpf-next 1/2] libbpf: Handle duplicate kprobe symbols in attach_kprobe_opts Andrey Grodzovsky
@ 2026-02-18 23:31   ` bot+bpf-ci
  2026-02-19  0:28   ` Emil Tsalapatis
  1 sibling, 0 replies; 10+ messages in thread
From: bot+bpf-ci @ 2026-02-18 23:31 UTC (permalink / raw)
  To: andrey.grodzovsky, bpf
  Cc: ast, daniel, andrii, jolsa, linux-open-source, ast, andrii,
	daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 5320 bytes --]

> 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

[ ... ]

> +/* 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) {

[ ... ]

> +		data->kaddr = sym_addr;

[ ... ]

> +	return 0;
> +}

find_kaddr_cb() skips module symbols and only matches vmlinux text
symbols. If the duplicate symbols that triggered -EADDRNOTAVAIL
happen to both be in modules (no vmlinux counterpart),
find_kaddr_cb() never matches anything. libbpf_kallsyms_parse()
returns 0 (success) but kaddr_ctx.kaddr remains 0.

In bpf_program__attach_kprobe_opts(), there is no validation that
kaddr was actually found before proceeding with the retry (see
below).

> @@ -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);

On the retry path, optional_func_name is NULL. gen_probe_legacy_event_name()
passes this directly to snprintf() as a %s argument:

    snprintf(buf, buf_sz, "libbpf_%u_%d_%s_0x%zx",
             getpid(), ..., name, offset);

Passing NULL for %s is undefined behavior. Could this be given a
fallback string, or could gen_probe_legacy_event_name() handle a NULL
name?

>
>  		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 */);

Similarly, when the retry path passes NULL as optional_func_name to
perf_event_kprobe_open_legacy(), its internal pr_warn() messages use
kfunc_name with %s:

    pr_warn("failed to add legacy kprobe event for '%s+0x%zx': %s\n",
            kfunc_name, offset, errstr(err));

This is the same undefined behavior if the legacy probe creation
fails on the retry attempt.

>  	}
>  	if (pfd < 0) {
> @@ -11707,6 +11783,46 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog,

[ ... ]

> +		if ((err == -EADDRNOTAVAIL || (legacy && err == -EINVAL)) && optional_func_name) {

[ ... ]

> +			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;

When libbpf_kallsyms_parse() returns 0 but find_kaddr_cb() never
found a matching symbol, kaddr_ctx.kaddr is still 0. The code then
does offset += 0 and retries with optional_func_name == NULL, which
would attempt to probe at the user's original offset as an absolute
address (typically 0x0).

Would it make sense to check kaddr_ctx.kaddr != 0 before proceeding
with the retry?

Also, when the retry path is taken in legacy mode, the previous
legacy_probe allocation from strdup() is overwritten without being
freed when the goto jumps back to retry_create and enters the
legacy branch again:

    legacy_probe = strdup(probe_name);

Does the old legacy_probe need to be freed before goto retry_create
to avoid a memory leak?

> +		}
> +
>  		goto err_out;
>  	}

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/22161347107

AI-authorship-score: medium
AI-authorship-explanation: Verbose explanatory comments and structured commit message suggest possible AI assistance, but domain-specific fscanf format and kprobe knowledge suggest human involvement.
issues-found: 4
issue-severity-score: medium
issue-severity-explanation: Memory leak of legacy_probe on retry path, missing validation of kaddr lookup result leading to probing at address 0, and undefined behavior from NULL passed to printf %s in legacy retry path.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH bpf-next 1/2] libbpf: Handle duplicate kprobe symbols in attach_kprobe_opts
  2026-02-18 22:56 ` [RFC PATCH bpf-next 1/2] libbpf: Handle duplicate kprobe symbols in attach_kprobe_opts Andrey Grodzovsky
  2026-02-18 23:31   ` bot+bpf-ci
@ 2026-02-19  0:28   ` Emil Tsalapatis
  2026-02-19 21:57     ` [External] " Andrey Grodzovsky
  1 sibling, 1 reply; 10+ messages in thread
From: Emil Tsalapatis @ 2026-02-19  0:28 UTC (permalink / raw)
  To: Andrey Grodzovsky, bpf; +Cc: ast, daniel, andrii, jolsa, linux-open-source

On Wed Feb 18, 2026 at 5:56 PM EST, Andrey Grodzovsky wrote:
> 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 <andrey.grodzovsky@crowdstrike.com>
> ---
>  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];

Why 128? __MODULE_NAME_LEN is defined as (64 - sizeof(unsigned long)) in
moduleparam.h.

>  	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);
> +


The following:

> +		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;
>  	}


is pretty difficult to follow. Can avoid the long if-else chain
immediately after the fscanf? Maybe something like the following:

char *name;

while (
	ret = fscanf(...);
	if (ret != 3 && ret != 4)
		break;

	if (ret == 4 && n > 0)
		name = module_name;
	else
		name = NULL;

	err = cb(..., name, ctx);
	if (err)
		...
}

if (!feof(f)) {
	pr_warn(...)
	ret = EINVAL;
}

It's also closer to the original code.

> @@ -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)
>  {

Is the new argument really needed here? It seems like both for kallsyms_cb
and avail_kallsyms_cb this change is actually there for find_kaddr_cb.
But find_kaddr_cb doesn't even use the module name, it checks if it's 0.
If not then it skips the function.

We can remove the extra argument from all the callback signatures, and
instead just change libbpf_kallsyms parse with an extra parameter:

static int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *ctx, bool
skip_if_module)

or something like that, and skip calling the callback when the flag is
set if module_name != NULL.

>  	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')

nit: toupper(symtype) != '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;

AFAICT this will cause NULL to be passed to snprintf in
gen_probe_legacy_event_name.

> +
> +			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;


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [External] Re: [RFC PATCH bpf-next 1/2] libbpf: Handle duplicate kprobe symbols in attach_kprobe_opts
  2026-02-19  0:28   ` Emil Tsalapatis
@ 2026-02-19 21:57     ` Andrey Grodzovsky
  0 siblings, 0 replies; 10+ messages in thread
From: Andrey Grodzovsky @ 2026-02-19 21:57 UTC (permalink / raw)
  To: Emil Tsalapatis; +Cc: bpf, ast, daniel, andrii, jolsa, linux-open-source

On Wed, Feb 18, 2026 at 7:28 PM Emil Tsalapatis <emil@etsalapatis.com> wrote:
>
> On Wed Feb 18, 2026 at 5:56 PM EST, Andrey Grodzovsky wrote:
> > 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 <andrey.grodzovsky@crowdstrike.com>
> > ---
> >  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];
>
> Why 128? __MODULE_NAME_LEN is defined as (64 - sizeof(unsigned long)) in
> moduleparam.h.

I see your point, I didn't see this one. I searched UAPI headers and
found nothing, so I basically
'appealed to authority' in the existing code, and I found that perf in
kernel_get_module_dso defines
Set the length arbitrarily to 128. SO i followed this. I am worried
that if we just redefine
__MODULE_NAME_LEN  (64 - sizeof(unsigned long)) locally here and use
it, if in future kernel
If this version definition gets longer, we will start failing the
parsing on those long names. So it seems
It seems reasonable to give this an extra length buffer. It's just a
stack variable and also it has no
performance penalty as we terminate the moment we see closing ']' anyway.
If you still strongly feel this needs to be changed I will do it. In
this case are you ok with me
locally define  __MODULE_NAME_LEN  (64 - sizeof(unsigned long))


>
> >       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);
> > +
>
>
> The following:
>
> > +             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;
> >       }
>
>
> is pretty difficult to follow. Can avoid the long if-else chain
> immediately after the fscanf? Maybe something like the following:
>
> char *name;
>
> while (
>         ret = fscanf(...);
>         if (ret != 3 && ret != 4)
>                 break;

All in all looks much better! Will do. One point: we still need to add

 if (ret == 4 && n == 0)
          break;

here because we don't want to skip validation where we see the opening '['
of [module_name] matched but the closing ']' didn't.

Andrey

>
>         if (ret == 4 && n > 0)
>                 name = module_name;
>         else
>                 name = NULL;
>
>         err = cb(..., name, ctx);
>         if (err)
>                 ...
> }
>
> if (!feof(f)) {
>         pr_warn(...)
>         ret = EINVAL;
> }
>
> It's also closer to the original code.
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH bpf-next 2/2] selftests/bpf: Add tests for duplicate kprobe symbol handling
  2026-02-18 22:56 ` [RFC PATCH bpf-next 2/2] selftests/bpf: Add tests for duplicate kprobe symbol handling Andrey Grodzovsky
@ 2026-02-23  9:19   ` Jiri Olsa
  2026-02-23 18:56     ` [External] " Andrey Grodzovsky
  0 siblings, 1 reply; 10+ messages in thread
From: Jiri Olsa @ 2026-02-23  9:19 UTC (permalink / raw)
  To: Andrey Grodzovsky; +Cc: bpf, ast, daniel, andrii, linux-open-source

On Wed, Feb 18, 2026 at 05:56:17PM -0500, Andrey Grodzovsky wrote:

SNIP

> 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 <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +
> +/* 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;
> +}

hi,
this fails to build for me:

  CC [M]  bpf_testmod_dup_sym.o
bpf_testmod_dup_sym.c:14:14: error: no previous prototype for ‘__x64_sys_nanosleep’ [-Werror=missing-prototypes]
   14 | noinline int __x64_sys_nanosleep(void)
      |              ^~~~~~~~~~~~~~~~~~~

jirka

> +
> +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
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [External] Re: [RFC PATCH bpf-next 2/2] selftests/bpf: Add tests for duplicate kprobe symbol handling
  2026-02-23  9:19   ` Jiri Olsa
@ 2026-02-23 18:56     ` Andrey Grodzovsky
  2026-02-24 10:54       ` Jiri Olsa
  0 siblings, 1 reply; 10+ messages in thread
From: Andrey Grodzovsky @ 2026-02-23 18:56 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: bpf, ast, daniel, andrii, linux-open-source

On Mon, Feb 23, 2026 at 4:19 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Wed, Feb 18, 2026 at 05:56:17PM -0500, Andrey Grodzovsky wrote:
>
> SNIP
>
> > 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 <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +
> > +/* 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;
> > +}
>
> hi,
> this fails to build for me:
>
>   CC [M]  bpf_testmod_dup_sym.o
> bpf_testmod_dup_sym.c:14:14: error: no previous prototype for ‘__x64_sys_nanosleep’ [-Werror=missing-prototypes]
>    14 | noinline int __x64_sys_nanosleep(void)
>       |              ^~~~~~~~~~~~~~~~~~~
>
> jirka

Hey Jiri, ran i on my local mirror of CI repo  with latest rebased to
origin/bpf-next_base with command
tools/testing/selftests/bpf/vmtest.sh -- ./test_progs -t attach_probe
and (arch is x86) and eveyrhing pass.
Then i also ran in guthub
https://github.com/kernel-patches/bpf/pull/11138 and the only single
failure i got
is during runtime is some unrealted test failure.
https://github.com/kernel-patches/bpf/actions/runs/22318486040/job/64571209293?pr=11138
This seems to be because of me defining sys_nanosleep duplicate
without static qualifier and so if your
local build has -Werror=missing-prototypes,  this warning cause a
failure? Do you think we should
add static here ?

Andrey

>
> > +
> > +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
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [External] Re: [RFC PATCH bpf-next 2/2] selftests/bpf: Add tests for duplicate kprobe symbol handling
  2026-02-23 18:56     ` [External] " Andrey Grodzovsky
@ 2026-02-24 10:54       ` Jiri Olsa
  2026-02-24 13:06         ` Jiri Olsa
  0 siblings, 1 reply; 10+ messages in thread
From: Jiri Olsa @ 2026-02-24 10:54 UTC (permalink / raw)
  To: Andrey Grodzovsky; +Cc: Jiri Olsa, bpf, ast, daniel, andrii, linux-open-source

On Mon, Feb 23, 2026 at 01:56:15PM -0500, Andrey Grodzovsky wrote:
> On Mon, Feb 23, 2026 at 4:19 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Wed, Feb 18, 2026 at 05:56:17PM -0500, Andrey Grodzovsky wrote:
> >
> > SNIP
> >
> > > 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 <linux/init.h>
> > > +#include <linux/module.h>
> > > +#include <linux/kernel.h>
> > > +
> > > +/* 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;
> > > +}
> >
> > hi,
> > this fails to build for me:
> >
> >   CC [M]  bpf_testmod_dup_sym.o
> > bpf_testmod_dup_sym.c:14:14: error: no previous prototype for ‘__x64_sys_nanosleep’ [-Werror=missing-prototypes]
> >    14 | noinline int __x64_sys_nanosleep(void)
> >       |              ^~~~~~~~~~~~~~~~~~~
> >
> > jirka
> 
> Hey Jiri, ran i on my local mirror of CI repo  with latest rebased to
> origin/bpf-next_base with command
> tools/testing/selftests/bpf/vmtest.sh -- ./test_progs -t attach_probe
> and (arch is x86) and eveyrhing pass.
> Then i also ran in guthub
> https://github.com/kernel-patches/bpf/pull/11138 and the only single
> failure i got
> is during runtime is some unrealted test failure.
> https://github.com/kernel-patches/bpf/actions/runs/22318486040/job/64571209293?pr=11138
> This seems to be because of me defining sys_nanosleep duplicate
> without static qualifier and so if your
> local build has -Werror=missing-prototypes,  this warning cause a
> failure? Do you think we should
> add static here ?

I think adding declaration will fix it, but if CI is fine,
it's something in my setup.. please ignore, I'll check

jirka

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [External] Re: [RFC PATCH bpf-next 2/2] selftests/bpf: Add tests for duplicate kprobe symbol handling
  2026-02-24 10:54       ` Jiri Olsa
@ 2026-02-24 13:06         ` Jiri Olsa
  0 siblings, 0 replies; 10+ messages in thread
From: Jiri Olsa @ 2026-02-24 13:06 UTC (permalink / raw)
  To: Andrey Grodzovsky; +Cc: Jiri Olsa, bpf, ast, daniel, andrii, linux-open-source

On Tue, Feb 24, 2026 at 11:54:24AM +0100, Jiri Olsa wrote:
> On Mon, Feb 23, 2026 at 01:56:15PM -0500, Andrey Grodzovsky wrote:
> > On Mon, Feb 23, 2026 at 4:19 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > On Wed, Feb 18, 2026 at 05:56:17PM -0500, Andrey Grodzovsky wrote:
> > >
> > > SNIP
> > >
> > > > 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 <linux/init.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/kernel.h>
> > > > +
> > > > +/* 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;
> > > > +}
> > >
> > > hi,
> > > this fails to build for me:
> > >
> > >   CC [M]  bpf_testmod_dup_sym.o
> > > bpf_testmod_dup_sym.c:14:14: error: no previous prototype for ‘__x64_sys_nanosleep’ [-Werror=missing-prototypes]
> > >    14 | noinline int __x64_sys_nanosleep(void)
> > >       |              ^~~~~~~~~~~~~~~~~~~
> > >
> > > jirka
> > 
> > Hey Jiri, ran i on my local mirror of CI repo  with latest rebased to
> > origin/bpf-next_base with command
> > tools/testing/selftests/bpf/vmtest.sh -- ./test_progs -t attach_probe
> > and (arch is x86) and eveyrhing pass.
> > Then i also ran in guthub
> > https://github.com/kernel-patches/bpf/pull/11138 and the only single
> > failure i got
> > is during runtime is some unrealted test failure.
> > https://github.com/kernel-patches/bpf/actions/runs/22318486040/job/64571209293?pr=11138
> > This seems to be because of me defining sys_nanosleep duplicate
> > without static qualifier and so if your
> > local build has -Werror=missing-prototypes,  this warning cause a
> > failure? Do you think we should
> > add static here ?
> 
> I think adding declaration will fix it, but if CI is fine,
> it's something in my setup.. please ignore, I'll check

ok it's just warning, but does not break the build, I can see it
in CI run as well..  but I guess we still want to fix it.. let's
add the missing externs?

jirka

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-02-24 13:06 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-18 22:56 [RFC PATCH bpf-next 0/2] libbpf: Handle duplicate kprobe symbols Andrey Grodzovsky
2026-02-18 22:56 ` [RFC PATCH bpf-next 1/2] libbpf: Handle duplicate kprobe symbols in attach_kprobe_opts Andrey Grodzovsky
2026-02-18 23:31   ` bot+bpf-ci
2026-02-19  0:28   ` Emil Tsalapatis
2026-02-19 21:57     ` [External] " Andrey Grodzovsky
2026-02-18 22:56 ` [RFC PATCH bpf-next 2/2] selftests/bpf: Add tests for duplicate kprobe symbol handling Andrey Grodzovsky
2026-02-23  9:19   ` Jiri Olsa
2026-02-23 18:56     ` [External] " Andrey Grodzovsky
2026-02-24 10:54       ` Jiri Olsa
2026-02-24 13:06         ` Jiri Olsa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox