Re: [PATCH bpf-next 1/2] bpf: Reject bpf_skb_output() from return-side tracing

[bot+bpf-ci@kernel.org]

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

> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 29260951aa871..cdede3926e5d6 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1339,6 +1339,20 @@ static inline bool is_trace_fsession(const struct bpf_prog *prog)
>  		prog->expected_attach_type == BPF_TRACE_FSESSION_MULTI);
>  }
>
> +static bool tracing_prog_may_run_after_target(const struct bpf_prog *prog)
> +{
> +	/* The target may consume pointer arguments before these programs run. */
> +	switch (prog->expected_attach_type) {
> +	case BPF_TRACE_FEXIT:
> +	case BPF_TRACE_FEXIT_MULTI:
> +	case BPF_TRACE_FSESSION:
> +	case BPF_TRACE_FSESSION_MULTI:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +

Does this new helper need to be static inline? The function is defined
unconditionally here but only referenced inside the CONFIG_NET block of
tracing_prog_func_proto():

kernel/trace/bpf_trace.c:tracing_prog_func_proto() {
    ...
    switch (func_id) {
#ifdef CONFIG_NET
    case BPF_FUNC_skb_output:
        if (tracing_prog_may_run_after_target(prog))
            return NULL;
        return &bpf_skb_output_proto;
    ...
#endif
    ...
}

bpf_trace.c is built from CONFIG_BPF_EVENTS (kernel/trace/Makefile) which
depends on BPF_SYSCALL and (KPROBE_EVENTS || UPROBE_EVENTS) and PERF_EVENTS
(kernel/trace/Kconfig) -- none of which imply CONFIG_NET.

CONFIG_BPF_EVENTS=y with CONFIG_NET=n is valid, and in that configuration
tracing_prog_may_run_after_target() becomes a defined-but-unreferenced
non-inline static:

  kernel/trace/bpf_trace.c:1342:13: warning: 'tracing_prog_may_run_after_target' defined but not used [-Wunused-function]

With CONFIG_WERROR=y (init/Kconfig) this becomes a build failure.

The pre-existing predicates immediately above (is_kprobe_session(),
is_uprobe_multi(), is_uprobe_session(), is_trace_fsession()) are all
written as static inline, which is why they don't trigger this warning.

Would declaring tracing_prog_may_run_after_target() as static inline fix
this?

>  static const struct bpf_func_proto *
>  kprobe_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  {
> @@ -1730,6 +1744,8 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  	switch (func_id) {
>  #ifdef CONFIG_NET
>  	case BPF_FUNC_skb_output:
> +		if (tracing_prog_may_run_after_target(prog))
> +			return NULL;
>  		return &bpf_skb_output_proto;
>  	case BPF_FUNC_xdp_output:
>  		return &bpf_xdp_output_proto;


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