Ideal way to read FUNC_PROTO in raw tp?

Ideal way to read FUNC_PROTO in raw tp?

[Yan Zhai]

Hi,

 I am building a tracing program around workqueue. But I encountered
following problem when I try to record a function pointer value from
trace_workqueue_execute_end on net-next kernel:

...
libbpf: prog 'workqueue_end': BPF program load failed: Permission
denied
libbpf: prog 'workqueue_end': -- BEGIN PROG LOAD LOG --
reg type unsupported for arg#0 function workqueue_end#5
0: R1=ctx(off=0,imm=0) R10=fp0
; int BPF_PROG(workqueue_end, struct work_struct *w, work_func_t f)
0: (79) r3 = *(u64 *)(r1 +8)
func 'workqueue_execute_end' arg1 type FUNC_PROTO is not a struct
invalid bpf_context access off=8 size=8
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 'workqueue_end': failed to load: -13
libbpf: failed to load object 'configs/test.bpf.o'
Error: failed to load object file
Warning: bpftool is now running in libbpf strict mode and has more
stringent requirements about BPF programs.
If it used to work for this object file but now doesn't, see --legacy
option for more details.
...

A simple reproducer for me is like:
#include "vmlinux.h"
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>

SEC("tp_btf/workqueue_execute_end")
int BPF_PROG(workqueue_end, struct work_struct *w, work_func_t f)
{
        u64 addr = (u64) f;
        bpf_printk("f is %lu\n", addr);

        return 0;
}

char LICENSE[] SEC("license") = "GPL";

I would like to use the function address to decode the kernel symbol
and track execution of these functions. Replacing raw tp to regular tp
solves the problem, but I am wondering if there is any go-to approach
to read the pointer value in a raw tp? Doesn't seem to find one in
selftests/samples. If not, does it make sense if we allow it in
the verifier for tracing programs like the attached patch?

Yan

---
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 821063660d9f..5f000ab4c8d0 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6308,6 +6308,11 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
                        __btf_name_by_offset(btf, t->name_off),
                        btf_type_str(t));
                return false;
+       } else if (prog->type == BPF_PROG_TYPE_TRACING || prog->type == BPF_PROG_TYPE_RAW_TRACEPOINT) {
+               /* allow reading function pointer value from a tracing program */
+               const struct btf_type *pointed = btf_type_by_id(btf, t->type);
+               if (btf_type_is_func_proto(pointed))
+                       return true;
        }

        /* check for PTR_TO_RDONLY_BUF_OR_NULL or PTR_TO_RDWR_BUF_OR_NULL */

Re: Ideal way to read FUNC_PROTO in raw tp?

[Alexei Starovoitov]

