| Message ID | 20221121085113.611504-1-hu1.chen@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf] selftests/bpf: Fix "missing ENDBR" BUG for destructor kfunc | expand |
On Mon, Nov 21, 2022 at 12:51:13AM -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(). nice :) thanks for the fix, some suggestions below > > This fix creates dummy compile-time references to destructor kfuncs so > ENDBR stay there. > > Signed-off-by: Chen Hu <hu1.chen@intel.com> > Tested-by: Pengfei Xu <pengfei.xu@intel.com> > --- > include/linux/btf_ids.h | 7 +++++++ > net/bpf/test_run.c | 2 ++ > 2 files changed, 9 insertions(+) > > diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h > index 2aea877d644f..6c6b520ea58f 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 BTF_IBT_NOSEAL(name) \ > + asm(IBT_NOSEAL(#name)); > +#else > +#define BTF_IBT_NOSEAL(name) > +#endif /* CONFIG_X86_KERNEL_IBT */ this is not BTF or BTF ID specific, instead should we add some generic macro like: FUNC_IBT_NOSEAL(...) > + > #endif > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c > index 13d578ce2a09..465952e5de11 100644 > --- a/net/bpf/test_run.c > +++ b/net/bpf/test_run.c > @@ -1653,6 +1653,8 @@ BTF_ID(struct, prog_test_ref_kfunc) > BTF_ID(func, bpf_kfunc_call_test_release) > BTF_ID(struct, prog_test_member) > BTF_ID(func, bpf_kfunc_call_memb_release) > +BTF_IBT_NOSEAL(bpf_kfunc_call_test_release) > +BTF_IBT_NOSEAL(bpf_kfunc_call_memb_release) same here, it looks like it's part of the list above, I think this would be better after function body like: noinline void bpf_kfunc_call_memb_release(struct prog_test_member *p) { } FUNC_IBT_NOSEAL(bpf_kfunc_call_memb_release) thanks, jirka
diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h index 2aea877d644f..6c6b520ea58f 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 BTF_IBT_NOSEAL(name) \ + asm(IBT_NOSEAL(#name)); +#else +#define BTF_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..465952e5de11 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -1653,6 +1653,8 @@ BTF_ID(struct, prog_test_ref_kfunc) BTF_ID(func, bpf_kfunc_call_test_release) BTF_ID(struct, prog_test_member) BTF_ID(func, bpf_kfunc_call_memb_release) +BTF_IBT_NOSEAL(bpf_kfunc_call_test_release) +BTF_IBT_NOSEAL(bpf_kfunc_call_memb_release) static int __init bpf_prog_test_run_init(void) {
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. Signed-off-by: Chen Hu <hu1.chen@intel.com> Tested-by: Pengfei Xu <pengfei.xu@intel.com> --- include/linux/btf_ids.h | 7 +++++++ net/bpf/test_run.c | 2 ++ 2 files changed, 9 insertions(+)