Re: [PATCH bpf-next v12 3/6] bpf: Add sleepable support for classic tracepoint programs

sashiko-bot
2026-04-22T23:06:25Z

Sashiko AI review found 2 potential issue(s):
- [Critical] Inverted RCU lock logic in `bpf_prog_run_array_sleepable()` causes sleepable programs to run under `rcu_read_lock()` and non-sleepable programs to run without it.
- [High] Uninitialized kernel stack memory is leaked to BPF programs via the `param` struct in `perf_call_bpf_enter()` and `perf_call_bpf_exit()`.
--

commit 37b318a3f61844cb76121bebc8014d8031e06996
Author: Mykyta Yatsenko <yatsenko@meta.com>

bpf: Add sleepable support for classic tracepoint programs

This commit adds trace_call_bpf_faultable() for faultable tracepoints,
supporting sleepable bpf programs. It also restructures perf syscall hooks
to run bpf before preempt_disable.

> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 4e763dd2aa2bb..bbc0710615014 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c

[ ... ]

> +unsigned int trace_call_bpf_faultable(struct trace_event_call *call, void *ctx)
> +{
> + struct bpf_prog_array *prog_array;
> +
> + might_fault();
> + guard(rcu_tasks_trace)();
> +
> + prog_array = rcu_dereference_check(call->prog_array,
> + rcu_read_lock_trace_held());
> + return bpf_prog_run_array_sleepable(prog_array, ctx, bpf_prog_run);
> +}

Does bpf_prog_run_array_sleepable() properly handle rcu locks for sleepable
programs?

A preceding patch introduced bpf_prog_run_array_sleepable() which appears
to have an inverted logic check:

if (prog->sleepable) {
guard(rcu)();
...
}

This causes sleepable programs to execute with the rcu read lock held,
preventing them from sleeping. Conversely, non-sleepable bpf programs run
without the rcu read lock, which could lead to regressions when accessing
rcu-protected data.

> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
> index 8ad72e17d8eb9..e98ee7e1e66f4 100644
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -1371,33 +1371,33 @@ static DECLARE_BITMAP(enabled_perf_exit_syscalls, NR_syscalls);
> static int sys_perf_refcount_enter;
> static int sys_perf_refcount_exit;
>
> -static int perf_call_bpf_enter(struct trace_event_call *call, struct pt_regs *regs,
> +static int perf_call_bpf_enter(struct trace_event_call *call,
> struct syscall_metadata *sys_data,
> - struct syscall_trace_enter *rec)
> + int syscall_nr, unsigned long *args)
> {
> struct syscall_tp_t {
> struct trace_entry ent;
> int syscall_nr;
> unsigned long args[SYSCALL_DEFINE_MAXARGS];
> } __aligned(8) param;
> + struct pt_regs regs = {};

Is it possible for this code to leak uninitialized kernel stack memory to
bpf programs?

The param struct is allocated on the stack but not fully initialized. On
32-bit systems, overwriting the first few bytes only partially covers ent,
leaving ent.pid uninitialized.

Additionally, if the system call has fewer than 6 arguments, the trailing
elements of param.args remain uninitialized and could be read by bpf
tracepoint programs.

[ ... ]

> -static int perf_call_bpf_exit(struct trace_event_call *call, struct pt_regs *regs,
> - struct syscall_trace_exit *rec)
> +static int perf_call_bpf_exit(struct trace_event_call *call,
> + int syscall_nr, long ret_val)
> {
> struct syscall_tp_t {
> struct trace_entry ent;
> int syscall_nr;
> unsigned long ret;
> } __aligned(8) param;
> -
> - /* bpf prog requires 'regs' to be the first member in the ctx (a.k.a. &param) */
> - perf_fetch_caller_regs(regs);
> - *(struct pt_regs **)&param = regs;
> - param.syscall_nr = rec->nr;
> - param.ret = rec->ret;
> - return trace_call_bpf(call, &param);
> + struct pt_regs regs = {};

Could this also leak uninitialized padding to bpf programs?

Similar to perf_call_bpf_enter(), param is not zero-initialized here.
On 64-bit systems, compiler-inserted padding between syscall_nr (4 bytes)
and ret (8 bytes) is left uninitialized and could also be read.

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260422-sleepable_tracepoints-v12-0-744bf0e3b311@meta.com?part=3