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

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] Uninitialized kernel stack memory leaked to BPF programs via `param` structure.
- [High] Moving BPF program execution before `syscall_get_data()` breaks `bpf_probe_read_user()` for non-sleepable programs by removing implicit page faulting.
--

commit 6d42174eb2c9a4cc877bdf6b63e5b37b8017a109
Author: Mykyta Yatsenko <yatsenko@meta.com>

bpf: Add sleepable support for classic tracepoint programs

This commit adds support for sleepable BPF programs on syscall tracepoints
by moving the BPF program execution out of the non-preemptible ring buffer
allocation path. It introduces trace_call_bpf_faultable() and restructures
perf_syscall_enter() and perf_syscall_exit().

> 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 = {};

Could param be zero-initialized here? 

Because param is allocated on the kernel stack without zero-initialization,
there is a 4-byte padding between syscall_nr and args. Additionally, only
sys_data->nb_args elements of the args array are initialized, leaving the
remaining elements uninitialized.

Since the BPF verifier allows tracing programs to read the full context size,
this padding and uninitialized array elements can be read by BPF programs,
leaking kernel stack memory. Zero-initializing param (e.g., param = {})
would prevent this.

[ ... ]

> @@ -1429,6 +1424,26 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
>  
>  	syscall_get_arguments(current, regs, args);
>  
> +	/*
> +	 * Run BPF program in faultable context before per-cpu buffer
> +	 * allocation, allowing sleepable BPF programs to execute.
> +	 */
> +	valid_prog_array = bpf_prog_array_valid(sys_data->enter_event);
> +	if (valid_prog_array &&
> +	    !perf_call_bpf_enter(sys_data->enter_event, sys_data,
> +				 syscall_nr, args))
> +		return;
> +

Does moving BPF program execution here break bpf_probe_read_user() for
existing non-sleepable BPF programs?

Previously, BPF programs ran after syscall_get_data() was called, which
explicitly faulted in user memory for string arguments. Because the memory
became resident, non-sleepable BPF programs could use bpf_probe_read_user()
to reliably read those arguments without taking page faults.

Now that the BPF program runs before syscall_get_data(), if the user memory
is paged out, bpf_probe_read_user() in non-sleepable programs might fail
with -EFAULT. 

[ ... ]

> -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 = {};

Similarly to perf_call_bpf_enter(), could param be zero-initialized here
to avoid leaking kernel stack memory through the padding between syscall_nr
and ret?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260422-sleepable_tracepoints-v13-0-99005dff21ef@meta.com?part=3