Re: [PATCH bpf v3 1/2] bpf: disable private stack for sleepable programs

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Non-sleepable BPF uprobe programs remain vulnerable to per-CPU private stack corruption.
--

--- Patch [1]: [PATCH bpf v3 1/2] bpf: disable private stack for sleepable programs ---
commit 5b27c96c855b5a6df31f1d2543a92540c69a5d98
Author: Christian Simon <[email protected]>

bpf: disable private stack for sleepable programs

This commit disables the use of private stacks for sleepable BPF programs
since they can be preempted, which would allow a second invocation of the
program on the same CPU to corrupt the first invocation's stack. The fix
forces sleepable programs to use the regular kernel stack which handles
preemption correctly.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 5e37ca75e5c4a..038753ef07a99 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5237,6 +5237,15 @@ static enum priv_stack_mode bpf_enable_priv_stack(struct bpf_prog *prog)
>  	if (!bpf_jit_supports_private_stack())
>  		return NO_PRIV_STACK;
>  
> +	/*
> +	 * Sleepable programs can be preempted, allowing another task to run
> +	 * the same program on the same CPU. Since private stack is per-CPU
> +	 * and per-program, the second invocation would corrupt the first's
> +	 * stack. Disable private stack for sleepable programs.
> +	 */
> +	if (prog->sleepable)
> +		return NO_PRIV_STACK;
> +
>  	/* bpf_prog_check_recur() checks all prog types that use bpf trampoline
>  	 * while kprobe/tp/perf_event/raw_tp don't use trampoline hence checked
>  	 * explicitly.

[Severity: High]
This is a pre-existing issue and was not introduced by this patch, but does
this leave non-sleepable uprobes vulnerable to the same stack corruption?

The patch correctly fixes sleepable programs, but the commit message mentions
that preemptible non-sleepable dispatch paths require separate protection.
However, non-sleepable uprobes are granted private stacks because they fall
under BPF_PROG_TYPE_KPROBE in the switch statement immediately below this code.

If we look at bpf_prog_run_array_uprobe() in include/linux/bpf.h,
non-sleepable uprobes are executed with only rcu_read_lock():

    if (!prog->sleepable)
        rcu_read_lock();
        
    run_ctx.bpf_cookie = item->bpf_cookie;
    ret &= run_prog(prog, ctx);
    item++;
    
    if (!prog->sleepable)
        rcu_read_unlock();

Under CONFIG_PREEMPT_RCU and CONFIG_PREEMPT, rcu_read_lock() does not disable
preemption. Could a task executing this non-sleepable uprobe be preempted,
allowing another task on the same CPU to trigger the same uprobe and overwrite
its per-CPU private stack?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1