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@xxxxxxxxx>
>> Tested-by: Pengfei Xu <pengfei.xu@xxxxxxxxx>
>> ---
>> 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@xxxxxxxxx/
>>
>> 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
>>
