From: Eduard Zingerman <eddyz87@gmail.com>
To: sashiko-reviews@lists.linux.dev
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf 1/2] bpf: backtracking shouldn't clear outer frame R1-R5 for callbacks
Date: Mon, 31 Aug 2026 23:20:48 -0700 [thread overview]
Message-ID: <836e4262927db41e736a461c45cd74c106525d1c.camel@gmail.com> (raw)
In-Reply-To: <20260901020211.6F4871F000E9@smtp.kernel.org>
On Tue, 2026-09-01 at 02:02 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Flawed heuristic for distinguishing subprog exits from callback exits causes the verifier to erroneously reject safe programs that call a subprog immediately before a looping callback.
> --
>
> commit 09e9430ffafbb3bf72d2c301a4d20da098050142
> Author: Eduard Zingerman <eddyz87@gmail.com>
>
> bpf: backtracking shouldn't clear outer frame R1-R5 for callbacks
>
> This commit fixes precision propagation for R1 during a second iteration of
> bpf_loop(). It removes the logic in backtrack_insn() that previously cleared
> R1-R5 when backtracking through a callback's BPF_EXIT, ensuring precision
> marks are not incorrectly reset.
>
> > diff --git a/kernel/bpf/backtrack.c b/kernel/bpf/backtrack.c
> > index a2b18a9f1694c..4fe9065106739 100644
> > --- a/kernel/bpf/backtrack.c
> > +++ b/kernel/bpf/backtrack.c
> > @@ -520,37 +520,34 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
> [ ... ]
> > } else if (opcode == BPF_EXIT) {
> > - bool r0_precise;
> > + bool from_subprog_call, r0_precise;
> > +
> > + /* BPF_EXIT in subprog or callback always returns
> > + * right after the call instruction, so by checking
> > + * whether the instruction at subseq_idx-1 is subprog
> > + * call or not we can distinguish actual exit from
> > + * *subprog* from exit from *callback*. In the former
> > + * case, we need to propagate r0 precision, if
> > + * necessary. In the former we never do that.
> > + */
> > + from_subprog_call = subseq_idx - 1 >= 0 &&
> > + bpf_pseudo_call(&env->prog->insnsi[subseq_idx - 1]);
>
> [Severity: High]
> Can this heuristic misclassify a callback exit as a subprog exit?
>
> If a program calls a subprog immediately followed by an iterating callback
> helper (like bpf_loop()), and we backtrack from the callback's BPF_EXIT,
> subseq_idx equals the index of bpf_loop(). Checking subseq_idx - 1 evaluates
> the instruction before bpf_loop(), which happens to be the subprog call.
>
> This makes from_subprog_call incorrectly evaluate to true for the callback.
But that can't really happen. If the code looks like this:
foo(...);
bpf_loop(...);
There should be r1-r4 setup for bpf_loop() call, because foo()
invalidates those.
> > +
> > + r0_precise = from_subprog_call && bt_is_reg_set(bt, BPF_REG_0);
> >
> > /* Backtracking to a nested function call, 'idx' is a part of
> > * the inner frame 'subseq_idx' is a part of the outer frame.
> > * In case of a regular function call, instructions giving
> > * precision to registers R1-R5 should have been found already.
> > - * In case of a callback, it is ok to have R1-R5 marked for
> > - * backtracking, as these registers are set by the function
> > - * invoking callback.
> > + * In case of a callback from bpf_loop(), R{1,4} in the calling
> > + * frame would be set as precise and that is correct.
> > */
> > - if (subseq_idx >= 0 && bpf_calls_callback(env, subseq_idx))
> > - for (i = BPF_REG_1; i <= BPF_REG_5; i++)
> > - bt_clear_reg(bt, i);
> > - if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) {
> > + if (from_subprog_call && (bt_reg_mask(bt) & BPF_REGMASK_ARGS)) {
> > verifier_bug(env, "backtracking exit unexpected regs %x",
> > bt_reg_mask(bt));
> > return -EFAULT;
> > }
>
> [Severity: High]
> Will this incorrectly trigger the verifier bug check for callbacks?
>
> Because bpf_loop() requests R1 precision (which falls under BPF_REGMASK_ARGS),
> the misclassified from_subprog_call causes this safety check to evaluate to
> true. Could this prematurely abort verification with -EFAULT and falsely reject
> perfectly safe BPF programs?
See above.
next prev parent reply other threads:[~2026-09-01 6:20 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-01 1:36 [PATCH bpf 1/2] bpf: backtracking shouldn't clear outer frame R1-R5 for callbacks Eduard Zingerman
2026-09-01 1:36 ` [PATCH bpf 2/2] selftests/bpf: test case for unsafe pruning of bpf_loop checkpoints Eduard Zingerman
2026-09-01 2:02 ` [PATCH bpf 1/2] bpf: backtracking shouldn't clear outer frame R1-R5 for callbacks sashiko-bot
2026-09-01 6:20 ` Eduard Zingerman [this message]
2026-09-02 18:20 ` 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=836e4262927db41e736a461c45cd74c106525d1c.camel@gmail.com \
--to=eddyz87@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