On Wed, 2023-12-06 at 09:59 -0800, Andrii Nakryiko wrote: [...] > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > > > index 16d5550eda4d..642260d277ce 100644 > > > --- a/kernel/bpf/verifier.c > > > +++ b/kernel/bpf/verifier.c > > > @@ -19899,18 +19899,6 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog) > > > /* 1st arg to a function */ > > > regs[BPF_REG_1].type = PTR_TO_CTX; > > > mark_reg_known_zero(env, regs, BPF_REG_1); > > > - ret = btf_check_subprog_arg_match(env, subprog, regs); > > > > Not sure if this is important or not. btf_check_subprog_arg_match() > > might have set 'func_info_aux[subprog].unreliable = true'. > > bpf_check_attach_target() checks this flag for subprograms that are > > being replaced, and seem to be ok accepting 'subprog == 0'. > > This change makes it so .unreliable is never set for 'subprog == 0'. > > True, I missed this corner case. But I'm now wondering if it is > actually better to have a dedicated check just for entry program which > explicitly checks that we have one argument and it's a PTR_TO_CTX > (taking into account tags as well). btf_check_subprog_arg_match() > business is way too indirect and subtle, IMO. Dedicated check sounds more direct indeed.
