From: sashiko-bot@kernel.org
To: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>, bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v4 21/46] bpf: Verify static subprogs on their own
Date: Sat, 07 Nov 2026 04:53:09 +0000 [thread overview]
Message-ID: <sashiko-outbox-202596@kernel.org> (raw)
In-Reply-To: <20261107033758.1547725-22-alexei.starovoitov@gmail.com>
Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [High] bpf: bpf_flow_return() drops speculative state flag on callee
exit
- [Medium] bpf: call_func_on_its_own() loses async callback context in
frame[0]
- [Medium] bpf: add_insns_of_callees() double-counts insns and misses
callx edges
- [Medium] bpf: bpf_flow_return() lazy fallback leaves stale post-call
states
--
commit 9af28e54478c87831c1e29826a9fb6f36a5e5256
Author: Alexei Starovoitov <ast@kernel.org>
bpf: Verify static subprogs on their own
This commit verifies eligible static subprograms independently of their
callers' stack frames using a single-frame entry state and remapped
inherited reference IDs.
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 0a315a88e355a..3c3d1c8ea4529 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -11849,6 +11852,185 @@ static void account_current_path(struct bpf_verifier_env *env);
[ ... ]
> +static bool func_is_on_its_own(struct bpf_verifier_env *env, struct bpf_verifier_state *state,
> + int subprog)
> +{
> + struct bpf_func_state *caller = state->frame[state->curframe];
> + struct bpf_subprog_info *sub = &env->subprog_info[subprog];
> + u16 live = env->insn_aux_data[sub->start].live_regs_before;
> + u32 i;
> +
> + if (sub->changes_pkt_data || sub->has_tail_call || sub->has_ld_abs ||
> + sub->is_cb || sub->is_async_cb || sub->is_exception_cb)
> + return false;
> + if (state->speculative)
> + return false;
> + if (sub->releases_inherited &&
> + (state->acquired_refs || state->active_rcu_locks || state->active_preempt_locks))
> + return false;
[Severity: Medium]
Can setting sub->releases_inherited (or sub->changes_pkt_data) lazily in
bpf_flow_return() leave stale post-call states at call_idx + 1 when a
subprogram only releases a caller-held resource on one branch?
When a non-releasing exit of the callee is processed first,
flow_apply_exit() -> bpf_flow_return() sees kept == nheld and delivers a
post-call state with the resource still held to call_idx + 1. When the
releasing exit later reaches bpf_flow_return(), it sets
sub->releases_inherited = true and returns 1 to re-verify the call in the
caller's context, but the state already delivered to call_idx + 1 is not
retracted and can cause check_resource_leak() to reject a valid program.
> + /* what must not be done in that callback is refused by looking at the frames */
> + if (in_rbtree_lock_required_cb(env))
> + return false;
> + /* there are that many names for what is held, and bpf_flow_return() has room for them */
> + if (state->acquired_refs > BPF_ID_MAP_SIZE / 2)
> + return false;
> + for (i = BPF_REG_1; i <= BPF_REG_5; i++) {
> + if (!(live & BIT(i)))
> + continue;
> + if (caller->regs[i].type == PTR_TO_STACK ||
> + base_type(caller->regs[i].type) == CONST_PTR_TO_DYNPTR)
> + return false;
> + if (arg_needs_caller(env, state, sub, &caller->regs[i]))
> + return false;
> + }
> + for (i = 0; i < bpf_in_stack_arg_cnt(sub); i++) {
> + if (caller->stack_arg_regs[i].type == PTR_TO_STACK ||
> + base_type(caller->stack_arg_regs[i].type) == CONST_PTR_TO_DYNPTR)
> + return false;
> + if (arg_needs_caller(env, state, sub, &caller->stack_arg_regs[i]))
> + return false;
> + }
> + return true;
> +}
> +
> +static int call_func_on_its_own(struct bpf_verifier_env *env, int subprog, int insn_idx)
> +{
> + u16 live = env->insn_aux_data[env->subprog_info[subprog].start].live_regs_before;
> + struct bpf_verifier_state *state = env->cur_state;
> + struct bpf_func_state *caller = state->frame[state->curframe], *callee;
> + struct bpf_verifier_state entry = {};
> + int i, err;
> +
> + callee = kzalloc_obj(*callee, GFP_KERNEL_ACCOUNT);
> + if (!callee)
> + return -ENOMEM;
> + entry.frame[0] = callee;
> + entry.in_sleepable = state->in_sleepable;
> + entry.callers_unknown = state->callers_unknown;
> + init_func_state(env, callee, BPF_MAIN_FUNC, 0, subprog);
> + /* the frame that says so is not there when the function is verified */
> + callee->in_rcu_callback_fn = in_rcu_callback(env);
[Severity: Medium]
Does replacing entry.frame[0] with callee lose
state->frame[0]->in_async_callback_fn and callback_ret_range when a static
subprogram is called from an async callback?
kernel/bpf/verifier.c:has_rcu_read_lock() {
if (state->active_rcu_locks || in_rcu_callback(env))
return true;
if (in_sleepable(env))
return false;
return !env->no_rcu_read_lock || state->frame[0]->in_async_callback_fn;
}
When a global subprogram is verified for BPF_FUNC_CTX_NO_RCU_LOCK
(env->no_rcu_read_lock == true) and schedules a non-sleepable async
callback such as a bpf_timer callback, has_rcu_read_lock() relies on
state->frame[0]->in_async_callback_fn. Without copying
in_async_callback_fn and callback_ret_range to callee, has_rcu_read_lock()
returns false inside the static subprogram (rejecting RCU-protected map
helpers and misclassifying nested global calls), and check_return_code()
on a default bpf_throw() checks the main program's return range instead of
the async callback's.
> + for (i = BPF_REG_1; i <= BPF_REG_5; i++)
> + if (live & BIT(i))
> + callee->regs[i] = caller->regs[i];
[ ... ]
> +/* The current state is at exit of a function that is verified on its own. */
> +static int exit_func_on_its_own(struct bpf_verifier_env *env)
> +{
> + struct bpf_verifier_state *state = env->cur_state;
> + struct bpf_func_state *callee = state->frame[0];
> + u32 i, nregs;
> + int err;
> +
> + nregs = bpf_ret_reg_pair(env, callee->subprogno) ? 2 : 1;
> + for (i = 0; i < nregs; i++) {
> + if (callee->regs[ret_regs[i]].type != PTR_TO_STACK)
> + continue;
> + verbose(env, "cannot return stack pointer to the caller\n");
> + return -EINVAL;
> + }
> + state->unwound |= bpf_is_unwind_exit(&env->prog->insnsi[env->insn_idx]);
> +
> + /* Invalidate callee-local dynptrs and their slices before the frame goes away. */
> + err = destroy_dynptrs_in_stack_slots(env, callee, 0,
> + callee->allocated_stack / BPF_REG_SIZE - 1);
> + if (err)
> + return err;
> +
> + account_current_path(env);
> + err = bpf_flow_exit(env, state, env->insn_idx);
> + return err ?: PROCESS_BPF_EXIT;
> +}
[Severity: High]
As reported by bot+bpf-ci@kernel.org on v2 [1], does returning from a
speculative path via exit_func_on_its_own() and bpf_flow_return() drop
exit->speculative?
Although func_is_on_its_own() rejects speculative caller states at entry,
a conditional branch or pointer ALU operation inside the callee can spawn
a speculative path with state->speculative = true. When that path reaches
BPF_EXIT, flow_apply_exit() copies the non-speculative waiting caller
state and calls bpf_flow_return(), which does not propagate
exit->speculative to caller->speculative. The speculative return state is
then delivered to the caller as a normal architectural state, marking
architecturally dead instructions as seen and turning recoverable
speculative faults into fatal verification errors. In addition,
flow_merge() -> flow_within() allows a non-speculative state from another
call entry to subsume a speculative state in the callee and cross-unite
their flow_entry sets via flow_unite().
[ ... ]
> @@ -23532,6 +23726,37 @@ static int do_check_all(struct bpf_verifier_env *env)
> }
> }
>
> +/*
> + * What it took to verify a static subprog on its own is not in insns_total of its callers.
> + * Add it, once per caller. The main prog and global subprogs have all of it already.
> + */
> +static void add_insns_of_callees(struct bpf_verifier_env *env)
> +{
> + struct bpf_subprog_info *si = env->subprog_info;
> + struct bpf_insn *insn = env->prog->insnsi;
> + int *added, i, j, s, c;
> +
> + added = kvcalloc(env->subprog_cnt, sizeof(*added), GFP_KERNEL_ACCOUNT);
> + if (!added)
> + return;
> + /* callees first */
> + for (i = 0; i < env->subprog_cnt; i++) {
> + s = env->subprog_topo_order[i];
> + if (!s || bpf_subprog_is_global(env, s))
> + continue;
> + for (j = si[s].start; j < si[s + 1].start; j++) {
> + if (!bpf_pseudo_call(&insn[j]))
> + continue;
> + c = bpf_find_subprog(env, j + insn[j].imm + 1);
> + if (c <= 0 || c == s || !si[c].on_its_own || added[c] == s)
> + continue;
> + added[c] = s;
> + si[s].insns_total += si[c].insns_total;
> + }
> + }
> + kvfree(added);
> +}
[Severity: Medium]
As reported by bot+bpf-ci@kernel.org on v2 [2], can add_insns_of_callees()
double-count si[c].insns_total and miss indirect callx callees?
When a static callee c is verified on its own at one call site (or sets
on_its_own before bpf_flow_return() falls back to in-frame verification,
or in an earlier do_check_all() round) and in-frame at another call from
static subprogram s, prepare_func_exit() -> account_processed_insns() has
already added the in-frame instructions to both si[c].insns_total and
si[s].insns_total, so adding si[c].insns_total here counts them a second
time. Conversely, static subprograms called via callx and verified on
their own via check_func_callx() -> check_static_func_call() are skipped
because the loop only checks bpf_pseudo_call().
[1] https://lore.kernel.org/r/fbc6a76bd8cb0b115a602fe1f8064612f09afac8924a23ae5d64cf4e7fe68b43@mail.kernel.org
[2] https://lore.kernel.org/r/fbc6a76bd8cb0b115a602fe1f8064612f09afac8924a23ae5d64cf4e7fe68b43@mail.kernel.org
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261107033758.1547725-1-alexei.starovoitov@gmail.com?part=21
next prev parent reply other threads:[~2026-11-07 4:53 UTC|newest]
Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-07 3:37 [PATCH bpf-next v4 00/46] bpf: A verifier that joins states Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 01/46] bpf: Add may_goto to the loops that the verifier marks Alexei Starovoitov
2026-11-07 4:12 ` sashiko-bot
2026-11-07 3:37 ` [PATCH bpf-next v4 02/46] bpf: Limit depth and time of recursive subprogs at run time Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 03/46] bpf: Add union and widening of the range of a register Alexei Starovoitov
2026-11-07 4:19 ` bot+bpf-ci
2026-11-07 3:37 ` [PATCH bpf-next v4 04/46] bpf: Add join of registers and stack slots Alexei Starovoitov
2026-11-07 4:18 ` sashiko-bot
2026-11-07 3:37 ` [PATCH bpf-next v4 05/46] bpf: Add join of verifier states Alexei Starovoitov
2026-11-07 4:18 ` sashiko-bot
2026-11-07 3:37 ` [PATCH bpf-next v4 06/46] bpf: Tell which arguments of a call a join must leave alone Alexei Starovoitov
2026-11-07 4:19 ` bot+bpf-ci
2026-11-07 3:37 ` [PATCH bpf-next v4 07/46] bpf: Add the data structures of the main pass over the control flow graph Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 08/46] bpf: Order the insns, find what stops a loop and where may_goto can go Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 09/46] bpf: Find the registers that hold a number or a pointer to arena Alexei Starovoitov
2026-11-07 4:11 ` sashiko-bot
2026-11-07 3:37 ` [PATCH bpf-next v4 10/46] bpf: Find the registers and stack slots that later checks depend on Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 11/46] bpf: Add the sets of states at an insn and the entries of functions Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 12/46] bpf: Compare states of different iterations of a loop Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 13/46] bpf: Decide which states are not joined Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 14/46] bpf: Merge the state that arrives into the set at an insn Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 15/46] bpf: Move states along edges, into loops and subprogs and out of them Alexei Starovoitov
2026-11-07 4:19 ` bot+bpf-ci
2026-11-07 3:37 ` [PATCH bpf-next v4 16/46] bpf: Make the state after a call from the caller's state and an exit state Alexei Starovoitov
2026-11-07 4:12 ` sashiko-bot
2026-11-07 3:37 ` [PATCH bpf-next v4 17/46] bpf: Enter and leave loops in the main pass, build flow.c Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 18/46] bpf: Tell the passes over the graph where an added may_goto goes Alexei Starovoitov
2026-11-07 4:10 ` sashiko-bot
2026-11-07 3:37 ` [PATCH bpf-next v4 19/46] bpf: Verify programs as a fixpoint over the control flow graph Alexei Starovoitov
2026-11-07 4:33 ` bot+bpf-ci
2026-11-07 4:40 ` sashiko-bot
2026-11-07 3:37 ` [PATCH bpf-next v4 20/46] bpf: Keep the count of a loop when a register has no closed form Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 21/46] bpf: Verify static subprogs on their own Alexei Starovoitov
2026-11-07 4:53 ` sashiko-bot [this message]
2026-11-07 3:37 ` [PATCH bpf-next v4 22/46] bpf: Allow recursive subprogs Alexei Starovoitov
2026-11-07 4:33 ` bot+bpf-ci
2026-11-07 4:48 ` sashiko-bot
2026-11-07 3:37 ` [PATCH bpf-next v4 23/46] bpf: Remove is_state_visited(), bpf_compute_scc() and iter.depth Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 24/46] bpf: Remove precision tracking Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 25/46] bpf: Remove prune points, levels of state comparison and pop_stack() Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 26/46] bpf: Update comments about paths, pruning and explored states Alexei Starovoitov
2026-11-07 4:03 ` sashiko-bot
2026-11-07 3:37 ` [PATCH bpf-next v4 27/46] bpf: Track a pointer to one of a set of functions Alexei Starovoitov
2026-11-07 4:31 ` sashiko-bot
2026-11-07 3:37 ` [PATCH bpf-next v4 28/46] bpf: callx reads what the subprogs whose address is taken read Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 29/46] bpf: Keep the first iteration of a counted loop apart Alexei Starovoitov
2026-11-07 4:34 ` sashiko-bot
2026-11-07 3:37 ` [PATCH bpf-next v4 30/46] bpf: What a call returns in each iteration is like what the loop loads Alexei Starovoitov
2026-11-07 4:26 ` sashiko-bot
2026-11-07 3:37 ` [PATCH bpf-next v4 31/46] bpf: Find where the jumps of a loop stop a number Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 32/46] bpf: A loop is one iteration shorter behind the jump that counts it Alexei Starovoitov
2026-11-07 4:19 ` bot+bpf-ci
2026-11-07 4:28 ` sashiko-bot
2026-11-07 3:37 ` [PATCH bpf-next v4 33/46] bpf: Give the places of a back edge to the jumps that count Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 34/46] bpf: Count a loop whose exit tests a copy of the counter Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 35/46] bpf: Allow tail calls and LD_ABS in subprogs that return void Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 36/46] bpf: A part of a pointer that is spilled is a number Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 37/46] bpf: 32-bit add to a pointer and pointer op pointer make " Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 38/46] selftests/bpf: Adjust tests to log lines that are gone Alexei Starovoitov
2026-11-07 4:19 ` bot+bpf-ci
2026-11-07 3:37 ` [PATCH bpf-next v4 39/46] selftests/bpf: Adjust tests to new verdicts, messages and order Alexei Starovoitov
2026-11-07 4:24 ` sashiko-bot
2026-11-07 3:37 ` [PATCH bpf-next v4 40/46] selftests/bpf: Test states that are not joined Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 41/46] selftests/bpf: Test static subprogs that are verified on their own Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 42/46] selftests/bpf: Test loops that are counted, walked or widened Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 43/46] selftests/bpf: Test progs that run out of budget Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 44/46] selftests/bpf: Test callx of one of a set of functions Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 45/46] selftests/bpf: Test pointers that become numbers and subprogs that return void Alexei Starovoitov
2026-11-07 3:37 ` [PATCH bpf-next v4 46/46] selftests/bpf: Test progs without bpf_for(), zero, can_loop and such Alexei Starovoitov
2026-11-07 4:19 ` bot+bpf-ci
2026-11-07 4:37 ` sashiko-bot
2026-11-07 5:00 ` [PATCH bpf-next v4 00/46] bpf: A verifier that joins states patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=sashiko-outbox-202596@kernel.org \
--to=sashiko-bot@kernel.org \
--cc=alexei.starovoitov@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox