[PATCH bpf-next v2 0/3] allow cpumask kfuncs in tracepoint, kprobe, perf event

[PATCH bpf-next v2 0/3] allow cpumask kfuncs in tracepoint, kprobe, perf event

[JP Kobryn]

It is possible to call a cpumask kfunc within a raw tp_btf program but not
possible within tracepoint, kprobe, or perf event programs. Currently, the
verifier receives -EACCESS from fetch_kfunc_meta() as a result of not
finding any kfunc hook associated with these program types.

This patch series adds new kfunc hooks and maps them to program types
where needed. The additional registration of the three program types
mentioned is done for the cpumask kfuncs allowing them to be called within
these types of programs.

Pre-submission CI run: https://github.com/kernel-patches/bpf/pull/7600

v2:
	- create new kfunc hooks for tracepoint and perf event
	- map tracepoint, and perf event prog types to kfunc hooks
	- register cpumask kfuncs with prog types in focus
	- expand existing verifier tests for cpumask kfuncs
v1:
	- map tracepoint type progs to tracing kfunc hook
	- new selftests for calling cpumask kfuncs in tracepoint prog
---
JP Kobryn (3):
  bpf: new btf kfunc hooks for tracepoint and perf event
  bpf: register additional prog types with cpumask kfuncs
  bpf/selftests: coverage for new program types using cpumask kfuncs

 kernel/bpf/btf.c                              |  6 +++++
 kernel/bpf/cpumask.c                          |  3 +++
 .../bpf/progs/verifier_kfunc_prog_types.c     | 24 +++++++++++++++++++
 3 files changed, 33 insertions(+)

-- 
2.46.0

[PATCH bpf-next v2 1/3] bpf: new btf kfunc hooks for tracepoint and perf event

[JP Kobryn]

The additional hooks (and prog-to-hook mapping) for tracepoint and perf
event programs allow for registering kfuncs to be used within these
program types.

Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
---
 kernel/bpf/btf.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 520f49f422fe..4816e309314e 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -210,6 +210,7 @@ enum btf_kfunc_hook {
 	BTF_KFUNC_HOOK_TC,
 	BTF_KFUNC_HOOK_STRUCT_OPS,
 	BTF_KFUNC_HOOK_TRACING,
+	BTF_KFUNC_HOOK_TRACEPOINT,
 	BTF_KFUNC_HOOK_SYSCALL,
 	BTF_KFUNC_HOOK_FMODRET,
 	BTF_KFUNC_HOOK_CGROUP_SKB,
@@ -219,6 +220,7 @@ enum btf_kfunc_hook {
 	BTF_KFUNC_HOOK_LWT,
 	BTF_KFUNC_HOOK_NETFILTER,
 	BTF_KFUNC_HOOK_KPROBE,
+	BTF_KFUNC_HOOK_PERF_EVENT,
 	BTF_KFUNC_HOOK_MAX,
 };
 
@@ -8306,6 +8308,8 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
 	case BPF_PROG_TYPE_TRACING:
 	case BPF_PROG_TYPE_LSM:
 		return BTF_KFUNC_HOOK_TRACING;
+	case BPF_PROG_TYPE_TRACEPOINT:
+		return BTF_KFUNC_HOOK_TRACEPOINT;
 	case BPF_PROG_TYPE_SYSCALL:
 		return BTF_KFUNC_HOOK_SYSCALL;
 	case BPF_PROG_TYPE_CGROUP_SKB:
@@ -8326,6 +8330,8 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
 		return BTF_KFUNC_HOOK_NETFILTER;
 	case BPF_PROG_TYPE_KPROBE:
 		return BTF_KFUNC_HOOK_KPROBE;
+	case BPF_PROG_TYPE_PERF_EVENT:
+		return BTF_KFUNC_HOOK_PERF_EVENT;
 	default:
 		return BTF_KFUNC_HOOK_MAX;
 	}
-- 
2.46.0

Re: [PATCH bpf-next v2 1/3] bpf: new btf kfunc hooks for tracepoint and perf event

[Alexei Starovoitov]