On Thu, Jun 6, 2024 at 3:03 PM Yan Zhai <yan@cloudflare.com> wrote:
>
> Hi,
>
>  I am building a tracing program around workqueue. But I encountered
> following problem when I try to record a function pointer value from
> trace_workqueue_execute_end on net-next kernel:
>
> ...
> libbpf: prog 'workqueue_end': BPF program load failed: Permission
> denied
> libbpf: prog 'workqueue_end': -- BEGIN PROG LOAD LOG --
> reg type unsupported for arg#0 function workqueue_end#5
> 0: R1=ctx(off=0,imm=0) R10=fp0
> ; int BPF_PROG(workqueue_end, struct work_struct *w, work_func_t f)
> 0: (79) r3 = *(u64 *)(r1 +8)
> func 'workqueue_execute_end' arg1 type FUNC_PROTO is not a struct
> invalid bpf_context access off=8 size=8
> 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 'workqueue_end': failed to load: -13
> libbpf: failed to load object 'configs/test.bpf.o'
> Error: failed to load object file
> Warning: bpftool is now running in libbpf strict mode and has more
> stringent requirements about BPF programs.
> If it used to work for this object file but now doesn't, see --legacy
> option for more details.
> ...
>
> A simple reproducer for me is like:
> #include "vmlinux.h"
> #include <bpf/bpf_helpers.h>
> #include <bpf/bpf_tracing.h>
>
> SEC("tp_btf/workqueue_execute_end")
> int BPF_PROG(workqueue_end, struct work_struct *w, work_func_t f)
> {
>         u64 addr = (u64) f;
>         bpf_printk("f is %lu\n", addr);
>
>         return 0;
> }
>
> char LICENSE[] SEC("license") = "GPL";
>
> I would like to use the function address to decode the kernel symbol
> and track execution of these functions. Replacing raw tp to regular tp
> solves the problem, but I am wondering if there is any go-to approach
> to read the pointer value in a raw tp? Doesn't seem to find one in
> selftests/samples. If not, does it make sense if we allow it in
> the verifier for tracing programs like the attached patch?
>
> Yan
>
> ---
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 821063660d9f..5f000ab4c8d0 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -6308,6 +6308,11 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
>                         __btf_name_by_offset(btf, t->name_off),
>                         btf_type_str(t));
>                 return false;
> +       } else if (prog->type == BPF_PROG_TYPE_TRACING || prog->type == BPF_PROG_TYPE_RAW_TRACEPOINT) {
> +               /* allow reading function pointer value from a tracing program */
> +               const struct btf_type *pointed = btf_type_by_id(btf, t->type);
> +               if (btf_type_is_func_proto(pointed))
> +                       return true;

The reason it wasn't supported in tp_btf is to avoid potential
backward compat issues when the verifier will start to recognize it
as a proper pointer to a function.
Since I didn't know what that support would look like I left it
as an error for now.

'return true' (which would mean scalar type to the verifier)
is a bit dangerous and I feel it's better to think it through right away.

Eventually it probably will be a new reg_type PTR_TO_KERN_FUNC or something.
And it won't be modifiable.
Like arithmetic won't be allowed.
Passing into other helpers (like prinkt in your example) is fine.
But if we do 'return true -> scalar' today then bpf prog
will be able to do math on it, including conditional jmps,
which will be disallowed once it becomes PTR_TO_KERN_FUNC.
And that would become a backward compat issue.
So pls think it through from that angle.

Re: Ideal way to read FUNC_PROTO in raw tp?

[Yan Zhai]

Hi Alexei,

Thanks for getting back.

On Fri, Jun 14, 2024 at 8:05 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Jun 6, 2024 at 3:03 PM Yan Zhai <yan@cloudflare.com> wrote:
> >
> > Hi,
> >
> >  I am building a tracing program around workqueue. But I encountered
> > following problem when I try to record a function pointer value from
> > trace_workqueue_execute_end on net-next kernel:
> >
> > ...
> > libbpf: prog 'workqueue_end': BPF program load failed: Permission
> > denied
> > libbpf: prog 'workqueue_end': -- BEGIN PROG LOAD LOG --
> > reg type unsupported for arg#0 function workqueue_end#5
> > 0: R1=ctx(off=0,imm=0) R10=fp0
> > ; int BPF_PROG(workqueue_end, struct work_struct *w, work_func_t f)
> > 0: (79) r3 = *(u64 *)(r1 +8)
> > func 'workqueue_execute_end' arg1 type FUNC_PROTO is not a struct
> > invalid bpf_context access off=8 size=8
> > 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 'workqueue_end': failed to load: -13
> > libbpf: failed to load object 'configs/test.bpf.o'
> > Error: failed to load object file
> > Warning: bpftool is now running in libbpf strict mode and has more
> > stringent requirements about BPF programs.
> > If it used to work for this object file but now doesn't, see --legacy
> > option for more details.
> > ...
> >
> > A simple reproducer for me is like:
> > #include "vmlinux.h"
> > #include <bpf/bpf_helpers.h>
> > #include <bpf/bpf_tracing.h>
> >
> > SEC("tp_btf/workqueue_execute_end")
> > int BPF_PROG(workqueue_end, struct work_struct *w, work_func_t f)
> > {
> >         u64 addr = (u64) f;
> >         bpf_printk("f is %lu\n", addr);
> >
> >         return 0;
> > }
> >
> > char LICENSE[] SEC("license") = "GPL";
> >
> > I would like to use the function address to decode the kernel symbol
> > and track execution of these functions. Replacing raw tp to regular tp
> > solves the problem, but I am wondering if there is any go-to approach
> > to read the pointer value in a raw tp? Doesn't seem to find one in
> > selftests/samples. If not, does it make sense if we allow it in
> > the verifier for tracing programs like the attached patch?
> >
> > Yan
> >
> > ---
> > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > index 821063660d9f..5f000ab4c8d0 100644
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -6308,6 +6308,11 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> >                         __btf_name_by_offset(btf, t->name_off),
> >                         btf_type_str(t));
> >                 return false;
> > +       } else if (prog->type == BPF_PROG_TYPE_TRACING || prog->type == BPF_PROG_TYPE_RAW_TRACEPOINT) {
> > +               /* allow reading function pointer value from a tracing program */
> > +               const struct btf_type *pointed = btf_type_by_id(btf, t->type);
> > +               if (btf_type_is_func_proto(pointed))
> > +                       return true;
>
> The reason it wasn't supported in tp_btf is to avoid potential
> backward compat issues when the verifier will start to recognize it
> as a proper pointer to a function.
> Since I didn't know what that support would look like I left it
> as an error for now.
>
> 'return true' (which would mean scalar type to the verifier)
> is a bit dangerous and I feel it's better to think it through right away.
>
> Eventually it probably will be a new reg_type PTR_TO_KERN_FUNC or something.
> And it won't be modifiable.
> Like arithmetic won't be allowed.
> Passing into other helpers (like prinkt in your example) is fine.
> But if we do 'return true -> scalar' today then bpf prog
> will be able to do math on it, including conditional jmps,
> which will be disallowed once it becomes PTR_TO_KERN_FUNC.
> And that would become a backward compat issue.
> So pls think it through from that angle.

Having a new reg_type makes sense to me, but IMHO it could still cause
a backward compatibility issue. In my example, the regular version TP
has the func pointer assigned as a void * arg, which I can do
arithmetics already with. So if anyone relies on this fact, then it
may break if we introduce the PTR_TO_KERN_FUNC and replace void * with
a proper kernel function pointer. But if we leave the void * in place,
then TP/Raw-TP have different capabilities on the same argument just
like today. WDYT?

Yan

Re: Ideal way to read FUNC_PROTO in raw tp?

[Andrii Nakryiko]

On Thu, Jun 6, 2024 at 3:03 PM Yan Zhai <yan@cloudflare.com> wrote:
>
> Hi,
>
>  I am building a tracing program around workqueue. But I encountered
> following problem when I try to record a function pointer value from
> trace_workqueue_execute_end on net-next kernel:
>
> ...
> libbpf: prog 'workqueue_end': BPF program load failed: Permission
> denied
> libbpf: prog 'workqueue_end': -- BEGIN PROG LOAD LOG --
> reg type unsupported for arg#0 function workqueue_end#5
> 0: R1=ctx(off=0,imm=0) R10=fp0
> ; int BPF_PROG(workqueue_end, struct work_struct *w, work_func_t f)
> 0: (79) r3 = *(u64 *)(r1 +8)
> func 'workqueue_execute_end' arg1 type FUNC_PROTO is not a struct
> invalid bpf_context access off=8 size=8
> 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 'workqueue_end': failed to load: -13
> libbpf: failed to load object 'configs/test.bpf.o'
> Error: failed to load object file
> Warning: bpftool is now running in libbpf strict mode and has more
> stringent requirements about BPF programs.
> If it used to work for this object file but now doesn't, see --legacy
> option for more details.
> ...
>
> A simple reproducer for me is like:
> #include "vmlinux.h"
> #include <bpf/bpf_helpers.h>
> #include <bpf/bpf_tracing.h>
>
> SEC("tp_btf/workqueue_execute_end")
> int BPF_PROG(workqueue_end, struct work_struct *w, work_func_t f)
> {
>         u64 addr = (u64) f;

you can work around with:

bpf_probe_read_kernel(&addr, sizeof(addr), &ctx[1]); /* ctx[1] is the
second argument */

Not great, but will get you past this easily.

>         bpf_printk("f is %lu\n", addr);
>
>         return 0;
> }
>
> char LICENSE[] SEC("license") = "GPL";
>
> I would like to use the function address to decode the kernel symbol
> and track execution of these functions. Replacing raw tp to regular tp
> solves the problem, but I am wondering if there is any go-to approach
> to read the pointer value in a raw tp? Doesn't seem to find one in
> selftests/samples. If not, does it make sense if we allow it in
> the verifier for tracing programs like the attached patch?
>
> Yan
>
> ---
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 821063660d9f..5f000ab4c8d0 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -6308,6 +6308,11 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
>                         __btf_name_by_offset(btf, t->name_off),
>                         btf_type_str(t));
>                 return false;
> +       } else if (prog->type == BPF_PROG_TYPE_TRACING || prog->type == BPF_PROG_TYPE_RAW_TRACEPOINT) {
> +               /* allow reading function pointer value from a tracing program */
> +               const struct btf_type *pointed = btf_type_by_id(btf, t->type);
> +               if (btf_type_is_func_proto(pointed))
> +                       return true;
>         }
>
>         /* check for PTR_TO_RDONLY_BUF_OR_NULL or PTR_TO_RDWR_BUF_OR_NULL */
>

