On 02/09, Alexei Starovoitov wrote:
On Wed, Feb 9, 2022 at 1:51 PM <sdf@xxxxxxxxxx> wrote:
>
> On 02/09, Martin KaFai Lau wrote:
> > On Wed, Feb 09, 2022 at 09:03:45AM -0800, sdf@xxxxxxxxxx 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!
