Messages in this thread | | | Subject | Re: [PATCH] bpf.h: fix clang compiler warning with unpriv_ebpf_notify() | From | Daniel Borkmann <> | Date | Wed, 11 May 2022 18:38:14 +0200 |
| |
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@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/ > > 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!
| |