On Mon, Aug 26, 2024 at 3:48 PM JP Kobryn <inwardvessel@gmail.com> wrote:
>
> The additional hooks (and prog-to-hook mapping) for tracepoint and perf
> event programs allow for registering kfuncs to be used within these
> program types.
>
> Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
> ---
>  kernel/bpf/btf.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 520f49f422fe..4816e309314e 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -210,6 +210,7 @@ enum btf_kfunc_hook {
>         BTF_KFUNC_HOOK_TC,
>         BTF_KFUNC_HOOK_STRUCT_OPS,
>         BTF_KFUNC_HOOK_TRACING,
> +       BTF_KFUNC_HOOK_TRACEPOINT,
>         BTF_KFUNC_HOOK_SYSCALL,
>         BTF_KFUNC_HOOK_FMODRET,
>         BTF_KFUNC_HOOK_CGROUP_SKB,
> @@ -219,6 +220,7 @@ enum btf_kfunc_hook {
>         BTF_KFUNC_HOOK_LWT,
>         BTF_KFUNC_HOOK_NETFILTER,
>         BTF_KFUNC_HOOK_KPROBE,
> +       BTF_KFUNC_HOOK_PERF_EVENT,
>         BTF_KFUNC_HOOK_MAX,
>  };
>
> @@ -8306,6 +8308,8 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
>         case BPF_PROG_TYPE_TRACING:
>         case BPF_PROG_TYPE_LSM:
>                 return BTF_KFUNC_HOOK_TRACING;
> +       case BPF_PROG_TYPE_TRACEPOINT:
> +               return BTF_KFUNC_HOOK_TRACEPOINT;

why special case tp and perf_event and only limit them to cpumask?
The following would be equally safe, no?
         case BPF_PROG_TYPE_TRACING:
         case BPF_PROG_TYPE_LSM:
 +       case BPF_PROG_TYPE_TRACEPOINT:
 +       case BPF_PROG_TYPE_PERF_EVENT:
                return BTF_KFUNC_HOOK_TRACING;
?

Re: [PATCH bpf-next v2 1/3] bpf: new btf kfunc hooks for tracepoint and perf event

[Andrii Nakryiko]

On Tue, Aug 27, 2024 at 2:01 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Aug 26, 2024 at 3:48 PM JP Kobryn <inwardvessel@gmail.com> wrote:
> >
> > The additional hooks (and prog-to-hook mapping) for tracepoint and perf
> > event programs allow for registering kfuncs to be used within these
> > program types.
> >
> > Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
> > ---
> >  kernel/bpf/btf.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > index 520f49f422fe..4816e309314e 100644
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -210,6 +210,7 @@ enum btf_kfunc_hook {
> >         BTF_KFUNC_HOOK_TC,
> >         BTF_KFUNC_HOOK_STRUCT_OPS,
> >         BTF_KFUNC_HOOK_TRACING,
> > +       BTF_KFUNC_HOOK_TRACEPOINT,
> >         BTF_KFUNC_HOOK_SYSCALL,
> >         BTF_KFUNC_HOOK_FMODRET,
> >         BTF_KFUNC_HOOK_CGROUP_SKB,
> > @@ -219,6 +220,7 @@ enum btf_kfunc_hook {
> >         BTF_KFUNC_HOOK_LWT,
> >         BTF_KFUNC_HOOK_NETFILTER,
> >         BTF_KFUNC_HOOK_KPROBE,
> > +       BTF_KFUNC_HOOK_PERF_EVENT,
> >         BTF_KFUNC_HOOK_MAX,
> >  };
> >
> > @@ -8306,6 +8308,8 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
> >         case BPF_PROG_TYPE_TRACING:
> >         case BPF_PROG_TYPE_LSM:
> >                 return BTF_KFUNC_HOOK_TRACING;
> > +       case BPF_PROG_TYPE_TRACEPOINT:
> > +               return BTF_KFUNC_HOOK_TRACEPOINT;
>
> why special case tp and perf_event and only limit them to cpumask?
> The following would be equally safe, no?

Assuming we don't have kfuncs that accepts program context (like
bpf_get_stack(), if it was a kfunc) and that doesn't access
bpf_run_ctx (like bpf_get_func_ip()). We just need to be careful about
adding new special kfuncs like that going forward (not sure how to
best ensure we don't forget, though). Other than that I agree that
it's all "tracing".

>          case BPF_PROG_TYPE_TRACING:
>          case BPF_PROG_TYPE_LSM:
>  +       case BPF_PROG_TYPE_TRACEPOINT:
>  +       case BPF_PROG_TYPE_PERF_EVENT:
>                 return BTF_KFUNC_HOOK_TRACING;
> ?

Re: [PATCH bpf-next v2 1/3] bpf: new btf kfunc hooks for tracepoint and perf event

[JP Kobryn]

On Tue, Aug 27, 2024 at 03:42:34PM -0700, Andrii Nakryiko wrote:
> On Tue, Aug 27, 2024 at 2:01 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Mon, Aug 26, 2024 at 3:48 PM JP Kobryn <inwardvessel@gmail.com> wrote:
> > >
> > > The additional hooks (and prog-to-hook mapping) for tracepoint and perf
> > > event programs allow for registering kfuncs to be used within these
> > > program types.
> > >
> > > Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
> > > ---
> > >  kernel/bpf/btf.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > > index 520f49f422fe..4816e309314e 100644
> > > --- a/kernel/bpf/btf.c
> > > +++ b/kernel/bpf/btf.c
> > > @@ -210,6 +210,7 @@ enum btf_kfunc_hook {
> > >         BTF_KFUNC_HOOK_TC,
> > >         BTF_KFUNC_HOOK_STRUCT_OPS,
> > >         BTF_KFUNC_HOOK_TRACING,
> > > +       BTF_KFUNC_HOOK_TRACEPOINT,
> > >         BTF_KFUNC_HOOK_SYSCALL,
> > >         BTF_KFUNC_HOOK_FMODRET,
> > >         BTF_KFUNC_HOOK_CGROUP_SKB,
> > > @@ -219,6 +220,7 @@ enum btf_kfunc_hook {
> > >         BTF_KFUNC_HOOK_LWT,
> > >         BTF_KFUNC_HOOK_NETFILTER,
> > >         BTF_KFUNC_HOOK_KPROBE,
> > > +       BTF_KFUNC_HOOK_PERF_EVENT,
> > >         BTF_KFUNC_HOOK_MAX,
> > >  };
> > >
> > > @@ -8306,6 +8308,8 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
> > >         case BPF_PROG_TYPE_TRACING:
> > >         case BPF_PROG_TYPE_LSM:
> > >                 return BTF_KFUNC_HOOK_TRACING;
> > > +       case BPF_PROG_TYPE_TRACEPOINT:
> > > +               return BTF_KFUNC_HOOK_TRACEPOINT;
> >
> > why special case tp and perf_event and only limit them to cpumask?
> > The following would be equally safe, no?
> 
> Assuming we don't have kfuncs that accepts program context (like
> bpf_get_stack(), if it was a kfunc) and that doesn't access
> bpf_run_ctx (like bpf_get_func_ip()). We just need to be careful about
> adding new special kfuncs like that going forward (not sure how to
> best ensure we don't forget, though). Other than that I agree that
> it's all "tracing".

What Alexei is suggesting works. I did something similar in v1[0] where I
associated BPF_PROG_TYPE_TRACEPOINT with BTF_KFUNC_HOOK_TRACING. But it
occurred to me that this circumvents the registration process during
initialization, so I want to make sure if this is or is not acceptable. See
below for my thoughts.
> 
> >          case BPF_PROG_TYPE_TRACING:
> >          case BPF_PROG_TYPE_LSM:
> >  +       case BPF_PROG_TYPE_TRACEPOINT:
> >  +       case BPF_PROG_TYPE_PERF_EVENT:
> >                 return BTF_KFUNC_HOOK_TRACING;
> > ?

With this change, anywhere we do
register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &some_kfunc_set),
BTF_KFUNC_HOOK_TRACING becomes allowed. So even if we never register the
extra program types like PROG_TYPE_PERF_EVENT, we still allow them as a
side effect since at runtime the program type mapping returns HOOK_TRACING.
Any program type associated with this hook will be allowed even though not
explicitly registered. My take on v2 was moving towards the element of
least surprise, and thought the explicit registration with the new hooks
made sense. I'm fine though, if we prefer this style above with the
implicit registration. Let me know and I can make a v3 if needed.

[0] https://lore.kernel.org/all/20240814235800.15253-3-inwardvessel@gmail.com/

Re: [PATCH bpf-next v2 1/3] bpf: new btf kfunc hooks for tracepoint and perf event

[Alexei Starovoitov]

On Wed, Aug 28, 2024 at 12:08 PM JP Kobryn <inwardvessel@gmail.com> wrote:
>
> With this change, anywhere we do
> register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &some_kfunc_set),
> BTF_KFUNC_HOOK_TRACING becomes allowed. So even if we never register the
> extra program types like PROG_TYPE_PERF_EVENT, we still allow them as a
> side effect since at runtime the program type mapping returns HOOK_TRACING.
> Any program type associated with this hook will be allowed even though not
> explicitly registered.

Correct. kfuncs differentiate by type of their arguments.
prog_type -> helper was an old style when context was the only
thing we had. So we had to differentiate by prog_type.
kfuncs are currently register by prog_type too, but that will go away.
We're planning to do large refactoring in that area to satisfy
sched-ext requests. It's not the prog type that would allow or
disallow certain kfuncs but rather hook point plus custom flags.

[PATCH bpf-next v2 2/3] bpf: register additional prog types with cpumask kfuncs

[JP Kobryn]

Allow the cpumask family of kfuncs to be called within tracepoint, kprobe,
and perf event programs.

Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
---
 kernel/bpf/cpumask.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/bpf/cpumask.c b/kernel/bpf/cpumask.c
index 33c473d676a5..30b0164a2212 100644
--- a/kernel/bpf/cpumask.c
+++ b/kernel/bpf/cpumask.c
@@ -475,6 +475,9 @@ static int __init cpumask_kfunc_init(void)
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &cpumask_kfunc_set);
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &cpumask_kfunc_set);
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &cpumask_kfunc_set);
+	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACEPOINT, &cpumask_kfunc_set);
+	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_KPROBE, &cpumask_kfunc_set);
+	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_PERF_EVENT, &cpumask_kfunc_set);
 	return  ret ?: register_btf_id_dtor_kfuncs(cpumask_dtors,
 						   ARRAY_SIZE(cpumask_dtors),
 						   THIS_MODULE);
