Re: [PATCH bpf-next 2/6] bpf: Add support to specify uprobe_multi target via file descriptor

[Jiri Olsa <olsajiri@gmail.com>]

On Tue, Jun 09, 2026 at 10:57:22AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] Missing strict zero-validation for the unused `path_fd` field when the `BPF_F_UPROBE_MULTI_PATH_FD` flag is not set.
> - [Low] Mixing scope-based cleanup (`CLASS()`) with `goto`-based error handling in `bpf_uprobe_multi_link_attach`.
> --
> 
> commit f7958ff1ce57c1ab456d3f134881a0081a58b8ed
> Author: Jiri Olsa <jolsa@kernel.org>
> 
> bpf: Add support to specify uprobe_multi target via file descriptor
> 
> This commit adds the BPF_F_UPROBE_MULTI_PATH_FD flag to allow uprobe_multi
> links to specify the target binary using an already opened file descriptor
> instead of a path string.
> 
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 970ce7bbf99e1..4965006d58ee6 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> [ ... ]
> > @@ -3240,29 +3241,51 @@ 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);
> > +	upath = u64_to_user_ptr(attr->link_create.uprobe_multi.path);
> >  
> > -	err = user_path_at(AT_FDCWD, upath, LOOKUP_FOLLOW, &path);
> > -	if (err)
> > -		return err;
> > +	if (flags & BPF_F_UPROBE_MULTI_PATH_FD) {
> > +		/*
> > +		 * When BPF_F_UPROBE_MULTI_PATH_FD is set, the executable is identified
> > +		 * by path_fd, and upath must be NULL.
> > +		 */
> > +		if (upath)
> > +			return -EINVAL;
> > +
> > +		CLASS(fd, f)(attr->link_create.uprobe_multi.path_fd);
> 
> [Severity: Low]
> Does this code violate the cleanup subsystem guidelines by mixing scope-based
> cleanup with goto-based error handling?
> 
> The bpf_uprobe_multi_link_attach() function relies heavily on goto labels
> for error paths (e.g., error_path_put, error_free, error_unregister). The
> kernel guidelines state that goto-based cleanup and scope-based cleanup
> helpers (like CLASS) should never be mixed in the same function.
> 
> Should this function be fully converted to scope-based cleanup, or should it
> use explicit fdget()/fdput() without CLASS() instead?

ah true, I'll unwind the fd retrieval or perhaps move the path retrival
into separate function, will check

> > +		if (fd_empty(f))
> > +			return -EBADF;
> > +		path = fd_file(f)->f_path;
> > +		path_get(&path);
> > +	} else {
> > +		/*
> > +		 * 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)
> > +			return -EINVAL;
> 
> [Severity: Medium]
> Should there be strict zero-validation for the unused path_fd field here?
> 
> When the BPF_F_UPROBE_MULTI_PATH_FD flag is not set, this branch is taken
> and attr->link_create.uprobe_multi.path_fd is ignored without being
> validated as zero.
> 
> If userspace passes uninitialized garbage in this field, it won't be
> rejected. Could this break forward compatibility if path_fd needs to be
> assigned a new meaning in the future?

right, I'll add the path_fd check in here

jirka

> 
> > +
> > +		err = user_path_at(AT_FDCWD, upath, LOOKUP_FOLLOW, &path);
> > +		if (err)
> > +			return err;
> > +	}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260609104244.588321-1-jolsa@kernel.org?part=2