[PATCH] tools/perf: Fix bpf__probe to set bpf_prog_type type only if differs from the desired one

[PATCH] tools/perf: Fix bpf__probe to set bpf_prog_type type only if differs from the desired one

[Athira Rajeev]

The test "BPF prologue generation" fails as below:

   Writing event: p:perf_bpf_probe/func _text+10423200 f_mode=+20(%gpr3):x32 offset=%gpr4:s64 orig=%gpr5:s32
   In map_prologue, ntevs=1
   mapping[0]=0
   libbpf: prog 'bpf_func__null_lseek': BPF program load failed: Permission denied
   libbpf: prog 'bpf_func__null_lseek': -- BEGIN PROG LOAD LOG --
   btf_vmlinux is malformed
   reg type unsupported for arg#0 function bpf_func__null_lseek#5
   0: R1=ctx(off=0,imm=0) R10=fp0
   ;
   0: (57) r3 &= 2
   R3 !read_ok
   processed 1 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
   -- END PROG LOAD LOG --
   libbpf: prog 'bpf_func__null_lseek': failed to load: -13
   libbpf: failed to load object '[bpf_prologue_test]'
   bpf: load objects failed: err=-13: (Permission denied)
   Failed to add events selected by BPF

This fails occurs after this commit:
commit d6e6286a12e7 ("libbpf: disassociate section handler
on explicit bpf_program__set_type() call")'

With this change, SEC_DEF handler libbpf which is determined
initially based on program's SEC() is set to NULL. The change
is made because sec_def is not valid when user sets the program
type with bpf_program__set_type function. This commit also fixed
bpf_prog_test_load() helper in selftests/bpf to force-set program
type only if it differs from the desired one.

The "bpf__probe" function in util/bpf-loader.c, also calls
bpf_program__set_type to set bpf_prog_type. Add similar fix in
here as well to avoid setting sec_def to NULL.

Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
 tools/perf/util/bpf-loader.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index 44cde27d6389..b8e0b430e302 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -870,12 +870,14 @@ int bpf__probe(struct bpf_object *obj)
 			goto out;
 		}
 
