Re: [PATCH v3 bpf-next 08/13] bpf, x86: add support for indirect jumps

Eduard Zingerman
2025-09-21T19:12:31Z

On Fri, 2025-09-19 at 17:28 -0700, Eduard Zingerman wrote:

[...]

> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 5c1e4e37d1f8..839260e62fa9 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c

[...]

> > +/* gotox *dst_reg */
> > +static int check_indirect_jump(struct bpf_verifier_env *env, struct bpf_insn *insn)
> > +{
> > + struct bpf_verifier_state *other_branch;
> > + struct bpf_reg_state *dst_reg;
> > + struct bpf_map *map;
> > + u32 min_index, max_index;
> > + int err = 0;
> > + u32 *xoff;
> > + int n;
> > + int i;
> > +
> > + dst_reg = reg_state(env, insn->dst_reg);
> > + if (dst_reg->type != PTR_TO_INSN) {
> > + verbose(env, "R%d has type %d, expected PTR_TO_INSN\n",
> > + insn->dst_reg, dst_reg->type);
> > + return -EINVAL;
> > + }
> > +
> > + map = dst_reg->map_ptr;
> > + if (verifier_bug_if(!map, env, "R%d has an empty map pointer", insn->dst_reg))
> > + return -EFAULT;
> > +
> > + if (verifier_bug_if(map->map_type != BPF_MAP_TYPE_INSN_ARRAY, env,
> > + "R%d has incorrect map type %d", insn->dst_reg, map->map_type))
> > + return -EFAULT;
> > +
> > + err = indirect_jump_min_max_index(env, insn->dst_reg, map, &min_index, &max_index);
> > + if (err)
> > + return err;
> > +
> > + xoff = kvcalloc(max_index - min_index + 1, sizeof(u32), GFP_KERNEL_ACCOUNT);
> > + if (!xoff)
> > + return -ENOMEM;
>
> Let's keep a buffer for this allocation in `env` and realloc it when needed.
> Would be good to avoid allocating memory each time this gotox is visited.

On a second thought, maybe put this array into bpf_subprog_info for
each function and avoid copy/sort on each gotox instruction as well?

> > +
> > + n = copy_insn_array_uniq(map, min_index, max_index, xoff);
> > + if (n < 0) {
> > + err = n;
> > + goto free_off;
> > + }
> > + if (n == 0) {
> > + verbose(env, "register R%d doesn't point to any offset in map id=%d\n",
> > + insn->dst_reg, map->id);
> > + err = -EINVAL;
> > + goto free_off;
> > + } …