On Thu, 28 Nov 2024 at 04:03, Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Thu, 2024-11-28 at 03:54 +0100, Kumar Kartikeya Dwivedi wrote:
>
> [...]
>
> > > > --- a/kernel/bpf/log.c
> > > > +++ b/kernel/bpf/log.c
> > > > @@ -756,6 +756,7 @@ static void print_reg_state(struct bpf_verifier_env *env,
> > > > void print_verifier_state(struct bpf_verifier_env *env, const struct bpf_func_state *state,
> > > > bool print_all)
> > > > {
> > > > + struct bpf_verifier_state *vstate = env->cur_state;
> > >
> > > This is not always true.
> > > For example, __mark_chain_precision does 'print_verifier_state(env, func, true)'
> > > for func obtained as 'func = st->frame[fr];' where 'st' iterates over parents
> > > of env->cur_state.
> >
> > Looking through the code, I'm thinking the only proper fix is
> > explicitly passing in the verifier state, I was hoping there would be
> > a link from func_state -> verifier_state but it is not the case.
> > Regardless, explicitly passing in the verifier state is probably cleaner. WDYT?
>
> Seems like it is (I'd also pass the frame number, instead of function
> state pointer, just to make it clear where the function state comes from,
> but feel free to ignore this suggestion).
I made this change, but not passing the frame number: while most call
sites have the frame number (or pass curframe), it needs to be
obtained explicitly for some, so I think it won't be worth it.
>
> [...]
>
