bpf: Allow uprobe_multi binary specified by file descriptor

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Missing access_ok() checks before calling __get_user() on user-controlled arrays, leading to arbitrary kernel memory read. --

commit e18574361f20d4f27005e4d927610e3ce16986dd
Author: Jiri Olsa <[email protected]>

bpf: Use user_path_at for path resolution in uprobe_multi

Resolve the uprobe_multi user path with user_path_at() instead of copying the string with strndup_user() and passing it to kern_path(). This removes the temporary allocation and keeps the lookup logic in one helper.

Quoted 4 lines

kernel/trace/bpf_trace.c +0 -0

index 90432f0fc2a8e..970ce7bbf99e1 100644

[ ... ]

Quoted 6 lines

kernel/trace/bpf_trace.c +0 -2

@@ -3261,14 +3260,7 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 	uref_ctr_offsets = u64_to_user_ptr(attr->link_create.uprobe_multi.ref_ctr_offsets);
 	ucookies = u64_to_user_ptr(attr->link_create.uprobe_multi.cookies);
 
-	name = strndup_user(upath, PATH_MAX);
-	if (IS_ERR(name)) {

kernel/trace/bpf_trace.c int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr · line 3266

Quoted 10 lines

kernel/trace/bpf_trace.c +1 -6

-		err = PTR_ERR(name);
-		return err;
-	}
-
-	err = kern_path(name, LOOKUP_FOLLOW, &path);
-	kfree(name);
+	err = user_path_at(AT_FDCWD, upath, LOOKUP_FOLLOW, &path);
 	if (err)
 		return err;
 

[Severity: Critical]
This isn't a bug introduced by this patch, but while looking at bpf_uprobe_multi_link_attach(), I noticed the user-provided arrays uoffsets, uref_ctr_offsets, and ucookies are accessed shortly after this path resolution:

