linux-kernel - Re: [PATCH bpf-next v7 2/9] libbpf: Add support for extended bpf syscall

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <541db2c8-7a96-4cc3-835e-b0eac64d4e3e@linux.dev>
Date: Mon, 26 Jan 2026 10:03:54 +0800
From: Leon Hwang <leon.hwang@...ux.dev>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Alexei Starovoitov <alexei.starovoitov@...il.com>,
 bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>,
 John Fastabend <john.fastabend@...il.com>,
 Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau
 <martin.lau@...ux.dev>, Eduard Zingerman <eddyz87@...il.com>,
 Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>,
 KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>,
 Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
 Shuah Khan <shuah@...nel.org>, Christian Brauner <brauner@...nel.org>,
 Seth Forshee <sforshee@...nel.org>, Yuichiro Tsuji <yuichtsu@...zon.com>,
 Andrey Albershteyn <aalbersh@...hat.com>,
 Willem de Bruijn <willemb@...gle.com>, Jason Xing
 <kerneljasonxing@...il.com>, Tao Chen <chen.dylane@...ux.dev>,
 Mykyta Yatsenko <yatsenko@...a.com>,
 Kumar Kartikeya Dwivedi <memxor@...il.com>,
 Anton Protopopov <a.s.protopopov@...il.com>, Amery Hung
 <ameryhung@...il.com>, Rong Tao <rongtao@...tc.cn>,
 LKML <linux-kernel@...r.kernel.org>, Linux API <linux-api@...r.kernel.org>,
 "open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>,
 kernel-patches-bot@...com
Subject: Re: [PATCH bpf-next v7 2/9] libbpf: Add support for extended bpf
 syscall



On 24/1/26 02:52, Andrii Nakryiko wrote:
> On Thu, Jan 22, 2026 at 8:19 PM Leon Hwang <leon.hwang@...ux.dev> wrote:
>>
>>
>>
>> On 23/1/26 12:12, Alexei Starovoitov wrote:
>>> On Thu, Jan 22, 2026 at 8:07 PM Leon Hwang <leon.hwang@...ux.dev> wrote:
>>>>
>>>>
>>>>
>>>> On 23/1/26 11:55, Alexei Starovoitov wrote:
>>>>> On Thu, Jan 22, 2026 at 7:25 PM Leon Hwang <leon.hwang@...ux.dev> wrote:
>>>>>>
>>>>>>
>>>>>> +static int probe_bpf_syscall_common_attrs(int token_fd)
>>>>>> +{
>>>>>> +       int ret;
>>>>>> +
>>>>>> +       ret = probe_sys_bpf_ext();
>>>>>> +       return ret > 0;
>>>>>> +}
>>>>>
>>>>> When you look at the above, what thoughts come to mind?
>>>>>
>>>>> ... and please don't use ai for answers.
>>>>
>>>> My initial thought was whether probe_fd() is needed here to handle and
>>>> close a returned fd, since the return value of probe_sys_bpf_ext() isn’t
>>>> obvious from the call site.
> 
> Have you looked at how probes are called (in feat_supported()?) They
> all follow the same contract: > 0 (normally just 1) means feature is
> supported, 0 means feature is not supported, and <0 means something
> went wrong. Libbpf will log an error and will assume feature is not
> supported.
> 

I’ve looked at feat_supported().

Even though I was aware of the probe contract, I should have thought it
through more carefully in the context of feat_supported() and
probe_sys_bpf_ext(). With that in mind, your suggestion makes sense now.

> probe_sys_bpf_ext() should either follow that convention or drop the
> probe_ prefix altogether to avoid confusion. And then
> probe_bpf_syscall_common_attrs() is necessary only as a wrapper around
> probe_sys_bpf_ext() to ignore mandatory (but unused) token_fd argument
> (so to make it "pluggable" into feat_supported() framework).
> 
> So, just make probe_sys_bpf_ext() follow probe contract as described,
> and then just:
> 
> static int probe_bpf_syscall_common_attr(int token_fd)
> {
>     return probe_sys_bpf_ext();
> }
> 

I’ll make probe_sys_bpf_ext() follow the standard probe convention, and
keep probe_bpf_syscall_common_attrs() as a thin wrapper to ignore the
mandatory (but unused) token_fd argument, so it plugs cleanly into
feat_supported() framework.

> Alternatively, just make probe_sys_bpf_ext() take token_fd (but ignore
> it), and just use probe_sys_bpf_ext() directly for feat_supported().
> 
> 
> probe_fd() is not suitable here because it's for a common case when we
> expect syscall to succeed and create fd, in which case that successful
> fd represents successful feature detection. This is not the case here,
> so probe_fd() is not what you should use.
> 

Agreed as well that probe_fd() is not suitable here, since this probe is
not expected to return a successful FD.

Thanks for the detailed explanation.

Thanks,
Leon

>>>
>>> Fair enough, but then collapse it into one helper if FD is a concern.
>>> My question was about stylistic/taste preferences.
>>
>> Understood, thanks for the clarification.
>>
>> I’ll rework it with the stylistic preference in mind.
>>
>> Thanks,
>> Leon
>>