Re: Ideal way to read FUNC_PROTO in raw tp?

[Yan Zhai]

On Fri, Jun 21, 2024 at 5:04 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Jun 6, 2024 at 3:03 PM Yan Zhai <yan@cloudflare.com> wrote:
> >
> > Hi,
> >
> >  I am building a tracing program around workqueue. But I encountered
> > following problem when I try to record a function pointer value from
> > trace_workqueue_execute_end on net-next kernel:
> >
> > ...
> > libbpf: prog 'workqueue_end': BPF program load failed: Permission
> > denied
> > libbpf: prog 'workqueue_end': -- BEGIN PROG LOAD LOG --
> > reg type unsupported for arg#0 function workqueue_end#5
> > 0: R1=ctx(off=0,imm=0) R10=fp0
> > ; int BPF_PROG(workqueue_end, struct work_struct *w, work_func_t f)
> > 0: (79) r3 = *(u64 *)(r1 +8)
> > func 'workqueue_execute_end' arg1 type FUNC_PROTO is not a struct
> > invalid bpf_context access off=8 size=8
> > 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 'workqueue_end': failed to load: -13
> > libbpf: failed to load object 'configs/test.bpf.o'
> > Error: failed to load object file
> > Warning: bpftool is now running in libbpf strict mode and has more
> > stringent requirements about BPF programs.
> > If it used to work for this object file but now doesn't, see --legacy
> > option for more details.
> > ...
> >
> > A simple reproducer for me is like:
> > #include "vmlinux.h"
> > #include <bpf/bpf_helpers.h>
> > #include <bpf/bpf_tracing.h>
> >
> > SEC("tp_btf/workqueue_execute_end")
> > int BPF_PROG(workqueue_end, struct work_struct *w, work_func_t f)
> > {
> >         u64 addr = (u64) f;
>
> you can work around with:
>
> bpf_probe_read_kernel(&addr, sizeof(addr), &ctx[1]); /* ctx[1] is the
> second argument */
>
> Not great, but will get you past this easily.
>
Yes that works, too. Currently I am using the regular tp to
workaround. That said, I am more interested in the verifier side of
this. I am happy to add it to my queue if no one has signed up to
support func_proto type in ctx. Would maintainers welcome RFC patches
for this or is it something that you prefer to leave for now?

best
Yan