Re: [PATCH v2 bpf-next 4/6] bpf: Simulate branches to prune based on range violations

Harishankar Vishwanathan
2026-03-24T20:36:19Z

On Mon, Mar 23, 2026 at 12:19 PM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> Paul Chaignon <paul.chaignon@gmail.com> writes:
>
> > From: Harishankar Vishwanathan <harishankar.vishwanathan@gmail.com>
> >
[...]
> > +static void regs_refine_cond_op(struct bpf_reg_state *reg1, struct bpf_reg_state *reg2,
> > + u8 opcode, bool is_jmp32);
> > +static u8 rev_opcode(u8 opcode);
> > +
> > +/* Learn more information about live branches by simulating both branches being
> > + * taken using regs_refine_cond_op. Because regs_refine_cond_op is sound when
> > + * the branch is taken, if it produces ill-formed register bounds, it must mean
> > + * that the branch is dead.
> > + */
> Sorry for being nit-picky, could you please use kernel style comment style +

Thanks for the suggestion. To clarify, by kernel-style did you mean
keeping the beginning line empty?
Or something else that I might have missed.

> maybe reword it a little bit, instead of:
>
> /*
> * Because regs_refine_cond_op is sound when
> * the branch is taken, if it produces ill-formed register bounds, it must mean
> * that the branch is dead.
> */
> Something like:
> /*
> * regs_refine_cond_op() is sound, so producing ill-formed register
> * bounds for the branch means that branch is dead.
> */
>
This sounds good.

> > +static int simulate_both_branches_taken(struct bpf_verifier_env *env, u8 opcode, bool is_jmp32)
> > +{
> > + /* Fallthrough (FALSE) branch */
> > + regs_refine_cond_op(&env->false_reg1, &env->false_reg2, rev_opcode(opcode), is_jmp32);
> > + reg_bounds_sync(&env->false_reg1);
> > + reg_bounds_sync(&env->false_reg2);
> This is probably more related to patch 2: is it necessary to have both
> true/false_reg1/2 pairs, it looks like we process false branch before
> the true branch and they never intersect, don't they? So it worth
> removing a pair of bpf_reg_states from env?

We do process the false branch before the true branch in
is_branch_taken->simulate_both_branches. But when both branches are possible
(we return -1), we need all the four env buffers to hold the updated results
of both branch simulations simultaneously.

The new verifier state for other_branch hasn't been created at the time
simulate_both_branches is called, so we cannot copy the results to their
final destinations immediately. We must hold them in the buffers until
after push_stack,
after which we can copy them back into their corresponding register states:

copy_register_state(dst_reg, &env->false_reg1);
copy_register_state(src_reg, &env->false_reg2);
copy_register_state(&other_branch_regs[insn->dst_reg], &env->true_reg1);
if (BPF_SRC(insn->code) == BPF_X)
copy_register_state(&other_branch_regs[insn->src_reg], &env->true_reg2); …