Re: [PATCH bpf-next 5/9] libbpf: Add log_opts support for BPF_TRACE_ITER

[Leon Hwang <leon.hwang@linux.dev>]

On 2026/11/9 22:36, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential
> issues to consider:
> - [Low] libbpf: missing OPTS_ZEROED() check for BPF_TRACE_ITER in
>   bpf_link_create()
> - [Low] libbpf: unreachable iter.len check in bpf_link_create()
> - [Low] libbpf: deprecated iter_info fields used in
>   bpf_program__attach_iter()
> --
> 
> commit 254d03820df80f0d29b270cec3be5cafe9178fe6
> Author: Leon Hwang <leon.hwang@linux.dev>
> 
> libbpf: Add log_opts support for BPF_TRACE_ITER
> 
> This commit adds iter.info and iter.len to struct bpf_link_create_opts so
> that BPF_TRACE_ITER link creation can pass iterator parameters alongside
> log_opts, while keeping the legacy iter_info and iter_info_len fields for
> callers that do not use log_opts.
> 
>> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
>> index 38dcb31e8c0ed..174a8bfaf6638 100644
>> --- a/tools/lib/bpf/bpf.c
>> +++ b/tools/lib/bpf/bpf.c
>> @@ -808,11 +808,6 @@ int bpf_link_create(int prog_fd, int target_fd,
>>  	if (iter_info_len && attach_type != BPF_TRACE_ITER)
>>  		return libbpf_err(-EINVAL);
>>  
>> -	if (iter_info_len)
>> -		iter_info = OPTS_GET(opts, iter_info, (void *)0);
>> -	else
>> -		log_opts = OPTS_GET(opts, log_opts, (void *)0);
>> -
>>  	/* validate we don't have unexpected combinations of non-zero fields */
>>  	if (iter_info_len || target_btf_id) {
>>  		if (iter_info_len && target_btf_id)
>> @@ -821,6 +816,16 @@ int bpf_link_create(int prog_fd, int target_fd,
>>  			return libbpf_err(-EINVAL);
>>  	}
>>  
>> +	if (iter_info_len) {
>> +		if (OPTS_GET(opts, iter.len, 0))
>> +			return libbpf_err(-EINVAL);
> 
> [Severity: Low]
> Can OPTS_GET(opts, iter.len, 0) ever be non-zero here?  When iter_info_len
> is set, the preceding if (iter_info_len || target_btf_id) block has already
> verified OPTS_ZEROED(opts, target_btf_id), which rejects any non-zero bytes
> in the union following target_btf_id, including opts->iter.
Agreed.

Will drop this iter.len check.

> 
>> +		iter_info = OPTS_GET(opts, iter_info, (void *)0);
>> +	} else {
>> +		log_opts = OPTS_GET(opts, log_opts, (void *)0);
>> +		iter_info = OPTS_GET(opts, iter.info, (void *)0);
>> +		iter_info_len = OPTS_GET(opts, iter.len, 0);
>> +	}
> 
> [Severity: Low]
> Should bpf_program__attach_iter() also be updated to populate
> link_create_opts.iter.info and link_create_opts.iter.len instead of the
> deprecated iter_info and iter_info_len fields?
> 
> tools/lib/bpf/libbpf.c:bpf_program__attach_iter() {
>     ...
> 	link_create_opts.iter_info = OPTS_GET(opts, link_info, (void *)0);
> 	link_create_opts.iter_info_len = OPTS_GET(opts, link_info_len, 0);
>     ...
> }
> 
> Because iter_info shares a union with log_opts, calling
> bpf_program__attach_iter() with link_info != NULL and link_info_len == 0
> causes bpf_link_create() to take the else branch above and dereference the
> union bpf_iter_link_info pointer as struct bpf_log_opts *log_opts in
> __sys_bpf_fd().
> 

Will update bpf_program__attach_iter() to use iter.info and iter.len in
the next revision. And, will add log_opts for bpf_program__attach_iter()
btw.

> [ ... ]
> 
>> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
>> index fe3b7fd8512bd..2ac143fd1102e 100644
>> --- a/tools/lib/bpf/bpf.h
>> +++ b/tools/lib/bpf/bpf.h
> 
> [ ... ]
> 
>> @@ -478,6 +482,10 @@ struct bpf_link_create_opts {
>>  			const __u64 *cookies;
>>  			__u32 cnt;
>>  		} tracing_multi;
>> +		struct {
>> +			union bpf_iter_link_info *info;
>> +			__u32 len;
>> +		} iter;
>>  	};
>>  	size_t :0;
>>  };
> 
> [Severity: Low]
> Does case BPF_TRACE_ITER in bpf_link_create() miss an OPTS_ZEROED(opts,
> iter) check?

Will add OPTS_ZEROED(opts, iter) check.

Thanks,
Leon

> 
> tools/lib/bpf/bpf.c:bpf_link_create() {
>     ...
> 	switch (attach_type) {
> 	case BPF_TRACE_ITER:
> 		attr.link_create.iter_info = ptr_to_u64(iter_info);
> 		attr.link_create.iter_info_len = iter_info_len;
> 		break;
>     ...
> }
> 
> When opts->iter is used (so iter_info_len and target_btf_id are 0),
> trailing bytes in the anonymous union after opts->iter overlapping larger
> members such as uprobe_multi are not checked for zero, unlike all other
> union members in the switch statement.  Also, opts->iter.info and
> opts->iter.len are read in the !iter_info_len branch even when attach_type
> is not BPF_TRACE_ITER.
>