netdev - Re: [PATCH bpf-next v6 05/10] bpf: implement BPF_PROG_QUERY for BPF_LSM

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: <20220507001250.yhisk4otoas7h4gx@kafai-mbp.dhcp.thefacebook.com>
Date:   Fri, 6 May 2022 17:12:50 -0700
From:   Martin KaFai Lau <kafai@...com>
To:     Stanislav Fomichev <sdf@...gle.com>
Cc:     netdev@...r.kernel.org, bpf@...r.kernel.org, ast@...nel.org,
        daniel@...earbox.net, andrii@...nel.org
Subject: Re: [PATCH bpf-next v6 05/10] bpf: implement BPF_PROG_QUERY for
 BPF_LSM_CGROUP

On Fri, Apr 29, 2022 at 02:15:35PM -0700, Stanislav Fomichev wrote:
> 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 <sdf@...gle.com>
> ---
>  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.

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.