Re: [PATCH bpf-next v9 01/23] bpf/verifier: allow all functions to read user provided context

[Kumar Kartikeya Dwivedi]

On Thu, 1 Sept 2022 at 18:48, Benjamin Tissoires
<benjamin.tissoires@xxxxxxxxxx> wrote:
>
> [...]
> If the above is correct, then yes, it would make sense to me to have 2
> distinct functions: one to check for the args types only (does the
> function definition in the problem matches BTF), and one to check for
> its use.
> Behind the scenes, btf_check_subprog_arg_match() calls
> btf_check_func_arg_match() which is the one function with entangled
> arguments type checking and actually assessing that the values
> provided are correct.
>
> I can try to split that  btf_check_func_arg_match() into 2 distinct
> functions, though I am not sure I'll get it right.

FYI, I've already split them into separate functions in my tree
because it had become super ugly at this point with all the new
support and I refactored it to add the linked list helpers support
using kfuncs (which requires some special handling for the args), so I
think you can just leave it with a "processing_call" check in for your
series for now.

> Maybe the hack about having "processing_call" for
> btf_check_func_arg_match() only will be good enough as a first step
> towards a better solution?
>
> > And may cleanup the rest of that function ?
> > Like all of if (is_kfunc) applies only to 'calling' case.
> > Other ideas?
> >
>
> I was trying to understand the problem most of today, and the only
> other thing I could think of was "why is the assumption that
> PTR_TO_CTX is not NULL actually required?". But again, this question
> is "valid" in the function declaration part, but not in the caller
> insn part. So I think splitting btf_check_subprog_arg_match() in 2 is
> probably the best.
>
> Cheers,
> Benjamin
>