netdev - Re: [PATCH 35/39] convert bpf_token

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: <CAEf4BzasSXFx5edPknxVnmk+o6oAyOU0h_Tg_yHVaJcaJfpPOQ@mail.gmail.com>
Date: Tue, 6 Aug 2024 15:42:56 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: viro@...nel.org
Cc: linux-fsdevel@...r.kernel.org, amir73il@...il.com, bpf@...r.kernel.org, 
	brauner@...nel.org, cgroups@...r.kernel.org, kvm@...r.kernel.org, 
	netdev@...r.kernel.org, torvalds@...ux-foundation.org
Subject: Re: [PATCH 35/39] convert bpf_token_create()

On Mon, Jul 29, 2024 at 10:27 PM <viro@...nel.org> wrote:
>
> From: Al Viro <viro@...iv.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@...iv.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@...nel.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@...nel.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@...iv.linux.org.uk>
    Signed-off-by: Andrii Nakryiko <andrii@...nel.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;


[...]