Re: [PATCH 35/39] convert bpf_token_create()

Andrii Nakryiko
2024-08-06T22:43:09Z

On Mon, Jul 29, 2024 at 10:27 PM <viro@kernel.org> wrote:
>
> From: Al Viro <viro@zeniv.linux.org.uk>
>
> keep file reference through the entire thing, don't bother with
> grabbing struct path reference (except, for now, around the LSM
> call and that only until it gets constified) and while we are
> at it, don't confuse the hell out of readers by random mix of
> path.dentry->d_sb and path.mnt->mnt_sb uses - these two are equal,
> so just put one of those into a local variable and use that.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> kernel/bpf/token.c | 69 +++++++++++++++++-----------------------------
> 1 file changed, 26 insertions(+), 43 deletions(-)
>

LGTM overall (modulo // comments, but see below)

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> diff --git a/kernel/bpf/token.c b/kernel/bpf/token.c
> index 9b92cb886d49..15da405d8302 100644
> --- a/kernel/bpf/token.c
> +++ b/kernel/bpf/token.c
> @@ -116,67 +116,52 @@ int bpf_token_create(union bpf_attr *attr)

[...]

> - err = security_bpf_token_create(token, attr, &path);
> + path_get(&path); // kill it
> + err = security_bpf_token_create(token, attr, &path); // constify
> + path_put(&path); // kill it
> if (err)
> goto out_token;
>

By constify you mean something like below?

commit 06a6442ca9cc441805881eea61fd57d7defadaca
Author: Andrii Nakryiko <andrii@kernel.org>
Date: Tue Aug 6 15:38:12 2024 -0700

security: constify struct path in bpf_token_create() LSM hook

There is no reason why struct path pointer shouldn't be const-qualified
when being passed into bpf_token_create() LSM hook. Add that const.

Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 855db460e08b..462b55378241 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -431,7 +431,7 @@ LSM_HOOK(int, 0, bpf_prog_load, struct bpf_prog
*prog, union bpf_attr *attr,
struct bpf_token *token)
LSM_HOOK(void, LSM_RET_VOID, bpf_prog_free, struct bpf_prog *prog)
LSM_HOOK(int, 0, bpf_token_create, struct bpf_token *token, union
bpf_attr *attr,
- struct path *path)
+ const struct path *path)
LSM_HOOK(void, LSM_RET_VOID, bpf_token_free, struct bpf_token *token)
LSM_HOOK(int, 0, bpf_token_cmd, const struct bpf_token *token, enum
bpf_cmd cmd)
LSM_HOOK(int, 0, bpf_token_capable, const struct bpf_token *token, int cap)
diff --git a/include/linux/security.h b/include/linux/security.h
index 1390f1efb4f0..31523a2c71c4 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -2137,7 +2137,7 @@ extern int security_bpf_prog_load(struct
bpf_prog *prog, union bpf_attr *attr,
struct bpf_token *token);
extern void security_bpf_prog_free(struct bpf_prog *prog);
extern int security_bpf_token_create(struct bpf_token *token, union
bpf_attr *attr,
- struct path *path);
+ const struct path *path);
extern void security_bpf_token_free(struct bpf_token *token);
extern int security_bpf_token_cmd(const struct bpf_token *token, enum
bpf_cmd cmd);
extern int security_bpf_token_capable(const struct bpf_token *token, int cap);
@@ -2177,7 +2177,7 @@ static inline void security_bpf_prog_free(struct
bpf_prog *prog)
{ }

static inline int security_bpf_token_create(struct bpf_token *token,
union bpf_attr *attr,
- struct path *path)
+ const struct path *path)
{
return 0;
}
diff --git a/security/security.c b/security/security.c
index 8cee5b6c6e6d..d8d0b67ced25 100644
--- a/security/security.c
+++ b/security/security.c
@@ -5510,7 +5510,7 @@ int security_bpf_prog_load(struct bpf_prog
*prog, union bpf_attr *attr,
* Return: Returns 0 on success, error on failure.
*/
int security_bpf_token_create(struct bpf_token *token, union bpf_attr *attr,
- struct path *path)
+ const struct path *path)
{
return call_int_hook(bpf_token_create, token, attr, path);
}
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 55c78c318ccd..0eec141a8f37 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6965,7 +6965,7 @@ static void selinux_bpf_prog_free(struct bpf_prog *prog)
}

static int selinux_bpf_token_create(struct bpf_token *token, union
bpf_attr *attr,
- struct path *path)
+ const struct path *path)
{
struct bpf_security_struct *bpfsec; …