Re: [PATCH bpf-next v4 5/6] libbpf: Add log_opts support for bpf_link_create

[Leon Hwang]

On 2026/10/25 00:13, bot+bpf-ci@xxxxxxxxxx wrote:
>> libbpf: Add log_opts support for bpf_link_create
> 
> TL;DR: In bpf_link_create(), the shared iter_info/log_opts slot is keyed only
> on iter_info_len, so an iter_info pointer with len 0 may be misread as
> log_opts, possibly causing out-of-bounds reads/writes in libbpf.
> 
>> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
>> index 9a2107d4f6d8..00efa90a6ed9 100644
>> --- a/tools/lib/bpf/bpf.c
>> +++ b/tools/lib/bpf/bpf.c
> 
> [ ... ]
> 
>> @@ -803,6 +805,14 @@ int bpf_link_create(int prog_fd, int target_fd,
>>  	iter_info_len = OPTS_GET(opts, iter_info_len, 0);
>>  	target_btf_id = OPTS_GET(opts, target_btf_id, 0);
>>  
>> +	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)
> 
> For BPF_TRACE_ITER, iter_info_len is now the only thing that decides whether
> the shared iter_info/log_opts slot holds a union bpf_iter_link_info pointer
> or a struct bpf_log_opts pointer.
> 
> Can a caller that sets opts->iter_info but leaves opts->iter_info_len at 0
> end up with its union bpf_iter_link_info being read as a struct
> bpf_log_opts here?

Users should not do like this. By convention, iter_info_len==0 means no
iter_info.

> 
> bpf_program__attach_iter() copies opts->link_info into
> link_create_opts.iter_info and opts->link_info_len into iter_info_len
> without checking that they agree, and nothing in bpf.h or libbpf.h says
> log_opts is only used when iter_info_len == 0, or that iter_info needs
> iter_info_len.
> 
> Before this patch the pointer went to the kernel, and bpf_iter_link_attach()
> rejected the mismatch cleanly:
> 
>     if (bpfptr_is_null(ulinfo) ^ !linfo_len)
>         return -EINVAL;
> 
> With this patch, __sys_bpf_fd() runs OPTS_VALID(log_opts, bpf_log_opts) on
> the 16-byte union bpf_iter_link_info.  The first 8 bytes of that union
> (map.map_fd, or task.tid/pid, or cgroup.order/cgroup_fd) become
> log_opts->sz:
> 
> - If map_fd or tid is in the range 8..27, sz passes validation, and size and
>   level are read from bytes 16..24, which lie past the end of the 16-byte
>   object.
> 
> - If sz is 28 to 31, OPTS_HAS(log_opts, true_size) is true, so
>   OPTS_SET(log_opts, true_size, ...) writes 4 bytes at offset 24, which is 8
>   bytes past the end of the caller's object.  Can this corrupt the caller's
>   stack or heap?
> 
> - If sz is larger (a typical tid, or a cgroup fd shifted into the high 32
>   bits), libbpf_is_mem_zeroed() scans caller memory from offset 28 up to sz.
>   It usually stops with a "bpf_log_opts has non-zero extra bytes" warning,
>   but it reads out of bounds first.
> 
> In addition, attr.link_create.iter_info is now NULL with iter_info_len of 0,
> which the kernel accepts.  If validation does pass, could a task iterator
> attach to all tasks instead of the requested tid, and a map iterator fail
> with -EBADF instead of -EINVAL?
> 
> The commit message says log_opts support for iter links is future work.
> Would it be better to treat the slot as iter_info whenever
> attach_type == BPF_TRACE_ITER, and read log_opts only for other attach
> types, instead of keying only on iter_info_len?  That would keep the old
> behaviour for iter links exactly.

Not exactly. iter_info and iter_info_len must be set at the same time
for iter links. If iter_info_len is 0, iter_info must be NULL or zeroed
memory. Users must not misuse them by iter_info_len==0 but iter_info is
not zeroed memory.

Thanks,
Leon