-		if (priv->is_tp) {
+		if (priv->is_tp && bpf_program__type(prog) != BPF_PROG_TYPE_TRACEPOINT) {
 			bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
 			continue;
 		}
 
-		bpf_program__set_type(prog, BPF_PROG_TYPE_KPROBE);
+		if (bpf_program__type(prog) != BPF_PROG_TYPE_KPROBE)
+			bpf_program__set_type(prog, BPF_PROG_TYPE_KPROBE);
+
 		pev = &priv->pev;
 
 		err = convert_perf_probe_events(pev, 1);
-- 
2.39.3

Re: [PATCH] tools/perf: Fix bpf__probe to set bpf_prog_type type only if differs from the desired one

[Sachin Sant]

> On 07-Aug-2023, at 10:22 AM, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
> 
> The test "BPF prologue generation" fails as below:
> 
>   Writing event: p:perf_bpf_probe/func _text+10423200 f_mode=+20(%gpr3):x32 offset=%gpr4:s64 orig=%gpr5:s32
>   In map_prologue, ntevs=1
>   mapping[0]=0
>   libbpf: prog 'bpf_func__null_lseek': BPF program load failed: Permission denied
>   libbpf: prog 'bpf_func__null_lseek': -- BEGIN PROG LOAD LOG --
>   btf_vmlinux is malformed
>   reg type unsupported for arg#0 function bpf_func__null_lseek#5
>   0: R1=ctx(off=0,imm=0) R10=fp0
>   ;
>   0: (57) r3 &= 2
>   R3 !read_ok
>   processed 1 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
>   -- END PROG LOAD LOG --
>   libbpf: prog 'bpf_func__null_lseek': failed to load: -13
>   libbpf: failed to load object '[bpf_prologue_test]'
>   bpf: load objects failed: err=-13: (Permission denied)
>   Failed to add events selected by BPF
> 
> This fails occurs after this commit:
> commit d6e6286a12e7 ("libbpf: disassociate section handler
> on explicit bpf_program__set_type() call")'
> 
> With this change, SEC_DEF handler libbpf which is determined
> initially based on program's SEC() is set to NULL. The change
> is made because sec_def is not valid when user sets the program
> type with bpf_program__set_type function. This commit also fixed
> bpf_prog_test_load() helper in selftests/bpf to force-set program
> type only if it differs from the desired one.
> 
> The "bpf__probe" function in util/bpf-loader.c, also calls
> bpf_program__set_type to set bpf_prog_type. Add similar fix in
> here as well to avoid setting sec_def to NULL.
> 
> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---

Thanks Athira for the fix.
With this patch applied perf BPF prologue sub test works correctly.

 42: BPF filter                                                     :
 42.1: Basic BPF filtering                                    : Ok
 42.2: BPF pinning                                              : Ok
 42.3: BPF prologue generation                          : Ok

Tested-by: Sachin Sant <sachinp@linux.ibm.com>

Can you please use the above mentioned id(without vnet) in the reported-by ?

- Sachin

Re: [PATCH] tools/perf: Fix bpf__probe to set bpf_prog_type type only if differs from the desired one

[Athira Rajeev]

> On 07-Aug-2023, at 11:07 AM, Sachin Sant <sachinp@linux.ibm.com> wrote:
> 
> 
> 
>> On 07-Aug-2023, at 10:22 AM, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
>> 
>> The test "BPF prologue generation" fails as below:
>> 
>>  Writing event: p:perf_bpf_probe/func _text+10423200 f_mode=+20(%gpr3):x32 offset=%gpr4:s64 orig=%gpr5:s32
>>  In map_prologue, ntevs=1
>>  mapping[0]=0
>>  libbpf: prog 'bpf_func__null_lseek': BPF program load failed: Permission denied
>>  libbpf: prog 'bpf_func__null_lseek': -- BEGIN PROG LOAD LOG --
>>  btf_vmlinux is malformed
>>  reg type unsupported for arg#0 function bpf_func__null_lseek#5
>>  0: R1=ctx(off=0,imm=0) R10=fp0
>>  ;
>>  0: (57) r3 &= 2
>>  R3 !read_ok
>>  processed 1 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
>>  -- END PROG LOAD LOG --
>>  libbpf: prog 'bpf_func__null_lseek': failed to load: -13
>>  libbpf: failed to load object '[bpf_prologue_test]'
>>  bpf: load objects failed: err=-13: (Permission denied)
>>  Failed to add events selected by BPF
>> 
>> This fails occurs after this commit:
>> commit d6e6286a12e7 ("libbpf: disassociate section handler
>> on explicit bpf_program__set_type() call")'
>> 
>> With this change, SEC_DEF handler libbpf which is determined
>> initially based on program's SEC() is set to NULL. The change
>> is made because sec_def is not valid when user sets the program
>> type with bpf_program__set_type function. This commit also fixed
>> bpf_prog_test_load() helper in selftests/bpf to force-set program
>> type only if it differs from the desired one.
>> 
>> The "bpf__probe" function in util/bpf-loader.c, also calls
>> bpf_program__set_type to set bpf_prog_type. Add similar fix in
>> here as well to avoid setting sec_def to NULL.
>> 
>> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>> ---
> 
> Thanks Athira for the fix.
> With this patch applied perf BPF prologue sub test works correctly.
> 
> 42: BPF filter                                                     :
> 42.1: Basic BPF filtering                                    : Ok
> 42.2: BPF pinning                                              : Ok
> 42.3: BPF prologue generation                          : Ok
> 
> Tested-by: Sachin Sant <sachinp@linux.ibm.com>
> 
> Can you please use the above mentioned id(without vnet) in the reported-by ?
> 
> - Sachin

Hi All,

Looking for review comments on this patch

Athira

Re: [PATCH] tools/perf: Fix bpf__probe to set bpf_prog_type type only if differs from the desired one

[Ian Rogers]

On Thu, Aug 17, 2023 at 10:35 AM Athira Rajeev
<atrajeev@linux.vnet.ibm.com> wrote:
>
>
>
> > On 07-Aug-2023, at 11:07 AM, Sachin Sant <sachinp@linux.ibm.com> wrote:
> >
> >
> >
> >> On 07-Aug-2023, at 10:22 AM, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
> >>
> >> The test "BPF prologue generation" fails as below:
> >>
> >>  Writing event: p:perf_bpf_probe/func _text+10423200 f_mode=+20(%gpr3):x32 offset=%gpr4:s64 orig=%gpr5:s32
> >>  In map_prologue, ntevs=1
> >>  mapping[0]=0
> >>  libbpf: prog 'bpf_func__null_lseek': BPF program load failed: Permission denied
> >>  libbpf: prog 'bpf_func__null_lseek': -- BEGIN PROG LOAD LOG --
> >>  btf_vmlinux is malformed
> >>  reg type unsupported for arg#0 function bpf_func__null_lseek#5
> >>  0: R1=ctx(off=0,imm=0) R10=fp0
> >>  ;
> >>  0: (57) r3 &= 2
> >>  R3 !read_ok
> >>  processed 1 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
> >>  -- END PROG LOAD LOG --
> >>  libbpf: prog 'bpf_func__null_lseek': failed to load: -13
> >>  libbpf: failed to load object '[bpf_prologue_test]'
> >>  bpf: load objects failed: err=-13: (Permission denied)
> >>  Failed to add events selected by BPF
> >>
> >> This fails occurs after this commit:
> >> commit d6e6286a12e7 ("libbpf: disassociate section handler
> >> on explicit bpf_program__set_type() call")'
> >>
> >> With this change, SEC_DEF handler libbpf which is determined
> >> initially based on program's SEC() is set to NULL. The change
> >> is made because sec_def is not valid when user sets the program
> >> type with bpf_program__set_type function. This commit also fixed
> >> bpf_prog_test_load() helper in selftests/bpf to force-set program
> >> type only if it differs from the desired one.
> >>
> >> The "bpf__probe" function in util/bpf-loader.c, also calls
> >> bpf_program__set_type to set bpf_prog_type. Add similar fix in
> >> here as well to avoid setting sec_def to NULL.
> >>
> >> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> >> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> >> ---
> >
> > Thanks Athira for the fix.
> > With this patch applied perf BPF prologue sub test works correctly.
> >
> > 42: BPF filter                                                     :
> > 42.1: Basic BPF filtering                                    : Ok
> > 42.2: BPF pinning                                              : Ok
> > 42.3: BPF prologue generation                          : Ok
> >
> > Tested-by: Sachin Sant <sachinp@linux.ibm.com>
> >
> > Can you please use the above mentioned id(without vnet) in the reported-by ?
> >
> > - Sachin
>
> Hi All,
>
> Looking for review comments on this patch
>
> Athira

Hi,

the patch set:
https://lore.kernel.org/lkml/20230810184853.2860737-1-irogers@google.com/
removes the affected code/test.

Thanks,
Ian