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;
MIME-Version: 1.0
References: <20221122073244.21279-1-hu1.chen@intel.com> <Y3zTF0CjQFt/dR2M@krava>
 <Y3zZQpHNQ8cRjKQY@hirez.programming.kicks-ass.net> <d6555f27-95bf-5474-3006-6f8d399ab556@intel.com>
 <Y4PfBZpdZL00tDMu@krava>
In-Reply-To: <Y4PfBZpdZL00tDMu@krava>
From:   Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date:   Sun, 27 Nov 2022 14:13:30 -0800
Message-ID: <CAADnVQJ4xaAacOUpzMG+bm2WK5u=1YLo5kLUL+RP3JZGW3Sfww@mail.gmail.com>
Subject: Re: [PATCH bpf v2] selftests/bpf: Fix "missing ENDBR" BUG for
 destructor kfunc
To:     Jiri Olsa <olsajiri@gmail.com>
Cc:     "Chen, Hu1" <hu1.chen@intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Josh Poimboeuf <jpoimboe@kernel.org>,
        Kumar Kartikeya Dwivedi <memxor@gmail.com>,
        bpf <bpf@vger.kernel.org>, Pengfei Xu <pengfei.xu@intel.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>,
        Stanislav Fomichev <sdf@google.com>,
        Hao Luo <haoluo@google.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Network Development <netdev@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sun, Nov 27, 2022 at 2:05 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Fri, Nov 25, 2022 at 09:44:29PM +0800, Chen, Hu1 wrote:
> > On 11/22/2022 10:14 PM, Peter Zijlstra wrote:
> > > On Tue, Nov 22, 2022 at 02:48:07PM +0100, Jiri Olsa wrote:
> > >> On Mon, Nov 21, 2022 at 11:32:43PM -0800, Chen Hu wrote:
> > >>> With CONFIG_X86_KERNEL_IBT enabled, the test_verifier triggers the
> > >>> following BUG:
> > >>>
> > >>>   traps: Missing ENDBR: bpf_kfunc_call_test_release+0x0/0x30
> > >>>   ------------[ cut here ]------------
> > >>>   kernel BUG at arch/x86/kernel/traps.c:254!
> > >>>   invalid opcode: 0000 [#1] PREEMPT SMP
> > >>>   <TASK>
> > >>>    asm_exc_control_protection+0x26/0x50
> > >>>   RIP: 0010:bpf_kfunc_call_test_release+0x0/0x30
> > >>>   Code: 00 48 c7 c7 18 f2 e1 b4 e8 0d ca 8c ff 48 c7 c0 00 f2 e1 b4 c3
> > >>>   0f 1f 44 00 00 66 0f 1f 00 0f 1f 44 00 00 0f 0b 31 c0 c3 66 90
> > >>>        <66> 0f 1f 00 0f 1f 44 00 00 48 85 ff 74 13 4c 8d 47 18 b8 ff ff ff
> > >>>    bpf_map_free_kptrs+0x2e/0x70
> > >>>    array_map_free+0x57/0x140
> > >>>    process_one_work+0x194/0x3a0
> > >>>    worker_thread+0x54/0x3a0
> > >>>    ? rescuer_thread+0x390/0x390
> > >>>    kthread+0xe9/0x110
> > >>>    ? kthread_complete_and_exit+0x20/0x20
> > >>>
> > >>> This is because there are no compile-time references to the destructor
> > >>> kfuncs, bpf_kfunc_call_test_release() for example. So objtool marked
> > >>> them sealable and ENDBR in the functions were sealed (converted to NOP)
> > >>> by apply_ibt_endbr().
> > >
> > > If there is no compile time reference to it, what stops an LTO linker
> > > from throwing it out in the first place?
> > >
> >
> > Ah, my stupid.
> >
> > The only references to this function from kernel space are:
> >     $ grep -r bpf_kfunc_call_test_release
> >     net/bpf/test_run.c:noinline void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p)
> >     net/bpf/test_run.c:BTF_ID_FLAGS(func, bpf_kfunc_call_test_release, KF_RELEASE)
> >     net/bpf/test_run.c:BTF_ID(func, bpf_kfunc_call_test_release)
> >
> > Macro BTF_ID_... puts the function names to .BTF_ids section. It looks
> > like:
> > __BTF_ID__func__bpf_kfunc_call_test_release__692
>
> bpf_kfunc_call_test_release test function called bpf program as kfunc
> (check tools/testing/selftests/bpf/progs/*.c)
>
> it's placed in BTF ID lists so verifier can validate its ID when called
> from bpf program.. it has no other caller from kernel side

They were added when we had no ability to call kfuncs from modules.
Now we should probably move all of them to bpf_testmod.