kernel/trace/bpf_trace.c:bpf_uprobe_multi_link_attach() {
    ...
    if (__get_user(uprobes[i].offset, uoffsets + i)) {
    ...
    if (uref_ctr_offsets && __get_user(uprobes[i].ref_ctr_offset, uref_ctr_offsets + i)) {
    ...
    if (ucookies && __get_user(uprobes[i].cookie, ucookies + i)) {
    ...
}

Are these arrays validated with access_ok() before __get_user() is called?

Because __get_user() bypasses the access_ok() address-space bounds check on most architectures, could an attacker supply kernel addresses for these arrays, causing the kernel to read arbitrary memory and store it into the uprobes structures?

If an attacker retrieves these contents by querying the link info via bpf_obj_get_info_by_fd() (which calls bpf_uprobe_multi_link_fill_link_info() and copies the memory back to user space), could this act as an arbitrary kernel memory read primitive to bypass kernel lockdown and KASLR?

kernel/bpf/syscall.c static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp) · line 3479

Quoted 11 lines

kernel/bpf/syscall.c +1 -1

index d4188a992bd8..cef9bd6316a5 100644
@@ -3475,7 +3475,7 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp)
 			seq_printf(m, "link_type:\t%s\n", link->flags == BPF_F_KPROBE_MULTI_RETURN ?
 				   "kretprobe_multi" : "kprobe_multi");
 		else if (link->type == BPF_LINK_TYPE_UPROBE_MULTI)
-			seq_printf(m, "link_type:\t%s\n", link->flags == BPF_F_UPROBE_MULTI_RETURN ?
+			seq_printf(m, "link_type:\t%s\n", link->flags & BPF_F_UPROBE_MULTI_RETURN ?
 				   "uretprobe_multi" : "uprobe_multi");

This change in bpf_link_show_fdinfo() switches the uprobe_multi check from an equality test to a bitwise test. Once a second flag bit exists (the BPF_F_UPROBE_MULTI_PATH_FD flag added by this same patch), a uretprobe_multi link created with both flags set would be misreported as "uprobe_multi" because the equality test no longer matches.

The equality comparison was introduced in:

803f0700a3bb ("bpf: Show precise link_type for {uprobe,kprobe}_multi fdinfo")

Should this change include:

Fixes 803f0700a3bb ("bpf: Show precise link_type for {uprobe,kprobe}_multi fdinfo")

Quoted 16 lines

kernel/trace/bpf_trace.c +7 -0

index 970ce7bbf99e..f5d68f6d90f3 100644
@@ -3214,6 +3215,38 @@ static u64 bpf_uprobe_multi_cookie(struct bpf_run_ctx *ctx)
 	return run_ctx->uprobe->cookie;
 }
 
+static int bpf_uprobe_multi_get_path(const union bpf_attr *attr, struct path *path)
+{
+	void __user *upath = u64_to_user_ptr(attr->link_create.uprobe_multi.path);
+	u32 path_fd = attr->link_create.uprobe_multi.path_fd;
+	u32 flags = attr->link_create.uprobe_multi.flags;
+
+	if (flags & BPF_F_UPROBE_MULTI_PATH_FD) {

[ ... ]

kernel/trace/bpf_trace.c static u64 bpf_uprobe_multi_cookie(struct bpf_run_ctx *ctx) · line 3245

Quoted 8 lines

kernel/trace/bpf_trace.c +8 -0

+	}
+
+	/*
+	 * When BPF_F_UPROBE_MULTI_PATH_FD is not set, the path is resolved
+	 * relative to the cwd (AT_FDCWD) or absolute using the upath string.
+	 */
+	if (!upath || path_fd)
+		return -EINVAL;

This isn't a bug, but the non-FD branch (taken when BPF_F_UPROBE_MULTI_PATH_FD is not set) reads path_fd in the check 'if (!upath || path_fd)'.

path_fd is only meaningful when BPF_F_UPROBE_MULTI_PATH_FD is set. A caller that supplies a valid string path but leaves path_fd as non-zero garbage (in a struct large enough to include path_fd) would get a spurious -EINVAL here.

The garbage value is never dereferenced or used for arithmetic, so this looks more like defensive input validation than a dangerous use, and old-style callers passing the pre-path_fd struct size get path_fd zeroed and pass cleanly.

Is rejecting a non-zero path_fd in the string-path mode intended, or should path_fd be ignored when the flag is absent?

kernel/trace/bpf_trace.c static u64 bpf_uprobe_multi_cookie(struct bpf_run_ctx *ctx) · line 3248

Quoted 3 lines

kernel/trace/bpf_trace.c +3 -0

+
+	return user_path_at(AT_FDCWD, upath, LOOKUP_FOLLOW, path);
+}

---
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/27284577665

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Compilation fails due to missing path_fd in the uprobe_multi UAPI struct. --

commit c73ec6f8d6c5a6bb4c8a75f70c333cac0d6d30d4
Author: Jiri Olsa <[email protected]>

libbpf: Add path_fd to struct bpf_link_create_opts

This commit adds the path_fd field to struct bpf_link_create_opts for uprobe_multi. It passes this new field through the kernel attribute interface.

tools/lib/bpf/bpf.c int bpf_link_create(int prog_fd, int target_fd, · line 845

Quoted 9 lines

tools/lib/bpf/bpf.c +1 -0

index f37e3416f61a5..96819c082c772 100644
@@ -842,6 +842,7 @@ int bpf_link_create(int prog_fd, int target_fd,
 		attr.link_create.uprobe_multi.ref_ctr_offsets = ptr_to_u64(OPTS_GET(opts, uprobe_multi.ref_ctr_offsets, 0));
 		attr.link_create.uprobe_multi.cookies = ptr_to_u64(OPTS_GET(opts, uprobe_multi.cookies, 0));
 		attr.link_create.uprobe_multi.pid = OPTS_GET(opts, uprobe_multi.pid, 0);
+		attr.link_create.uprobe_multi.path_fd = OPTS_GET(opts, uprobe_multi.path_fd, 0);

[Severity: Low]
Will this assignment cause a build failure?

Looking at the UAPI header in tools/include/uapi/linux/bpf.h, the uprobe_multi struct inside union bpf_attr doesn't appear to have a path_fd member:

	struct {
		__aligned_u64	path;
		__aligned_u64	offsets;
		__aligned_u64	ref_ctr_offsets;
		__aligned_u64	cookies;
		__u32		cnt;
		__u32		flags;
		__u32		pid;
	} uprobe_multi;

Should the path_fd member be added to the UAPI bpf_attr union as well?

Quoted 3 lines

tools/lib/bpf/bpf.c +0 -0

 		if (!OPTS_ZEROED(opts, uprobe_multi))
 			return libbpf_err(-EINVAL);
 		break;

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The selftest references a nonexistent struct member path_fd and an undeclared flag BPF_F_UPROBE_MULTI_PATH_FD, which will cause a compilation error. --

commit e5937d581d08ece9671b371d37fb6cd0d497b692
Author: Jiri Olsa <[email protected]>

selftests/bpf: Add uprobe_multi path_fd test

This commit adds a uprobe_multi link API selftest that opens /proc/self/exe and passes the resulting descriptor through opts.uprobe_multi.path_fd with BPF_F_UPROBE_MULTI_PATH_FD set.

Quoted 4 lines

tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c +0 -0

index 56cbea280fbd2..ffcf3c92f0474 100644

[ ... ]

Quoted 9 lines

tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c +5 -0

@@ -757,6 +758,65 @@ static void test_link_api(void)
 	__test_link_api(&child);
 }
 
+static void test_link_api_path_fd(void)
+{
+	LIBBPF_OPTS(bpf_link_create_opts, opts);
+	const char *resolve_path = "/proc/self/exe";
+	int prog_fd, link_fd = -1, path_fd = -1;

[ ... ]

tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c static void test_link_api(void) · line 787

Quoted 8 lines

tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c +8 -0

+	path_fd = open(resolve_path, O_RDONLY);
+	if (!ASSERT_GE(path_fd, 0, "path_fd"))
+		goto cleanup;
+
+	opts.uprobe_multi.path_fd = path_fd;
+	opts.uprobe_multi.offsets = offsets;
+	opts.uprobe_multi.cnt = ARRAY_SIZE(syms);
+	opts.uprobe_multi.flags = BPF_F_UPROBE_MULTI_PATH_FD;

[Severity: Low]
Does this code reference an API that hasn't been introduced yet?

The struct field path_fd is not a valid member of uprobe_multi within struct bpf_link_create_opts in the libbpf headers, and the BPF_F_UPROBE_MULTI_PATH_FD macro is undeclared.

Because the patch doesn't include the corresponding kernel or libbpf changes, it looks like this will cause a compilation error.

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch references an undefined struct member path_fd and an undeclared macro BPF_F_UPROBE_MULTI_PATH_FD, which will cause compilation to fail. --

commit 0e3f7d4f9ed46a64d02a1c29acb4f8e2215cb7c0
Author: Jiri Olsa <[email protected]>

selftests/bpf: Add uprobe_multi path_fd fail tests

This commit adds tests to the attach_api_fails suite to verify that invalid setups using the new path_fd feature in uprobe_multi fail as expected.

Quoted 4 lines

tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c +0 -0

@@ -536,7 +536,37 @@ static void test_attach_api_fails(void)

[ ... ]

tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c static void test_attach_api_fails(void) · line 547

Quoted 9 lines

tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c +8 -1

-	ASSERT_EQ(link_fd, -EINVAL, "pid_is_wrong");
+	if (!ASSERT_EQ(link_fd, -EINVAL, "pid_is_wrong"))
+		goto cleanup;
+
+	/* wrong path_fd */
+	LIBBPF_OPTS_RESET(opts,
+		.uprobe_multi.path = NULL,
+		.uprobe_multi.path_fd = -1,
+		.uprobe_multi.flags = BPF_F_UPROBE_MULTI_PATH_FD,

[Severity: Low]
Does this commit depend on a missing prerequisite patch?

The path_fd member and BPF_F_UPROBE_MULTI_PATH_FD macro do not appear to be defined in the current tree's bpf_link_create_opts struct or UAPI headers, which will cause a compilation failure here.

Quoted 19 lines

tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c +18 -0

+		.uprobe_multi.offsets = (unsigned long *)&offset,
+		.uprobe_multi.cnt = 1,
+	);
+
+	link_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_UPROBE_MULTI, &opts);
+	if (!ASSERT_ERR(link_fd, "link_fd"))
+		goto cleanup;
+	if (!ASSERT_EQ(link_fd, -EBADF, "path_fd_is_wrong"))
+		goto cleanup;
+
+	/* path and path_fd both set with BPF_F_UPROBE_MULTI_PATH_FD flag */
+	LIBBPF_OPTS_RESET(opts,
+		.uprobe_multi.path = path,
+		.uprobe_multi.path_fd = 1,
+		.uprobe_multi.flags = BPF_F_UPROBE_MULTI_PATH_FD,
+		.uprobe_multi.offsets = (unsigned long *)&offset,
+		.uprobe_multi.cnt = 1,
+	);

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1