On Fri, Nov 18, 2022 at 09:04:15AM IST, Alexei Starovoitov wrote:
> On Fri, Nov 18, 2022 at 07:26:01AM +0530, Kumar Kartikeya Dwivedi wrote:
> > static int btf_check_func_arg_match(struct bpf_verifier_env *env,
> > const struct btf *btf, u32 func_id,
> > struct bpf_reg_state *regs,
> > bool ptr_to_mem_ok,
> > - struct bpf_kfunc_arg_meta *kfunc_meta,
> > bool processing_call)
>
> Something odd here.
> Benjamin added the processing_call flag in
> commit 95f2f26f3cac ("bpf: split btf_check_subprog_arg_match in two")
> and we discussed to remove it.
>
> > } else if (ptr_to_mem_ok && processing_call) {
>
> since kfunc bit is gone from here the processing_call can be removed.
> ptr_to_mem_ok and processing_call are two bool flags for the same thing, right?
>
I think so, I'll check it out and send a follow up patch.
> > +static int process_kf_arg_ptr_to_kptr_strong(struct bpf_verifier_env *env,
>
> I fixed this bit while applying.
>
Thanks.
> > +static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_arg_meta *meta)
>
> This function looks much better now.
> The split of kfunc vs helper was long overdue.
> Thank you for doing this.
>
> I'm not convinced that KF_ARG_* is necessary, but it's much better than before.
> So it's a step forward.
>
Yes. Eventually we should be merging checks for both helpers and kfuncs, but
that needs more work and would have been out of scope for this set. We can
probably synthesize a bpf_func_proto for the kfunc from BTF and then offload to
check_helper_call.
> Pls watch for CI errors and follow up when necessary.
Will do.
Thanks!
