Re: [PATCH bpf-next v6 03/10] bpf: per-cgroup lsm flavor

[Stanislav Fomichev <sdf@google.com>]

On Mon, May 9, 2022 at 2:51 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Apr 29, 2022 at 2:15 PM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > Allow attaching to lsm hooks in the cgroup context.
> >
> > Attaching to per-cgroup LSM works exactly like attaching
> > to other per-cgroup hooks. New BPF_LSM_CGROUP is added
> > to trigger new mode; the actual lsm hook we attach to is
> > signaled via existing attach_btf_id.
> >
> > For the hooks that have 'struct socket' or 'struct sock' as its first
> > argument, we use the cgroup associated with that socket. For the rest,
> > we use 'current' cgroup (this is all on default hierarchy == v2 only).
> > Note that for some hooks that work on 'struct sock' we still
> > take the cgroup from 'current' because some of them work on the socket
> > that hasn't been properly initialized yet.
> >
> > Behind the scenes, we allocate a shim program that is attached
> > to the trampoline and runs cgroup effective BPF programs array.
> > This shim has some rudimentary ref counting and can be shared
> > between several programs attaching to the same per-cgroup lsm hook.
> >
> > Note that this patch bloats cgroup size because we add 211
> > cgroup_bpf_attach_type(s) for simplicity sake. This will be
> > addressed in the subsequent patch.
> >
> > Also note that we only add non-sleepable flavor for now. To enable
> > sleepable use-cases, bpf_prog_run_array_cg has to grab trace rcu,
> > shim programs have to be freed via trace rcu, cgroup_bpf.effective
> > should be also trace-rcu-managed + maybe some other changes that
> > I'm not aware of.
> >
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> >  arch/x86/net/bpf_jit_comp.c     |  22 ++--
> >  include/linux/bpf-cgroup-defs.h |   6 ++
> >  include/linux/bpf-cgroup.h      |   7 ++
> >  include/linux/bpf.h             |  15 +++
> >  include/linux/bpf_lsm.h         |  14 +++
> >  include/uapi/linux/bpf.h        |   1 +
> >  kernel/bpf/bpf_lsm.c            |  64 ++++++++++++
> >  kernel/bpf/btf.c                |  11 ++
> >  kernel/bpf/cgroup.c             | 179 +++++++++++++++++++++++++++++---
> >  kernel/bpf/syscall.c            |  10 ++
> >  kernel/bpf/trampoline.c         | 161 ++++++++++++++++++++++++++++
> >  kernel/bpf/verifier.c           |  32 ++++++
> >  tools/include/uapi/linux/bpf.h  |   1 +
> >  13 files changed, 503 insertions(+), 20 deletions(-)
> >
>
> [...]
>
> > @@ -3474,6 +3476,11 @@ static int bpf_prog_attach(const union bpf_attr *attr)
> >         case BPF_PROG_TYPE_CGROUP_SOCKOPT:
> >         case BPF_PROG_TYPE_CGROUP_SYSCTL:
> >         case BPF_PROG_TYPE_SOCK_OPS:
> > +       case BPF_PROG_TYPE_LSM:
> > +               if (ptype == BPF_PROG_TYPE_LSM &&
> > +                   prog->expected_attach_type != BPF_LSM_CGROUP)
> > +                       return -EINVAL;
> > +
>
> Is it a hard requirement to support non-bpf_link attach for these BPF
> trampoline-backed programs? Can we keep it bpf_link-only and use
> LINK_CREATE for attachment? That way we won't need to extend query
> command and instead add new field to bpf_link_info?

I didn't think it was an option :-) So if non-link-based apis are
deprecated, I'll drop them from the patch series.