-- 
2.46.0

[PATCH bpf-next v2 3/3] bpf/selftests: coverage for new program types using cpumask kfuncs

[JP Kobryn]

This coverage ensures that the cpumask family of kfuncs are allowed within
tracepoint, kprobe, and perf event programs.

Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
---
 .../bpf/progs/verifier_kfunc_prog_types.c     | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_kfunc_prog_types.c b/tools/testing/selftests/bpf/progs/verifier_kfunc_prog_types.c
index cb32b0cfc84b..ac1a64974187 100644
--- a/tools/testing/selftests/bpf/progs/verifier_kfunc_prog_types.c
+++ b/tools/testing/selftests/bpf/progs/verifier_kfunc_prog_types.c
@@ -120,3 +120,27 @@ int BPF_PROG(cpumask_kfunc_syscall)
 	cpumask_kfunc_load_test();
 	return 0;
 }
+
+SEC("tracepoint")
+__success
+int BPF_PROG(cpumask_kfunc_tracepoint)
+{
+	cpumask_kfunc_load_test();
+	return 0;
+}
+
+SEC("kprobe")
+__success
+int BPF_PROG(cpumask_kfunc_kprobe)
+{
+	cpumask_kfunc_load_test();
+	return 0;
+}
+
+SEC("perf_event")
+__success
+int BPF_PROG(cpumask_kfunc_perf_event)
+{
+	cpumask_kfunc_load_test();
+	return 0;
+}
-- 
2.46.0