Re: [PATCH bpf-next v14 2/8] libbpf: Add support for extended BPF syscall

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

On 13/5/26 00:23, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
>> index 5846de364209..9d8740761b7a 100644
>> --- a/tools/lib/bpf/bpf.c
>> +++ b/tools/lib/bpf/bpf.c
> 
> [ ... ]
> 
>> +int probe_sys_bpf_ext(void)
>> +{
>> +	const size_t attr_sz = offsetofend(union bpf_attr, prog_token_fd);
>> +	union bpf_attr attr;
>> +	int fd;
>> +
>> +	memset(&attr, 0, attr_sz);
>> +	fd = syscall(__NR_bpf, BPF_PROG_LOAD | BPF_COMMON_ATTRS, &attr, attr_sz, NULL,
>> +		     sizeof(struct bpf_common_attr));
>> +	if (fd >= 0) {
>> +		close(fd);
>> +		return -EINVAL;
>                         ^^^^^^
> 
> Should probe_sys_bpf_ext() return 1 instead of -EINVAL when fd >= 0?
> 
> The probe function detects kernel support for extended BPF syscall
> attributes by calling the BPF syscall with the BPF_COMMON_ATTRS flag
> and a NULL pointer for common attributes.
> 
> The expected outcomes are:
> - New kernel with support: errno=EFAULT (can't copy from NULL) -> return 1
> - Old kernel without support: errno!=EFAULT (invalid command) -> return 0
> 
> If the syscall succeeds (fd >= 0), that definitively proves the kernel
> accepts the extended syscall format and supports BPF_COMMON_ATTRS. The
> function should return 1 (supported) rather than -EINVAL (error).
> 
> Returning -EINVAL causes feat_supported() in features.c to print a
> warning "Detection of kernel BPF syscall common attributes support
> support failed: Invalid argument" and mark the feature as MISSING,
> incorrectly reporting that the kernel doesn't support the feature when
> the successful syscall proves it does.
> 

Should not return 1.

This return was discussed in v5 and v6.

Cite Andrii's review in v5:

But fd should really not be >= 0, and if it is -- it's some problem,
so I'd return an error in that case to keep us aware, which is why I'm
saying I'd just return inside if (fd >= 0) { }

Thanks,
Leon

>> +	}
>> +	return errno == EFAULT ? 1 : 0;
>> +}
> 
> [ ... ]
> 
> 
> ---
> 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/25745661172