Re: [PATCH 2/2] [net-next] bpf: fix out-of-bounds access warning in bpf_check

[Alexei Starovoitov]

On Thu, Nov 02, 2017 at 05:14:00PM +0100, Arnd Bergmann wrote:
> On Thu, Nov 2, 2017 at 4:59 PM, Alexei Starovoitov
> <alexei.starovoitov@xxxxxxxxx> wrote:
> > On Thu, Nov 02, 2017 at 12:05:52PM +0100, Arnd Bergmann wrote:
> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> >> index 750aff880ecb..debb60ad08ee 100644
> >> --- a/kernel/bpf/verifier.c
> >> +++ b/kernel/bpf/verifier.c
> >> @@ -4447,6 +4447,10 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr)
> >>       struct bpf_verifer_log *log;
> >>       int ret = -EINVAL;
> >>
> >> +     /* no program is valid */
> >> +     if (ARRAY_SIZE(bpf_verifier_ops) == 0)
> >> +             return -EINVAL;
> >
> > sorry I don't see how bpf_verifier_ops can be empty.
> > Did you mix it up with your previous patch when you made bpf_analyzer_ops empty?
> 
> I confused the two a couple of times while creating the patches, but
> I'm still fairly
> sure I got it right in the end:
> 
> bpf_verifier_ops is an array that gets generated by including linux/bpf_types.h.
> That file has two kinds of entries:
> 
> - BPF_MAP_TYPE() entries are left out, as that macro is defined to an
> empty string
>   here.
> 
> - BPF_PROG_TYPE() entries are conditional depending on CONFIG_NET and
>   CONFIG_BPF_EVENTS. In the configuration that produces the warning,
>   both are disabled.

I see. Didn't realize that it's possible to enable bpf syscall
without networking and tracing support.
I'm thinking whether it's better to disallow such uselss mode in kconfig,
but it's probably going to be convoluted.
Above if (ARRAY_SIZE(bpf_verifier_ops) == 0) will be optimized away
by gcc in 99.9% of configs, so I guess that's fine, so:
Acked-by: Alexei Starovoitov <ast@xxxxxxxxxx>