On 2026/10/17 22:43, [email protected] wrote:
> 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 <[email protected]>
>
> 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);
> }
Will move OPTS_VALID(opts, bpf_link_create_opts) to __sys_bpf_fd() in
the next revision.
>
> 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.
>
Oops. Will move it to the LIBBPF_1.9.0 section.
Thanks,
Leon
