Re: Override default socket policy per cgroup

[sdf@google.com]

On 02/09, Alexei Starovoitov wrote:
> On Wed, Feb 9, 2022 at 1:51 PM <sdf@google.com> wrote:
> >
> > On 02/09, Martin KaFai Lau wrote:
> > > On Wed, Feb 09, 2022 at 09:03:45AM -0800, sdf@google.com wrote:
> > > > Let's say I want to set some default sk_priority for all sockets in  
> a
> > > > specific cgroup. I can do it right now using cgroup/sock_create,  
> but it
> > > > applies only to AF_INET{,6} sockets. I'd like to do the same for raw
> > > > (AF_PACKET) sockets and cgroup/sock_create doesn't trigger for  
> them :-(
> > > Other than AF_PACKET and INET[6], do you have use cases for other
> > > families?
> >
> > No, I only need AF_PACKET for now. But I feel like we should create
> > a more extensible hook point this time (if we go this route).
> >
> > > > (1) My naive approach would be to add another  
> cgroup/sock_post_create
> > > > which runs late from __sock_create and triggers on everything.
> > > >
> > > > (2) Another approach might be to move BPF_CGROUP_RUN_PROG_INET_SOCK  
> and
> > > > make it work with AF_PACKET. This might be not 100% backwards  
> compatible
> > > > but I'd assume that most users should look at the socket family  
> before
> > > > doing anything. (in this case it feels like we can extend
> > > > sock_bind/release for af_packets as well, just for accounting  
> purposes,
> > > > without any way to override the target ifindex).
> > > If adding a hook at __sock_create, I think having a new
> > > CGROUP_POST_SOCK_CREATE
> > > may be better instead of messing with the current inet assumption
> > > in CGROUP_'INET'_SOCK_CREATE.  Running all CGROUP_*_SOCK_CREATE at
> > > __sock_create could be a nice cleanup such that a few lines can be
> > > removed from inet[6]_create but an extra family check will be needed.
> >
> > SG. Hopefully I can at least reuse exiting progtype and just introduce
> > new hook point in __sock_create.

> Can you take a look at what it would take to add cgroup scope
> to bpf_lsm ?
> __sock_create() already has
> security_socket_create and security_socket_post_create
> in the right places.

> bpf_lsm cannot write directly into PTR_TO_BTF_ID like the 1st 'sock'  
> pointer.
> We can whitelist the write for certain cases.
> Maybe prototype it with bpf_lsm and use
> bpf_current_task_under_cgroup() helper to limit the scope
> before implementing cgroup-scoped bpf_lsm?

> There were cases in the past where bpf_lsm hook was in the ideal
> spot, but lack of cgroup scoping was a show stopper.
> This use case is another example and motivation to extend
> what bpf can do with lsm hooks. That's better than
> adding a new bpf_cgroup hook in the same location.

Cool, if you think we can whitelist some writes in lsm/fentry that
might be a perfect solution (especially if it gets per-cgroup scope).
I'll try to look in that, thanks!