On Tue, Dec 5, 2023 at 3:22 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > 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@xxxxxxxxxx> > > --- > > Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx> > > > 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. func_id is not actually used anymore, so I'll just drop it altogether. I agree that func_id is a confusing name. > >
