[PATCH bpf v2] selftests/bpf: Fix "missing ENDBR" BUG for destructor kfunc

[PATCH bpf v2] selftests/bpf: Fix "missing ENDBR" BUG for destructor kfunc

[Chen Hu]

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 */
+
 #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

Re: [PATCH bpf v2] selftests/bpf: Fix "missing ENDBR" BUG for destructor kfunc

[Jiri Olsa]

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
> 

Re: [PATCH bpf v2] selftests/bpf: Fix "missing ENDBR" BUG for destructor kfunc

[Peter Zijlstra]

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?

Re: [PATCH bpf v2] selftests/bpf: Fix "missing ENDBR" BUG for destructor kfunc

[Chen, Hu1]

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.

Re: [PATCH bpf v2] selftests/bpf: Fix "missing ENDBR" BUG for destructor kfunc

[Jiri Olsa]

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.

Re: [PATCH bpf v2] selftests/bpf: Fix "missing ENDBR" BUG for destructor kfunc

[Alexei Starovoitov]

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.

Re: [PATCH bpf v2] selftests/bpf: Fix "missing ENDBR" BUG for destructor kfunc

[Chen, Hu1]

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
>>

Re: [PATCH bpf v2] selftests/bpf: Fix "missing ENDBR" BUG for destructor kfunc

[Jiri Olsa]

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
> >>