Sashiko discussions
 help / color / mirror / Atom feed
* Re: [PATCHv5 bpf-next 10/28] bpf: Add multi tracing attach types
       [not found] ` <20260418040909.5F3B7C19424@smtp.kernel.org>
@ 2026-04-21  8:55   ` Jiri Olsa
  0 siblings, 0 replies; only message in thread
From: Jiri Olsa @ 2026-04-21  8:55 UTC (permalink / raw)
  To: sashiko; +Cc: bpf

On Sat, Apr 18, 2026 at 04:09:08AM +0000, sashiko-bot@kernel.org wrote:
> Sashiko AI review found 2 potential issue(s):
> - [High] FENTRY_MULTI and FEXIT_MULTI programs can bypass verifier bounds checks and read out-of-bounds kernel stack memory by directly accessing the `ctx` array.
> - [High] bpf_multi_func lacks the __used attribute, causing it to be optimized out by Clang LTO, which completely breaks the feature at runtime.
> --
> 
> Note: The format of this review has been altered due to system recitation
> restrictions. Direct quoting of the patch diff has been omitted, and the
> findings are provided as a free-form summary.
> 
> commit ca84efa8f86b1acb8cf0bd98e80446dbd7c92e12
> Author: Jiri Olsa <jolsa@kernel.org>
> Subject: bpf: Add multi tracing attach types
> 
> This commit introduces new multi-tracing attach types BPF_TRACE_FENTRY_MULTI
> and BPF_TRACE_FEXIT_MULTI, along with base verifier support. It uses a dummy
> function bpf_multi_func for initial verification before the specific link
> attachment happens.
> 
> Can this approach lead to out-of-bounds kernel stack memory reads? 
> 
> Since the initial verification is done against bpf_multi_func, which is 
> defined with 6 arguments, the verifier will statically permit direct context 
> accesses up to ctx[5] (and ctx[6] for the return value in FEXIT_MULTI). 
> However, when the program is attached to a real function at runtime, the 
> trampoline allocates the context array on the kernel stack based on the 
> target function's actual argument count. 
> 
> If the target function has fewer than 6 arguments, would reading ctx[5] read 
> past the allocated context array and leak memory from the caller's stack 
> frame? Additionally, would reading ctx[6] for the return value return 
> unrelated stack memory instead of the actual return value, since the 
> trampoline places the return value dynamically? Does the verifier need to 
> explicitly reject direct memory accesses to the context array for these 
> program types and enforce the use of bounds-checked helpers like 
> bpf_get_func_arg()?

ok, seems like we need to disable the direct access

> 
> Is it possible for Clang LTO to optimize out the bpf_multi_func function 
> entirely? 
> 
> The dummy function is defined without a __used or __attribute__((retain)) 
> attribute, and the BTF_ID_LIST_GLOBAL_SINGLE macro only embeds the function 
> name in an inline assembly section without generating a C-level reference. 
> In kernel builds with Link Time Optimization enabled, could the compiler 
> eliminate this function as dead code?
> 
> If this occurs, pahole would not generate BTF data for it, leaving the BTF 
> ID as 0. Would this result in the verifier's bpf_check_attach_target() 
> unconditionally rejecting multi-tracing program loads by throwing a missing 
> btf_id error? Should a __used attribute be added to the function definition 
> to prevent this?

I did not check with LTO, will check and probably add that attribute

jirka

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260417192502.194548-11-jolsa@kernel.org?part=1

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2026-04-21  8:55 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260417192502.194548-11-jolsa@kernel.org>
     [not found] ` <20260418040909.5F3B7C19424@smtp.kernel.org>
2026-04-21  8:55   ` [PATCHv5 bpf-next 10/28] bpf: Add multi tracing attach types Jiri Olsa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox