netdev - Re: [PATCH bpf-next 6/6] bpf: BPF_PROG_TYPE_CGROUP_{SKB,SOCK,SOCK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181215204015.GB20955@mini-arch.hsd1.ca.comcast.net>
Date:   Sat, 15 Dec 2018 12:40:15 -0800
From:   Stanislav Fomichev <sdf@...ichev.me>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Stanislav Fomichev <sdf@...gle.com>,
        Network Development <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Daniel Borkmann <daniel@...earbox.net>,
        Edward Cree <ecree@...arflare.com>,
        Quentin Monnet <quentin.monnet@...ronome.com>
Subject: Re: [PATCH bpf-next 6/6] bpf:
 BPF_PROG_TYPE_CGROUP_{SKB,SOCK,SOCK_ADDR} require cgroups enabled

On 12/14, Alexei Starovoitov wrote:
> On Thu, Dec 13, 2018 at 11:03 AM Stanislav Fomichev <sdf@...gle.com> wrote:
> >
> > There is no way to exercise appropriate attach points without cgroups
> > enabled. This lets test_verifier correctly skip tests for these
> > prog_types if kernel was compiled without BPF cgroup support.
> >
> > Signed-off-by: Stanislav Fomichev <sdf@...gle.com>
> > ---
> >  include/linux/bpf_types.h |  2 ++
> >  net/core/filter.c         | 18 ++++++++++++++++++
> >  2 files changed, 20 insertions(+)
> >
> > diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> > index 44d9ab4809bd..08bf2f1fe553 100644
> > --- a/include/linux/bpf_types.h
> > +++ b/include/linux/bpf_types.h
> > @@ -6,9 +6,11 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_SOCKET_FILTER, sk_filter)
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_SCHED_CLS, tc_cls_act)
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_SCHED_ACT, tc_cls_act)
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_XDP, xdp)
> > +#ifdef CONFIG_CGROUP_BPF
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SKB, cg_skb)
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCK, cg_sock)
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCK_ADDR, cg_sock_addr)
> > +#endif
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_IN, lwt_in)
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_OUT, lwt_out)
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_XMIT, lwt_xmit)
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index f9348806e843..6a390e519431 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -5315,6 +5315,7 @@ bpf_base_func_proto(enum bpf_func_id func_id)
> >         }
> >  }
> >
> > +#ifdef CONFIG_CGROUP_BPF
> >  static const struct bpf_func_proto *
> >  sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> >  {
> > @@ -5364,6 +5365,7 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> >                 return bpf_base_func_proto(func_id);
> >         }
> >  }
> > +#endif
> >
> >  static const struct bpf_func_proto *
> >  sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > @@ -5382,6 +5384,7 @@ sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> >         }
> >  }
> >
> > +#ifdef CONFIG_CGROUP_BPF
> >  static const struct bpf_func_proto *
> >  cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> 
> I don't think it's worth uglifying the code like this.
> I prefer to leave it as-is.
Sure, up to you. I mostly included it for completeness sake. I tested on
two configs: the first one is allyesbpf, the second one is minimal set
of bpf features and no cgroups.

(For my usecase cgroups and hence these prog types are always enabled,
so it doesn't matter for me).