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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.