Re: [PATCH v2 bpf-next 3/3] selftests/bpf: Add a runtime test for private stack and stack arguments

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 4173 bytes --]

> selftests/bpf: Add a runtime test for private stack and stack arguments

TL;DR: Commit message only: it may be inaccurate in saying fentry/fexit tests
are left at NO_PRIV_STACK, since tracing programs that recurse are eligible
for a private stack; the review suggests rewording.

>     selftests/bpf: Add a runtime test for private stack and stack arguments
>
>     No existing test reaches the combination. bpf_enable_priv_stack() grants
>     an adaptive private stack only to kprobe, tracepoint, perf_event and
>     raw_tracepoint programs, and to tracing, lsm and struct_ops ones that
>     request it or recurse. The stack-argument tests run from tc, socket,
>     syscall and fentry/fexit, all left at NO_PRIV_STACK; the private-stack
>     tests call subprograms of up to three arguments, which stay in
>     registers. Neither pairs a private stack with outgoing stack arguments,
>     where the caller writes the argument area relative to SP and the callee
>     reads it relative to the frame pointer.
>
>     Add a runtime test for that combination. A static ten-argument callee
>     writes through a pointer into the caller frame and keeps a 128-byte
>     volatile local array, so its own depth crosses the 64-byte threshold at
>     which the verifier grants a subprog the adaptive private stack. The same
>     workload runs from a raw tracepoint, which gets a private stack, and
>     from tc, which does not; the test pins itself to each allowed CPU in
>     turn and checks the CPU the program reported.
>
>     A runtime result cannot show where the three stack areas landed, so the
>     riscv64 __jited patterns pin the layout: the caller's kernel frame is 48
>     bytes against the control group's 176 and the callee's 64 against 208,
>     the 128 and 144 byte differences being exactly the BPF stack that moved
>     to the per-CPU area. Both groups write the two outgoing arguments at
>     0x0(sp) and 0x8(sp) and the callee reads them back at 0x0(s0) and
>     0x8(s0), so the outgoing area, the saved registers and the BPF stack
>     occupy disjoint ranges. None of the patterns assert the per-CPU lookup,
>     which is only emitted on CONFIG_SMP, so one set matches SMP and UP
>     alike.
>
>     The zero-depth struct_ops program is a control case for the kernel
>     stack path, not a private-stack allocation: the verifier grants the
>     adaptive private stack only once a subprog's stack depth, rounded up to
>     the JIT's 16-byte granularity, reaches 64 bytes.
>
>     Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>

The commit message says the stack-argument tests run from "tc, socket,
syscall and fentry/fexit, all left at NO_PRIV_STACK".  Is that accurate
for fentry/fexit?

Those are BPF_PROG_TYPE_TRACING programs, and bpf_enable_priv_stack() does:

    case BPF_PROG_TYPE_TRACING:
    case BPF_PROG_TYPE_LSM:
    case BPF_PROG_TYPE_STRUCT_OPS:
        if (prog->aux->priv_stack_requested || bpf_prog_check_recur(prog))
            return PRIV_STACK_ADAPTIVE;

bpf_prog_check_recur() returns true for TRACING unless expected_attach_type
is BPF_TRACE_ITER, so the fentry/fexit tracers in
progs/tracing_stack_arg_tailcall_tracer.c and
progs/tracing_struct_int128_tracer.c get PRIV_STACK_ADAPTIVE at the program
level.  That is the "tracing ... ones that ... recurse" case named in the
preceding sentence.

The real reasons those tracers never pair a private stack with stack
arguments seem to be different.  check_max_stack_depth_subprog() only gives
a subprog PRIV_STACK_ADAPTIVE when its rounded depth reaches
BPF_PRIV_STACK_MIN_SIZE (64), and these programs only copy a few ctx[]
slots into globals.  They also make no outgoing stack-argument calls; the
stack arguments belong to the traced syscall-program target.

Could the sentence say that the fentry/fexit tracers are eligible for a
private stack, but stay below the 64-byte threshold and make no
stack-argument calls?


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