Re: [PATCH v2 bpf-next 2/2] selftests/bpf: Check bpf_skb_output() tracing restrictions

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 4181 bytes --]

> commit 586c31fe079529e232f3760545b476754b91501b
> Author:     Feng Yang <yangfeng@kylinos.cn>
> AuthorDate: Tue Sep 22 09:54:17 2026 +0800
> Commit:     Kernel Patches Daemon <kernel-patches-bot@fb.com>
> CommitDate: Mon Sep 21 19:02:48 2026 -0700
>
>     selftests/bpf: Check bpf_skb_output() tracing restrictions
>
>     Add verifier coverage for the bpf_skb_output() tracing policy. Verify
>     that an fentry program can still use the helper, while the equivalent
>     fexit program is rejected at load time.
>
>     Signed-off-by: Yun Lu <luyun@kylinos.cn>
>     Signed-off-by: Feng Yang<yangfeng@kylinos.cn>

There's a formatting issue in the second Signed-off-by line - it's missing
the space between the name and email address. Should this be:

  Signed-off-by: Feng Yang <yangfeng@kylinos.cn>

instead of "Feng Yang<yangfeng@kylinos.cn>"?

> diff --git a/tools/testing/selftests/bpf/progs/verifier_helper_restricted.c b/tools/testing/selftests/bpf/progs/verifier_helper_restricted.c
> index 889c9b78b912e..058e71927cd5c 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_helper_restricted.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_helper_restricted.c
> @@ -3,6 +3,7 @@
>
>  #include <linux/bpf.h>
>  #include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
>  #include "bpf_misc.h"
>
>  struct val {
> @@ -17,6 +18,13 @@ struct {
>  	__type(value, struct val);
>  } map_spin_lock SEC(".maps");
>
> +struct {
> +	__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
> +	__uint(max_entries, 1);
> +	__type(key, __u32);
> +	__type(value, __u32);
> +} perf_event_map SEC(".maps");
> +

[ ... ]

> @@ -165,4 +173,28 @@ l0_%=:	exit;						\
>  	: __clobber_all);
>  }
>
> +SEC("fentry/skb_tx_error")
> +__description("bpf_skb_output is allowed in BPF_TRACE_FENTRY")
> +__success
> +int BPF_PROG(skb_output_fentry, void *skb)
> +{
> +	__u64 meta = 0;
> +
> +	bpf_skb_output(skb, &perf_event_map, BPF_F_CURRENT_CPU,
> +		       &meta, sizeof(meta));
> +	return 0;
> +}
> +
> +SEC("fexit/skb_tx_error")
> +__description("bpf_skb_output is forbidden in BPF_TRACE_FEXIT")
> +__failure __msg("program of this type cannot use helper bpf_skb_output")
> +int BPF_PROG(skb_output_fexit, void *skb)
> +{
> +	__u64 meta = 0;
> +
> +	bpf_skb_output(skb, &perf_event_map, BPF_F_CURRENT_CPU,
> +		       &meta, sizeof(meta));
> +	return 0;
> +}
> +
>  char _license[] SEC("license") = "GPL";

The kernel change this tests (538d3dadf8d0e) gates bpf_skb_output() on
four attach types in tracing_prog_may_run_after_target():

  case BPF_TRACE_FEXIT:
  case BPF_TRACE_FEXIT_MULTI:
  case BPF_TRACE_FSESSION:
  case BPF_TRACE_FSESSION_MULTI:
          return true;

Only BPF_TRACE_FEXIT gets a test case here. Would it make sense to add
coverage for BPF_TRACE_FEXIT_MULTI, BPF_TRACE_FSESSION and
BPF_TRACE_FSESSION_MULTI as well?

The fsession case in particular is the one whose rationale the parent
commit message spends a paragraph on - that fsession must be rejected
because the same program runs on both entry and return and the verifier
cannot prove that a helper call is entry-only.

All three have libbpf section prefixes available (tools/lib/bpf/libbpf.c
has "fsession+", "fsession.multi+", "fexit.multi+"), so the extra test cases
would be copies of the existing fexit block with different SEC() strings.

Kumar Kartikeya Dwivedi raised a design concern about the overall approach
in v2:

https://lore.kernel.org/all/DLLHE9PBLDU0.XQGEOQQIR4HV@gmail.com/

  "Sorry, this is not an acceptable fix. This negates usage of the
  function in all fexit and fession attach points. Unless there is a
  simpler way to enumerate in which attach points this helper should be
  disabled, it might not be worth doing and leaving this be as is."

The concern is that disabling bpf_skb_output() in all fexit/fsession
programs may be too broad. Since this test verifies the blanket restriction
that was rejected, has this feedback been addressed?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/35678199634