Re: [PATCH bpf-next 06/13] bpf: remove unnecessary and (mostly) ignored BTF check for main program

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Dec 5, 2023 at 3:21 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Mon, 2023-12-04 at 15:39 -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.

I'll think a bit more and see what's the best way forward, thanks for
spotting this!

> [...]
>
>





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux