netdev - Re: [RFC PATCH bpf-next] libbpf: make bpf_object_

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]

Date:   Fri, 23 Nov 2018 18:34:44 +0800
From:   "Wangnan (F)" <wangnan0@...wei.com>
To:     Daniel Borkmann <daniel@...earbox.net>,
        "Nikita V. Shirokov" <tehnerd@...nerd.com>,
        Alexei Starovoitov <ast@...nel.org>
CC:     <netdev@...r.kernel.org>
Subject: Re: [RFC PATCH bpf-next] libbpf: make bpf_object__open default to
 UNSPEC



On 2018/11/23 5:52, Daniel Borkmann wrote:
> [ +Wang ]
> 
> On 11/22/2018 07:03 AM, Nikita V. Shirokov wrote:
>> currently by default libbpf's bpf_object__open requires
>> bpf's program to specify  version in a code because of two things:
>> 1) default prog type is set to KPROBE
>> 2) KPROBE requires (in kernel/bpf/syscall.c) version to be specified
>>
>> in this RFC i'm proposing change default to UNSPEC and also changing
>> logic of libbpf that it would reflect what we have today in kernel
>> (aka only KPROBE type requires for version to be explicitly set).
>>
>> reason for change:
>> currently only libbpf requires by default version to be
>> explicitly set. it would be really hard for mainteiners of other custom
>> bpf loaders to migrate to libbpf (as they dont control user's code
>> and migration to the new loader (libbpf) wont be transparent for end
>> user).
>>
>> what is going to be broken after this change:
>> if someone were relying on default to be KPROBE for bpf_object__open
>> his code will stop to work. however i'm really doubtfull that anyone
>> is using this for kprobe type of programs (instead of, say, bcc or
>> other tracing frameworks)
>>
>> other possible solutions (for discussion, would require more machinery):
>> add another function like bpf_object__open w/ default to unspec
>>
>> Signed-off-by: Nikita V. Shirokov <tehnerd@...nerd.com>
>> ---
>>  tools/lib/bpf/libbpf.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index 0f14f7c074c2..ed4212a4c5f9 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -333,7 +333,7 @@ bpf_program__init(void *data, size_t size, char *section_name, int idx,
>>  	prog->idx = idx;
>>  	prog->instances.fds = NULL;
>>  	prog->instances.nr = -1;
>> -	prog->type = BPF_PROG_TYPE_KPROBE;
>> +	prog->type = BPF_PROG_TYPE_UNSPEC;
>>  	prog->btf_fd = -1;
> 
> Seems this was mostly for historic reasons, but for a generic library this
> would indeed be an odd convention for default. Wang, given 5f44e4c810bf
> ("tools lib bpf: New API to adjust type of a BPF program"), are you in any
> way relying on this default or using things like bpf_program__set_kprobe()
> instead which you've added there? If latter, I'd say we should then change
> it better now than later when there's even more lib usage (and in particular
> before we add official ABI versioning).

OK. I don't rely on that now.

Thank you.