Re: [PATCH v7 11/18] bpf,lsm: add bpf_token_create and bpf_token_free LSM hooks

From: Paul Moore <paul@paul-moore.com>
To: Andrii Nakryiko <andrii@kernel.org>, <bpf@vger.kernel.org>,
	<netdev@vger.kernel.org>
Cc: <linux-fsdevel@vger.kernel.org>,
	<linux-security-module@vger.kernel.org>, <keescook@chromium.org>,
	<brauner@kernel.org>, <lennart@poettering.net>,
	<kernel-team@meta.com>, <sargun@sargun.me>
Subject: Re: [PATCH v7 11/18] bpf,lsm: add bpf_token_create and bpf_token_free  LSM hooks
Date: Fri, 13 Oct 2023 17:15:38 -0400	[thread overview]
Message-ID: <91ed4874a98b620dfa2bd6fe2966f8a7.paul@paul-moore.com> (raw)
In-Reply-To: <20231012222810.4120312-12-andrii@kernel.org>

On Oct 12, 2023 Andrii Nakryiko <andrii@kernel.org> wrote:
> 
> Wire up bpf_token_create and bpf_token_free LSM hooks, which allow to
> allocate LSM security blob (we add `void *security` field to struct
> bpf_token for that), but also control who can instantiate BPF token.
> This follows existing pattern for BPF map and BPF prog.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  include/linux/bpf.h           |  3 +++
>  include/linux/lsm_hook_defs.h |  3 +++
>  include/linux/security.h      | 11 +++++++++++
>  kernel/bpf/bpf_lsm.c          |  2 ++
>  kernel/bpf/token.c            |  6 ++++++
>  security/security.c           | 28 ++++++++++++++++++++++++++++
>  6 files changed, 53 insertions(+)

...

> diff --git a/kernel/bpf/token.c b/kernel/bpf/token.c
> index d4e0cc8075d3..18fd1e04f92d 100644
> --- a/kernel/bpf/token.c
> +++ b/kernel/bpf/token.c
> @@ -7,6 +7,7 @@
>  #include <linux/idr.h>
>  #include <linux/namei.h>
>  #include <linux/user_namespace.h>
> +#include <linux/security.h>
>  
>  bool bpf_token_capable(const struct bpf_token *token, int cap)
>  {
> @@ -28,6 +29,7 @@ void bpf_token_inc(struct bpf_token *token)
>  
>  static void bpf_token_free(struct bpf_token *token)
>  {
> +	security_bpf_token_free(token);
>  	put_user_ns(token->userns);
>  	kvfree(token);
>  }
> @@ -183,6 +185,10 @@ int bpf_token_create(union bpf_attr *attr)
>  	token->allowed_progs = mnt_opts->delegate_progs;
>  	token->allowed_attachs = mnt_opts->delegate_attachs;
>  
> +	err = security_bpf_token_create(token, attr, &path);
> +	if (err)
> +		goto out_token;
> +
>  	fd = get_unused_fd_flags(O_CLOEXEC);
>  	if (fd < 0) {
>  		err = fd;

As long as bpf_token_alloc() remains separate from bpf_token_create()
I'm not comfortable not having a security_bpf_token_alloc() hook in
bpf_token_alloc().  If you really don't want a LSM token alloc hook
can you fold bpf_token_alloc() into bpf_token_create()?

--
paul-moore.com