BPF List
 help / color / mirror / Atom feed
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.

  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