On Fri, May 6, 2022 at 5:12 PM Martin KaFai Lau [off-list ref] wrote:

On Fri, Apr 29, 2022 at 02:15:35PM -0700, Stanislav Fomichev wrote:

quoted

We have two options:
1. Treat all BPF_LSM_CGROUP as the same, regardless of attach_btf_id
2. Treat BPF_LSM_CGROUP+attach_btf_id as a separate hook point

I'm doing (2) here and adding attach_btf_id as a new BPF_PROG_QUERY
argument. The downside is that it requires iterating over all possible
bpf_lsm_ hook points in the userspace which might take some time.

Signed-off-by: Stanislav Fomichev <redacted>
---
 include/uapi/linux/bpf.h       |  1 +
 kernel/bpf/cgroup.c            | 43 ++++++++++++++++++++++++----------
 kernel/bpf/syscall.c           |  3 ++-
 tools/include/uapi/linux/bpf.h |  1 +
 tools/lib/bpf/bpf.c            | 42 ++++++++++++++++++++++++++-------
 tools/lib/bpf/bpf.h            | 15 ++++++++++++
 tools/lib/bpf/libbpf.map       |  1 +
 7 files changed, 85 insertions(+), 21 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 112e396bbe65..e38ea0b47b6a 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h

@@ -1431,6 +1431,7 @@ union bpf_attr {
              __u32           attach_flags;
              __aligned_u64   prog_ids;
              __u32           prog_cnt;
+             __u32           attach_btf_id;  /* for BPF_LSM_CGROUP */

If the downside/concern on (1) is the bpftool cannot show
which bpf_lsm_* hook that a cgroup-lsm is attached to,
how about adding this attach_btf_id to the bpf_prog_info instead.
The bpftool side is getting the bpf_prog_info (e.g. for the name) anyway.

Probably need to rename it to attach_func_btf_id (== prog->aux->attach_btf_id)
and then also add the attach_btf_id (== prog->aux->attach_btf->id) to
bpf_prog_info.

The bpftool then will work mostly the same and no need to iterate btf_vmlinux
to figure out the btf_id for all bpf_lsm_* hooks and no need to worry about
the increasing total number of lsm hooks in the future while
the latter bpftool patch has a static 1024.

If you also agree on (1), for this patch on the kernel side concern,
it needs to return all BPF_LSM_CGROUP progs to the userspace.

I was exploring this initially but with this scheme I'm not sure how
to export attach_flags. I'm assuming that one lsm hook can, say, be
attached as BPF_F_ALLOW_OVERRIDE and the other one can use
BPF_F_ALLOW_MULTI. Now, if we return all BPF_LSM_CGROUP programs (1),
we have a problem because there is only one attach_flags field per
attach_type.

I can extend BPF_PROG_QUERY with another user-provided pointer where
the kernel can put per-program attach_flags, doesn't seem like there
would be a problem, right?

Feel free to put the bpf_prog_info modification and bpftool changes as a follow up
patch.  In this same set is also fine.  Suggesting it because this set is
getting long already.

SG. Let's discuss it first. I can do a follow up series to add this
query api, the series is getting long indeed :-(