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

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 5e37ca75e5c4..038753ef07a9 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.
> +	 */

Is this comment accurate for all the program types below?

For BPF_PROG_TYPE_TRACING / LSM / STRUCT_OPS, private stack is only
granted when bpf_prog_check_recur(prog) holds, and that same predicate
makes bpf_trampoline_enter() install __bpf_prog_enter_sleepable_recur(),
which bumps the per-CPU prog->active counter under migrate_disable() and
skips the nested invocation.

Similarly, the sleepable raw-tracepoint runner __bpf_trace_run() calls
bpf_prog_get_recursion_context().

The statement "the second invocation would corrupt the first's stack"
would be true for sleepable uprobe programs run via uprobe_prog_run(),
which takes no recursion context. Could the comment explain why the check
is placed before the switch rather than in the case arm that actually
needs it?

> +	if (prog->sleepable)
> +		return NO_PRIV_STACK;
> +

Does placing this check before the prog-type switch have unintended
consequences?

The check strips the private stack from BPF_PROG_TYPE_TRACING / LSM /
STRUCT_OPS even though those paths are already protected. For these types,
bpf_trampoline_enter() consults bpf_prog_check_recur(prog):

kernel/bpf/trampoline.c:bpf_trampoline_enter() {
    if (bpf_prog_check_recur(prog))
            return sleepable ? __bpf_prog_enter_sleepable_recur :
                    __bpf_prog_enter_recur;

and __bpf_prog_enter_sleepable_recur() does migrate_disable() and then
bpf_prog_get_recursion_context(prog), which uses a per-CPU prog->active
counter. A second task entering the same program on the same CPU sees
active != 1 and is skipped, so it can never reach the private stack.

The real hole appears to be below the new check: BPF_PROG_TYPE_KPROBE /
TRACEPOINT / PERF_EVENT / RAW_TRACEPOINT return PRIV_STACK_ADAPTIVE
unconditionally. For sleepable uprobes (the only sleepable case allowed by
can_be_sleepable()), the uprobe runner takes no recursion context:

kernel/trace/bpf_trace.c:uprobe_prog_run() {
    if (sleepable)
            rcu_read_lock_trace();
    ...
    migrate_disable();
    err = bpf_prog_run(link->link.prog, regs);

so a sleepable uprobe can genuinely be re-entered on the same CPU.

Moving the check into the case arm (BPF_PROG_TYPE_KPROBE through
BPF_PROG_TYPE_RAW_TRACEPOINT) would close the uprobe hole without taking
the private stack away from the trampoline types that already handle
re-entry.

There's also a user-visible side effect: priv_stack_mode decides how
check_max_stack_depth_subprog() budgets stack. PRIV_STACK_ADAPTIVE checks
each subprog on its own against MAX_BPF_STACK; NO_PRIV_STACK accumulates
depth += subprog_depth across bpf2bpf frames and checks the combined total
against MAX_BPF_STACK (512).

Any existing sleepable program whose bpf2bpf subtree exceeds 512 bytes
total - SEC("fentry.s/..."), SEC("fexit.s/..."), SEC("fmod_ret.s/..."),
SEC("uprobe.s/...") - now fails to load with -EACCES and "combined stack
size of %d calls is %d. Too large", where it verified fine before. The
selftest at tools/testing/selftests/bpf/progs/verifier_private_stack.c:86
("Private stack, subtree > MAX_BPF_STACK", 512-byte main + 32-byte
subprog, __success) has exactly this shape; it only survives because it
uses the non-sleepable SEC("kprobe"). With Cc: [email protected],
this would break working programs on a stable update.

>  	/* 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.

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