Message ID | 20221122073244.21279-1-hu1.chen@intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp2058676wrr; Mon, 21 Nov 2022 23:40:03 -0800 (PST) X-Google-Smtp-Source: AA0mqf53zRsz2JOMCbzd72OTk5aefl/7NAqAB+CIfdVcyqTc+aDi8Sy8uNhcrHpjzjaS2A7R75WJ X-Received: by 2002:a17:90b:484:b0:218:9d3d:71f4 with SMTP id bh4-20020a17090b048400b002189d3d71f4mr13984478pjb.148.1669102802721; Mon, 21 Nov 2022 23:40:02 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1669102802; cv=none; d=google.com; s=arc-20160816; b=gQ/JCwx1FLXkjabp5/XEQk7/JN18wKwe3B/KfgzoNJHE8sgrKhLo1wj8QZi2SD4tXJ Id8QreDVOAlvX0TX+RgjpazAOJH9zcXfv1QJrbcpr4A+EaZ2uAdfBr7ONyvEfJYEvT7I FeM4bcTfe6OlxkIFntgnL09/iHq7b31ktIHupZnxAaAe/2jaq8T1ype/MXN1ROSUKQYf UymqAX2BbG573yYHJ4ZEb+vlxx5b0YXuW8jVSmBgE/QLQz392x7E1t9QmiXsgfAhrM90 JhMn5JHq9YNkHA3orORgAkBfKTgd0Ao7JQtY1rUVpw6gpGi7UDhvFA38jmgcWmlLIoXV BJ7w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:to:content-transfer-encoding:mime-version :message-id:date:subject:cc:from:dkim-signature; bh=tHE6DtffWBby0yKkoaPtqd4U3Hw0TZJU+RtTeXgIQAs=; b=wANikyJcsLRQ19F+8mlZNbCPg3ckKoPVic5zy1WjYnU/Z6Al/oKA10FxRn1sRPZTgt Ekc/EYeako/vGE8+LJD+ASe67DhYA572zIoUgqwKNN54Y3G29FdLA22HEwhTbEfv9Nnx QvvLzdD6SLE+gArXIryLaZrIX83qYtYOlkZOuS+cqzVL9jO0DrtL9zN7gtFH7P9xXtg0 P4s88eYwP8ueyymZxJdi+a7vkNLdQ3ABIcJJ/tCA7PgnrfUHJcZn0woJzU0TJn97bGjR lvd3vlU0oHRJic/ka9v0M6RJlPP/Amd+1uLHywDJm+OXtk5CWPsYjKzVLzM9dpLNgvuk 85jA== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@intel.com header.s=Intel header.b=UkZN1cTz; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id m3-20020a170902db0300b001872cd80411si15033652plx.193.2022.11.21.23.39.48; Mon, 21 Nov 2022 23:40:02 -0800 (PST) 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; Authentication-Results: mx.google.com; dkim=fail header.i=@intel.com header.s=Intel header.b=UkZN1cTz; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232370AbiKVHeB (ORCPT <rfc822;cjcooper78@gmail.com> + 99 others); Tue, 22 Nov 2022 02:34:01 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46214 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232360AbiKVHd6 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 22 Nov 2022 02:33:58 -0500 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A26CC17057; Mon, 21 Nov 2022 23:33:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1669102437; x=1700638437; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=GlheoP3S8KQYlEN0ZjlBFqE/Uf9kh5WCBdTEusKSsHE=; b=UkZN1cTzD5/gIu4fr20ZcOanEOieuyFkWTXjlZVsGBwjet9KFdWnfZwk PJoplC4nONg5gS8402cQu2f1nDOxoPwG/nZSR1FxKEntfOU6iQgh+il9L nBCr3mWmRs8NTB5YD5b+c/0ILWayXey164ZP+94721uiGbDIOZWoZi8Ez 6aEfDF9fSjSNQ8lqupG/KZQ9aMkHt9MTM/+1gFlNgvbEjj5v6DI/G1SlX lKdJW0M6CRgeOmiMGX7/QFoK74p8reePa61x9ejXZ2PTnbcR2TwFXpHoZ 6I1AEV7Pt8tavFQ9oEyvrvacrtRsFtcCSROb8AITW6gUfLWKkfbypxHB+ A==; X-IronPort-AV: E=McAfee;i="6500,9779,10538"; a="313785041" X-IronPort-AV: E=Sophos;i="5.96,183,1665471600"; d="scan'208";a="313785041" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Nov 2022 23:33:57 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10538"; a="641323236" X-IronPort-AV: E=Sophos;i="5.96,183,1665471600"; d="scan'208";a="641323236" Received: from a0cec8d9c8fc.jf.intel.com ([10.45.77.137]) by orsmga002.jf.intel.com with ESMTP; 21 Nov 2022 23:33:56 -0800 From: Chen Hu <hu1.chen@intel.com> Cc: hu1.chen@intel.com, jpoimboe@kernel.org, memxor@gmail.com, 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>, Jiri Olsa <jolsa@kernel.org>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, linux-kernel@vger.kernel.org, netdev@vger.kernel.org Subject: [PATCH bpf v2] selftests/bpf: Fix "missing ENDBR" BUG for destructor kfunc Date: Mon, 21 Nov 2022 23:32:43 -0800 Message-Id: <20221122073244.21279-1-hu1.chen@intel.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net To: unlisted-recipients:; (no To-header on input) Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1750181140426171857?= X-GMAIL-MSGID: =?utf-8?q?1750181140426171857?= |
Series |
[bpf,v2] selftests/bpf: Fix "missing ENDBR" BUG for destructor kfunc
|
|
Commit Message
Chen Hu
Nov. 22, 2022, 7:32 a.m. UTC
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(). This fix creates dummy compile-time references to destructor kfuncs so ENDBR stay there. Fixes: 05a945deefaa ("selftests/bpf: Add verifier tests for kptr") Signed-off-by: Chen Hu <hu1.chen@intel.com> Tested-by: Pengfei Xu <pengfei.xu@intel.com> --- v2: - Use generic macro name and place the macro after function body as - suggested by Jiri Olsa v1: https://lore.kernel.org/all/20221121085113.611504-1-hu1.chen@intel.com/ include/linux/btf_ids.h | 7 +++++++ net/bpf/test_run.c | 4 ++++ 2 files changed, 11 insertions(+)
Comments
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(). > > This fix creates dummy compile-time references to destructor kfuncs so > ENDBR stay there. > > Fixes: 05a945deefaa ("selftests/bpf: Add verifier tests for kptr") > Signed-off-by: Chen Hu <hu1.chen@intel.com> > Tested-by: Pengfei Xu <pengfei.xu@intel.com> > --- > v2: > - Use generic macro name and place the macro after function body as > - suggested by Jiri Olsa > > v1: https://lore.kernel.org/all/20221121085113.611504-1-hu1.chen@intel.com/ > > include/linux/btf_ids.h | 7 +++++++ > net/bpf/test_run.c | 4 ++++ > 2 files changed, 11 insertions(+) > > diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h > index 2aea877d644f..db02691b506d 100644 > --- a/include/linux/btf_ids.h > +++ b/include/linux/btf_ids.h > @@ -266,4 +266,11 @@ MAX_BTF_TRACING_TYPE, > > extern u32 btf_tracing_ids[]; > > +#if defined(CONFIG_X86_KERNEL_IBT) && !defined(__DISABLE_EXPORTS) > +#define FUNC_IBT_NOSEAL(name) \ > + asm(IBT_NOSEAL(#name)); > +#else > +#define FUNC_IBT_NOSEAL(name) > +#endif /* CONFIG_X86_KERNEL_IBT */ hum, IBT_NOSEAL is x86 specific, so this will probably fail build on other archs.. I think we could ifdef it with CONFIG_X86, but it should go to some IBT related header? surely not to btf_ids.h cc-ing Peter and Josh thanks, jirka > + > #endif > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c > index 13d578ce2a09..07263b7cc12d 100644 > --- a/net/bpf/test_run.c > +++ b/net/bpf/test_run.c > @@ -597,10 +597,14 @@ noinline void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) > refcount_dec(&p->cnt); > } > > +FUNC_IBT_NOSEAL(bpf_kfunc_call_test_release) > + > noinline void bpf_kfunc_call_memb_release(struct prog_test_member *p) > { > } > > +FUNC_IBT_NOSEAL(bpf_kfunc_call_memb_release) > + > noinline void bpf_kfunc_call_memb1_release(struct prog_test_member1 *p) > { > WARN_ON_ONCE(1); > -- > 2.34.1 >
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?
On 11/22/2022 9:48 PM, 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(). >> >> This fix creates dummy compile-time references to destructor kfuncs so >> ENDBR stay there. >> >> Fixes: 05a945deefaa ("selftests/bpf: Add verifier tests for kptr") >> Signed-off-by: Chen Hu <hu1.chen@intel.com> >> Tested-by: Pengfei Xu <pengfei.xu@intel.com> >> --- >> v2: >> - Use generic macro name and place the macro after function body as >> - suggested by Jiri Olsa >> >> v1: https://lore.kernel.org/all/20221121085113.611504-1-hu1.chen@intel.com/ >> >> include/linux/btf_ids.h | 7 +++++++ >> net/bpf/test_run.c | 4 ++++ >> 2 files changed, 11 insertions(+) >> >> diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h >> index 2aea877d644f..db02691b506d 100644 >> --- a/include/linux/btf_ids.h >> +++ b/include/linux/btf_ids.h >> @@ -266,4 +266,11 @@ MAX_BTF_TRACING_TYPE, >> >> extern u32 btf_tracing_ids[]; >> >> +#if defined(CONFIG_X86_KERNEL_IBT) && !defined(__DISABLE_EXPORTS) >> +#define FUNC_IBT_NOSEAL(name) \ >> + asm(IBT_NOSEAL(#name)); >> +#else >> +#define FUNC_IBT_NOSEAL(name) >> +#endif /* CONFIG_X86_KERNEL_IBT */ > > hum, IBT_NOSEAL is x86 specific, so this will probably fail build > on other archs.. I think we could ifdef it with CONFIG_X86, but > it should go to some IBT related header? surely not to btf_ids.h > > cc-ing Peter and Josh > > thanks, > jirka > The lkp reports build success because X86_KERNEL_IBT alredy depends on X86_64. Currently, arch/x86/include/asm/ibt.h which defines macro IBT_NOSEAL is x86 specific. How about we just put asm at test_run.c directly (ugly?): #if defined(CONFIG_X86_KERNEL_IBT) && !defined(__DISABLE_EXPORTS) asm(IBT_NOSEAL("bpf_kfunc_call_test_release")); asm(IBT_NOSEAL("bpf_kfunc_call_memb_release")); #endif thanks Chen Hu > >> + >> #endif >> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c >> index 13d578ce2a09..07263b7cc12d 100644 >> --- a/net/bpf/test_run.c >> +++ b/net/bpf/test_run.c >> @@ -597,10 +597,14 @@ noinline void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) >> refcount_dec(&p->cnt); >> } >> >> +FUNC_IBT_NOSEAL(bpf_kfunc_call_test_release) >> + >> noinline void bpf_kfunc_call_memb_release(struct prog_test_member *p) >> { >> } >> >> +FUNC_IBT_NOSEAL(bpf_kfunc_call_memb_release) >> + >> noinline void bpf_kfunc_call_memb1_release(struct prog_test_member1 *p) >> { >> WARN_ON_ONCE(1); >> -- >> 2.34.1 >>
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 When running, it uses kallsyms_lookup_name() to find the function address via names in .BTF_ids section. Hi jirka, Please kindly correct me if my understanding of BTF_ids is wrong.
On Fri, Nov 25, 2022 at 09:28:28PM +0800, Chen, Hu1 wrote: > On 11/22/2022 9:48 PM, 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(). > >> > >> This fix creates dummy compile-time references to destructor kfuncs so > >> ENDBR stay there. > >> > >> Fixes: 05a945deefaa ("selftests/bpf: Add verifier tests for kptr") > >> Signed-off-by: Chen Hu <hu1.chen@intel.com> > >> Tested-by: Pengfei Xu <pengfei.xu@intel.com> > >> --- > >> v2: > >> - Use generic macro name and place the macro after function body as > >> - suggested by Jiri Olsa > >> > >> v1: https://lore.kernel.org/all/20221121085113.611504-1-hu1.chen@intel.com/ > >> > >> include/linux/btf_ids.h | 7 +++++++ > >> net/bpf/test_run.c | 4 ++++ > >> 2 files changed, 11 insertions(+) > >> > >> diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h > >> index 2aea877d644f..db02691b506d 100644 > >> --- a/include/linux/btf_ids.h > >> +++ b/include/linux/btf_ids.h > >> @@ -266,4 +266,11 @@ MAX_BTF_TRACING_TYPE, > >> > >> extern u32 btf_tracing_ids[]; > >> > >> +#if defined(CONFIG_X86_KERNEL_IBT) && !defined(__DISABLE_EXPORTS) > >> +#define FUNC_IBT_NOSEAL(name) \ > >> + asm(IBT_NOSEAL(#name)); > >> +#else > >> +#define FUNC_IBT_NOSEAL(name) > >> +#endif /* CONFIG_X86_KERNEL_IBT */ > > > > hum, IBT_NOSEAL is x86 specific, so this will probably fail build > > on other archs.. I think we could ifdef it with CONFIG_X86, but > > it should go to some IBT related header? surely not to btf_ids.h > > > > cc-ing Peter and Josh > > > > thanks, > > jirka > > > > The lkp reports build success because X86_KERNEL_IBT alredy depends on > X86_64. ah right, so please just move it to some other header jirka > > Currently, arch/x86/include/asm/ibt.h which defines macro IBT_NOSEAL is > x86 specific. How about we just put asm at test_run.c directly (ugly?): > > #if defined(CONFIG_X86_KERNEL_IBT) && !defined(__DISABLE_EXPORTS) > asm(IBT_NOSEAL("bpf_kfunc_call_test_release")); > asm(IBT_NOSEAL("bpf_kfunc_call_memb_release")); > #endif > > thanks > Chen Hu > > > > >> + > >> #endif > >> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c > >> index 13d578ce2a09..07263b7cc12d 100644 > >> --- a/net/bpf/test_run.c > >> +++ b/net/bpf/test_run.c > >> @@ -597,10 +597,14 @@ noinline void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) > >> refcount_dec(&p->cnt); > >> } > >> > >> +FUNC_IBT_NOSEAL(bpf_kfunc_call_test_release) > >> + > >> noinline void bpf_kfunc_call_memb_release(struct prog_test_member *p) > >> { > >> } > >> > >> +FUNC_IBT_NOSEAL(bpf_kfunc_call_memb_release) > >> + > >> noinline void bpf_kfunc_call_memb1_release(struct prog_test_member1 *p) > >> { > >> WARN_ON_ONCE(1); > >> -- > >> 2.34.1 > >>
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 jirka > > When running, it uses kallsyms_lookup_name() to find the function > address via names in .BTF_ids section. > > > Hi jirka, > Please kindly correct me if my understanding of BTF_ids is wrong.
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.
diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h index 2aea877d644f..db02691b506d 100644 --- a/include/linux/btf_ids.h +++ b/include/linux/btf_ids.h @@ -266,4 +266,11 @@ MAX_BTF_TRACING_TYPE, extern u32 btf_tracing_ids[]; +#if defined(CONFIG_X86_KERNEL_IBT) && !defined(__DISABLE_EXPORTS) +#define FUNC_IBT_NOSEAL(name) \ + asm(IBT_NOSEAL(#name)); +#else +#define FUNC_IBT_NOSEAL(name) +#endif /* CONFIG_X86_KERNEL_IBT */ + #endif diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index 13d578ce2a09..07263b7cc12d 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -597,10 +597,14 @@ noinline void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) refcount_dec(&p->cnt); } +FUNC_IBT_NOSEAL(bpf_kfunc_call_test_release) + noinline void bpf_kfunc_call_memb_release(struct prog_test_member *p) { } +FUNC_IBT_NOSEAL(bpf_kfunc_call_memb_release) + noinline void bpf_kfunc_call_memb1_release(struct prog_test_member1 *p) { WARN_ON_ONCE(1);