linux-kernel - Re: [PATCH] bpf.h: fix clang compiler warning with unpriv_ebpf

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 11 May 2022 18:38:14 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Luis Chamberlain <mcgrof@...nel.org>
Cc:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Song Liu <songliubraving@...com>, bpf <bpf@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>, llvm@...ts.linux.dev,
        Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH] bpf.h: fix clang compiler warning with
 unpriv_ebpf_notify()

On 5/11/22 6:24 PM, Luis Chamberlain wrote:
> On Wed, May 11, 2022 at 06:17:26PM +0200, Daniel Borkmann wrote:
>> On 5/11/22 6:08 PM, Luis Chamberlain wrote:
>>> On Wed, May 11, 2022 at 09:03:13AM -0700, Alexei Starovoitov wrote:
>>>> On Wed, May 11, 2022 at 8:58 AM Luis Chamberlain <mcgrof@...nel.org> wrote:
>>>>> On Mon, May 09, 2022 at 01:36:23PM -0700, Luis Chamberlain wrote:
>>>>>> The recent commit "bpf: Move BPF sysctls from kernel/sysctl.c to BPF core"
>>>>>> triggered 0-day to issue an email for what seems to have been an old
>>>>>> clang warning. So this issue should have existed before as well, from
>>>>>> what I can tell. The issue is that clang expects a forward declaration
>>>>>> for routines declared as weak while gcc does not.
>>>>>>
>>>>>> This can be reproduced with 0-day's x86_64-randconfig-c007
>>>>>> https://download.01.org/0day-ci/archive/20220424/202204240008.JDntM9cU-lkp@intel.com/config
>>>>>>
>>>>>> And using:
>>>>>>
>>>>>> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=x86_64 SHELL=/bin/bash kernel/bpf/syscall.o
>>>>>> Compiler will be installed in /home/mcgrof/0day
>>>>>> make --keep-going HOSTCC=/home/mcgrof/0day/clang/bin/clang CC=/home/mcgrof/0day/clang/bin/clang LD=/home/mcgrof/0day/clang/bin/ld.lld HOSTLD=/home/mcgrof/0day/clang/bin/ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump OBJSIZE=llvm-size READELF=llvm-readelf HOSTCXX=clang++ HOSTAR=llvm-ar CROSS_COMPILE=x86_64-linux-gnu- --jobs=24 W=1 ARCH=x86_64 SHELL=/bin/bash kernel/bpf/syscall.o
>>>>>>     DESCEND objtool
>>>>>>     CALL    scripts/atomic/check-atomics.sh
>>>>>>     CALL    scripts/checksyscalls.sh
>>>>>>     CC      kernel/bpf/syscall.o
>>>>>> kernel/bpf/syscall.c:4944:13: warning: no previous prototype for function 'unpriv_ebpf_notify' [-Wmissing-prototypes]
>>>>>> void __weak unpriv_ebpf_notify(int new_state)
>>>>>>               ^
>>>>>> kernel/bpf/syscall.c:4944:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>>>>>> void __weak unpriv_ebpf_notify(int new_state)
>>>>>> ^
>>>>>> static
>>>>>>
>>>>>> Fixes: 2900005ea287 ("bpf: Move BPF sysctls from kernel/sysctl.c to BPF core")
>>>>>> Signed-off-by: Luis Chamberlain <mcgrof@...nel.org>
>>>>>> ---
>>>>>>
>>>>>> Daniel,
>>>>>>
>>>>>> Given what we did fore 2900005ea287 ("bpf: Move BPF sysctls from
>>>>>> kernel/sysctl.c to BPF core") where I had pulled pr/bpf-sysctl a
>>>>>> while ago into sysctl-next and then merged the patch in question,
>>>>>> should I just safely carry this patch onto sysctl-next? Let me know
>>>>>> how you'd like to proceed.
>>>>>>
>>>>>> Also, it wasn't clear if putting this forward declaration on
>>>>>> bpf.h was your ideal preference.
>>>>>
>>>>> After testing this on sysctl-testing without issues going to move this
>>>>> to sysctl-next now.
>>>>
>>>> Hmm. No.
>>>> A similar patch should be in tip already. You have to wait
>>>> for it to go through Linus's tree and back to whatever tree you use.
>>>
>>> I'm a bit confused, the patch in question which my patch fixes should only
>>> be in my sysctl-next tree at this point, not in Linus's tree.
>>
>> Borislav was planning to route it via tip tree, maybe confusion was that the
>> fix in the link below is from Josh:
>>
>> https://lore.kernel.org/bpf/CAADnVQKjfQMG_zFf9F9P7m0UzqESs7XoRy=udqrDSodxa8yBpg@mail.gmail.com/
> 
> Ah, Josh posted a fix for the same compile warning.
> 
>> But I presume this is routed as fix to Linus, so should land in both sysctl
>> and bpf tree at some point after re-sync.
> 
> It may be the case indeed that the code in question was triggering a
> compile warning without the patch I have merged which moves the BPF
> sysctls ("bpf: Move BPF sysctls from kernel/sysctl.c to BPF core").

Yes, it was indeed independent of the move.

> So I'll just drop my fix.

Agree, that's the best way forward, thanks Luis!