On Sat, Oct 21, 2023 at 03:59:35AM +0300, Eduard Zingerman wrote:
>
> +static struct bpf_verifier_state_list **__explored_state(struct bpf_verifier_env *env,
> + int idx,
> + int callsite);
...
> +static struct bpf_verifier_state *find_prev_entry(struct bpf_verifier_env *env,
> + struct bpf_verifier_state *cur,
> + int insn_idx)
> +{
> + struct bpf_verifier_state_list *sl;
> + struct bpf_verifier_state *st;
> +
> + /* Explored states are pushed in stack order, most recent states come first */
> + sl = *__explored_state(env, insn_idx, cur->frame[cur->curframe]->callsite);
...
> + prev_st = find_prev_entry(env, cur_st->parent, insn_idx);
...
> +static struct bpf_verifier_state_list **__explored_state(struct bpf_verifier_env *env,
> + int idx,
> + int callsite)
> +{
> + return &env->explored_states[(idx ^ callsite) % state_htab_size(env)];
> +}
> +
> static struct bpf_verifier_state_list **explored_state(
> struct bpf_verifier_env *env,
> int idx)
> @@ -15032,7 +15161,7 @@ static struct bpf_verifier_state_list **explored_state(
> struct bpf_verifier_state *cur = env->cur_state;
> struct bpf_func_state *state = cur->frame[cur->curframe];
>
> - return &env->explored_states[(idx ^ state->callsite) % state_htab_size(env)];
> + return __explored_state(env, idx, state->callsite);
> }
Do we really need to introduce this new helper?
I suspect the concern was that cur->callsite != cur->parent->callsite, right?
But that can never be the case, since bpf_iter_num_next() is force checkpoint,
so inside process_iter_next_call() cur_st->parent is guaranteed to be from
the same function and callsites will be the same.
I can undo above and replace with a warn_on (or with a comment) while applying?
