Re: [PATCH bpf-next 09/13] bpf: reuse subprog argument parsing logic for subprog call checks

[Eduard Zingerman <eddyz87@gmail.com>]

On Mon, 2023-12-04 at 15:39 -0800, Andrii Nakryiko wrote:
> Remove duplicated BTF parsing logic when it comes to subprog call check.
> Instead, use (potentially cached) results of btf_prepare_func_args() to
> abstract away expectations of each subprog argument in generic terms
> (e.g., "this is pointer to context", or "this is a pointer to memory of
> size X"), and then use those simple high-level argument type
> expectations to validate actual register states to check if they match
> expectations.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

>  kernel/bpf/verifier.c                         | 109 ++++++------------
>  .../selftests/bpf/progs/test_global_func5.c   |   2 +-
>  2 files changed, 37 insertions(+), 74 deletions(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 2103f94b605b..5787b7fd16ba 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -9214,21 +9214,23 @@ static int setup_func_entry(struct bpf_verifier_env *env, int subprog, int calls
>  	return err;
>  }
>  
> -static int btf_check_func_arg_match(struct bpf_verifier_env *env,
> +static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
>  				    const struct btf *btf, u32 func_id,
> -				    struct bpf_reg_state *regs,
> -				    bool ptr_to_mem_ok)
> +				    struct bpf_reg_state *regs)

Nit: It looks like 'func_id' is always 'prog->aux->func_info[subprog].type_id'.
     Maybe remove this parameter and retrieve func_id inside this function?
     Or at-least, could you please rename it to subprog_btf_id? 
     For me names 'subprog' and 'func_id' seem interchangeable and thus confusing.