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

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 2387 bytes --]

> libbpf: Add log_opts support for BPF_TRACE_ITER

TL;DR:
- libbpf BPF_TRACE_ITER case may not reject stray bytes after iter.len
  (missing OPTS_ZEROED check), so unknown fields could be silently ignored
- Nit: iter.len test in the legacy branch looks like dead code

> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c

[ ... ]

> @@ -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);
> +		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);
> +	}
> +

When the legacy iter_info_len is non-zero, the check above already requires
OPTS_ZEROED(opts, target_btf_id), which covers every byte from that offset
up to sz, including iter.info and iter.len.  Can this OPTS_GET(opts,
iter.len, 0) test ever be true?  It looks like dead code.

[ ... ]

>  	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;

Every other union member's case ends with a check such as:

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

Should the BPF_TRACE_ITER case do the same with OPTS_ZEROED(opts, iter)?

On the new iter.info/iter.len path, nothing checks the bytes after iter.len.
A caller that sets opts.iter.info, opts.iter.len and opts.tracing_multi.cnt
gets the stray field silently ignored instead of -EINVAL.  Likewise, a
later field added to the iter struct after len, built against newer
headers, would be silently dropped by this libbpf because sz still equals
sizeof() and OPTS_VALID() cannot see it.

Legacy callers would be unaffected, since their union is already zero, and
the iter.len test in the legacy branch could then be dropped.  The later
commits in this series only touch selftests and do not address this.


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