Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH] bpf.h: fix clang compiler warning with
 unpriv_ebpf_notify()
To:     Luis Chamberlain <mcgrof@kernel.org>,
        Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc:     Alexei Starovoitov <ast@kernel.org>,
        Song Liu <songliubraving@fb.com>, bpf <bpf@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>, llvm@lists.linux.dev,
        Borislav Petkov <bp@alien8.de>
References: <20220509203623.3856965-1-mcgrof@kernel.org>
 <YnvdOAaYmhNiA5WN@bombadil.infradead.org>
 <CAADnVQLCvjqphpJDkz-5bpJLs3k_PRH1JcwehCRLrWYvsA9ENw@mail.gmail.com>
 <YnvflsM1t5vL/ViP@bombadil.infradead.org>
From:   Daniel Borkmann <daniel@iogearbox.net>
Message-ID: <3e3ed3d1-937b-a715-376d-43a8b7485f68@iogearbox.net>
Date:   Wed, 11 May 2022 18:17:26 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
MIME-Version: 1.0
In-Reply-To: <YnvflsM1t5vL/ViP@bombadil.infradead.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@kernel.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@kernel.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/

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.

Thanks,
Daniel