netdev - Re: [PATCHv5 bpf] bpf: Move iterator functions into special init section

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: <073f2234-b3ce-aa6d-f1d9-e216aeede68a@iogearbox.net>
Date:   Wed, 11 Nov 2020 00:30:40 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Jiri Olsa <jolsa@...hat.com>
Cc:     Jiri Olsa <jolsa@...nel.org>, Alexei Starovoitov <ast@...nel.org>,
        Andrii Nakryiko <andriin@...com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Yonghong Song <yhs@...com>, Song Liu <songliubraving@...com>,
        Andrii Nakryiko <andrii@...nel.org>, netdev@...r.kernel.org,
        bpf@...r.kernel.org, Martin KaFai Lau <kafai@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...omium.org>
Subject: Re: [PATCHv5 bpf] bpf: Move iterator functions into special init
 section

On 11/10/20 11:35 AM, Jiri Olsa wrote:
> On Mon, Nov 09, 2020 at 11:04:34PM +0100, Daniel Borkmann wrote:
> 
> SNIP
> 
>>> index 7b53cb3092ee..a7c71e3b5f9a 100644
>>> --- a/include/linux/init.h
>>> +++ b/include/linux/init.h
>>> @@ -52,6 +52,7 @@
>>>    #define __initconst	__section(".init.rodata")
>>>    #define __exitdata	__section(".exit.data")
>>>    #define __exit_call	__used __section(".exitcall.exit")
>>> +#define __init_bpf_preserve_type __section(".init.bpf.preserve_type")
>>
>> Small nit, why this detour via BPF_INIT define? Couldn't we just:
>>
>> #ifdef CONFIG_DEBUG_INFO_BTF
>> #define __init_bpf_preserve_type   __section(".init.bpf.preserve_type")
>> #else
>> #define __init_bpf_preserve_type   __init
>> #endif
>>
>> Also, the comment above the existing defines says '/* These are for everybody (although
>> not all archs will actually discard it in modules) */' ... We should probably not add
>> the __init_bpf_preserve_type right under this listing as-is in your patch, but instead
>> 'separate' it by adding a small comment on top of its definition by explaining its
>> purpose more clearly for others.
> 
> ok, for some reason I thought I needed to add it to init.h,
> but as it's bpf specific, perhaps we can omit init.h change
> completely.. how about the change below?

Agree, that looks much better, thanks!