Re: [PATCH bpf-next 5/6] libbpf: Add bpf_link_create_log API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] libbpf: missing OPTS_VALID() check for log_opts in __bpf_link_create()
- [Medium] libbpf: bpf_link_create_log added to released LIBBPF_1.8.0 in libbpf.map
--

commit 5adb04f5088a9ab6aa21d83e4c9370c13ebdaab6
Author: Leon Hwang <leon.hwang@linux.dev>

libbpf: Add bpf_link_create_log API

This commit introduces the bpf_link_create_log() API to pass struct
bpf_log_opts when creating BPF links without modifying struct
bpf_link_create_opts.

> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 17e73e443bc7a..eb127d71f3c83 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -788,9 +788,10 @@ int bpf_prog_detach2(int prog_fd, int target_fd, enum bpf_attach_type type)
>  	return bpf_prog_detach_opts(prog_fd, target_fd, type, NULL);
>  }
>  
> -int bpf_link_create(int prog_fd, int target_fd,
> -		    enum bpf_attach_type attach_type,
> -		    const struct bpf_link_create_opts *opts)
> +static int __bpf_link_create(int prog_fd, int target_fd,
> +			     enum bpf_attach_type attach_type,
> +			     const struct bpf_link_create_opts *opts,
> +			     struct bpf_log_opts *log_opts)
>  {
>  	const size_t attr_sz = offsetofend(union bpf_attr, link_create);
>  	__u32 target_btf_id, iter_info_len, relative_id;
> @@ -965,7 +966,7 @@ int bpf_link_create(int prog_fd, int target_fd,
>  		break;
>  	}
>  proceed:
> -	fd = sys_bpf_fd(BPF_LINK_CREATE, &attr, attr_sz);
> +	fd = __sys_bpf_fd(&attr, attr_sz, BPF_LINK_CREATE, log_opts);

[Severity: Medium]
Should __bpf_link_create() validate log_opts with OPTS_VALID(log_opts,
bpf_log_opts) before passing it to __sys_bpf_fd()?

Currently __bpf_link_create() validates opts, but not log_opts:

tools/lib/bpf/bpf.c:__bpf_link_create() {
	if (!OPTS_VALID(opts, bpf_link_create_opts))
		return libbpf_err(-EINVAL);
}

When a caller passes a malformed struct bpf_log_opts (such as an invalid sz
or non-zero unknown trailing fields) to bpf_link_create_log(),
__bpf_link_create() forwards it directly to __sys_bpf_fd(), which accesses
log_opts via OPTS_GET() and OPTS_SET() without checking OPTS_VALID().

For comparison, bpf_map_create() validates log_opts before calling
__sys_bpf_fd():

tools/lib/bpf/bpf.c:bpf_map_create() {
	log_opts = OPTS_GET(opts, log_opts, NULL);
	if (!OPTS_VALID(log_opts, bpf_log_opts))
		return libbpf_err(-EINVAL);

	fd = __sys_bpf_fd(&attr, attr_sz, BPF_MAP_CREATE, log_opts);
}

[ ... ]

> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index eeba4072a5dac..990e2f926ecc3 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -458,6 +458,7 @@ LIBBPF_1.7.0 {
>  
>  LIBBPF_1.8.0 {
>  	global:
> +		bpf_link_create_log;
>  		bpf_map__attach_cgroup_opts;
>  		bpf_prog_stream_open;
>  		bpf_program__add_flags;

[Severity: Medium]
Should bpf_link_create_log be placed under the LIBBPF_1.9.0 stanza in
tools/lib/bpf/libbpf.map instead of LIBBPF_1.8.0?

LIBBPF_1.8.0 is already succeeded by LIBBPF_1.9.0 at the bottom of
tools/lib/bpf/libbpf.map:

LIBBPF_1.9.0 {
} LIBBPF_1.8.0;

Adding bpf_link_create_log to LIBBPF_1.8.0 exports the new symbol under the
previous version node rather than the current LIBBPF_1.9.0 development cycle.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017142649.51047-1-leon.hwang@linux.dev?part=5