[PATCH bpf-next 0/8] BPF follow ups to struct fd refactorings

[PATCH bpf-next 0/8] BPF follow ups to struct fd refactorings

[Andrii Nakryiko]

This patch set extracts all the BPF-related changes done in [0] into
a separate series based on top of stable-struct_fd branch ([1]) merged into
bpf-next tree. There are also a few changes, additions, and adjustments:

  - patch subjects adjusted to use "bpf: " prefix consistently;
  - patch #2 is extracting bpf-related changes from original patch #19
    ("fdget_raw() users: switch to CLASS(fd_raw, ...)") and is ordered a bit
    earlier in this patch set;
  - patch #3 is reimplemented and replaces original patch #17
    ("bpf: resolve_pseudo_ldimm64(): take handling of a single ldimm64 insn into helper")
    completely;
  - in patch #4 ("bpf: switch maps to CLASS(fd, ...)"), which was originally
    patch #18 ("bpf maps: switch to CLASS(fd, ...)"), I've combined
    __bpf_get_map() and bpf_file_to_map() into __bpf_get_map(), as the latter
    is only used from it and makes no sense to keep separate;
  - as part of rebasing patch #4, I adjusted newly added in patch #3
    add_used_map_from_fd() function to use CLASS(fd, ...), as now
    __bpf_get_map() doesn't do its own fdput() anymore. This made unnecessary
    any further bpf_map_inc() changes, because we still rely on struct fd to
    keep map's file reference alive;
  - patches #5 and #6 are BPF-specific bits extracted from original patch #23
    ("fdget(), trivial conversions") and #24 ("fdget(), more trivial conversions");
  - patch #7 constifies security_bpf_token_create() LSM hook;
  - patch #8 is original patch #35 ("convert bpf_token_create()"), with
    path_get()+path_put() removed now that LSM hook above was adjusted.

All these patches were pushed into a separate bpf-next/struct_fd branch ([2]).
They were also merged into bpf-next/for-next so they can get early testing in
linux-next.

  [0] https://lore.kernel.org/bpf/20240730050927.GC5334@ZenIV/
  [1] https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git/log/?h=stable-struct_fd
  [2] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/log/?h=struct_fd

Al Viro (6):
  bpf: convert __bpf_prog_get() to CLASS(fd, ...)
  bpf: switch fdget_raw() uses to CLASS(fd_raw, ...)
  bpf: switch maps to CLASS(fd, ...)
  bpf: trivial conversions for fdget()
  bpf: more trivial fdget() conversions
  bpf: convert bpf_token_create() to CLASS(fd, ...)

Andrii Nakryiko (2):
  bpf: factor out fetching bpf_map from FD and adding it to used_maps
    list
  security,bpf: constify struct path in bpf_token_create() LSM hook

 include/linux/bpf.h            |  11 +-
 include/linux/lsm_hook_defs.h  |   2 +-
 include/linux/security.h       |   4 +-
 kernel/bpf/bpf_inode_storage.c |  24 ++---
 kernel/bpf/btf.c               |  11 +-
 kernel/bpf/map_in_map.c        |  38 ++-----
 kernel/bpf/syscall.c           | 181 +++++++++------------------------
 kernel/bpf/token.c             |  74 +++++---------
 kernel/bpf/verifier.c          | 110 +++++++++++---------
 net/core/sock_map.c            |  23 ++---
 security/security.c            |   2 +-
 security/selinux/hooks.c       |   2 +-
 12 files changed, 179 insertions(+), 303 deletions(-)

-- 
2.43.5

[PATCH bpf-next 1/8] bpf: convert __bpf_prog_get() to CLASS(fd, ...)

[Andrii Nakryiko]

From: Al Viro <viro@zeniv.linux.org.uk>

Irregularity here is fdput() not in the same scope as fdget();
just fold ____bpf_prog_get() into its (only) caller and that's
it...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 kernel/bpf/syscall.c | 31 +++++++++----------------------
 1 file changed, 9 insertions(+), 22 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 3093bf2cc266..4909e3f23065 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2407,18 +2407,6 @@ int bpf_prog_new_fd(struct bpf_prog *prog)
 				O_RDWR | O_CLOEXEC);
 }
 
-static struct bpf_prog *____bpf_prog_get(struct fd f)
-{
-	if (!fd_file(f))
-		return ERR_PTR(-EBADF);
-	if (fd_file(f)->f_op != &bpf_prog_fops) {
-		fdput(f);
-		return ERR_PTR(-EINVAL);
-	}
-
-	return fd_file(f)->private_data;
-}
-
 void bpf_prog_add(struct bpf_prog *prog, int i)
 {
 	atomic64_add(i, &prog->aux->refcnt);
@@ -2474,20 +2462,19 @@ bool bpf_prog_get_ok(struct bpf_prog *prog,
 static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type,
 				       bool attach_drv)
 {
-	struct fd f = fdget(ufd);
+	CLASS(fd, f)(ufd);
 	struct bpf_prog *prog;
 
-	prog = ____bpf_prog_get(f);
-	if (IS_ERR(prog))
-		return prog;
-	if (!bpf_prog_get_ok(prog, attach_type, attach_drv)) {
-		prog = ERR_PTR(-EINVAL);
-		goto out;
-	}
+	if (fd_empty(f))
+		return ERR_PTR(-EBADF);
+	if (fd_file(f)->f_op != &bpf_prog_fops)
+		return ERR_PTR(-EINVAL);
+
+	prog = fd_file(f)->private_data;
+	if (!bpf_prog_get_ok(prog, attach_type, attach_drv))
+		return ERR_PTR(-EINVAL);
 
 	bpf_prog_inc(prog);
-out:
-	fdput(f);
 	return prog;
 }
 
-- 
2.43.5

[PATCH bpf-next 2/8] bpf: switch fdget_raw() uses to CLASS(fd_raw, ...)

[Andrii Nakryiko]

From: Al Viro <viro@zeniv.linux.org.uk>

Swith fdget_raw() use cases in bpf_inode_storage.c to CLASS(fd_raw).

Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 kernel/bpf/bpf_inode_storage.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/kernel/bpf/bpf_inode_storage.c b/kernel/bpf/bpf_inode_storage.c
index 0a79aee6523d..29da6d3838f6 100644
--- a/kernel/bpf/bpf_inode_storage.c
+++ b/kernel/bpf/bpf_inode_storage.c
@@ -78,13 +78,12 @@ void bpf_inode_storage_free(struct inode *inode)
 static void *bpf_fd_inode_storage_lookup_elem(struct bpf_map *map, void *key)
 {
 	struct bpf_local_storage_data *sdata;
-	struct fd f = fdget_raw(*(int *)key);
+	CLASS(fd_raw, f)(*(int *)key);
 
-	if (!fd_file(f))
+	if (fd_empty(f))
 		return ERR_PTR(-EBADF);
 
 	sdata = inode_storage_lookup(file_inode(fd_file(f)), map, true);
-	fdput(f);
 	return sdata ? sdata->data : NULL;
 }
 
@@ -92,19 +91,16 @@ static long bpf_fd_inode_storage_update_elem(struct bpf_map *map, void *key,
 					     void *value, u64 map_flags)
 {
 	struct bpf_local_storage_data *sdata;
-	struct fd f = fdget_raw(*(int *)key);
+	CLASS(fd_raw, f)(*(int *)key);
 
-	if (!fd_file(f))
+	if (fd_empty(f))
 		return -EBADF;
-	if (!inode_storage_ptr(file_inode(fd_file(f)))) {
-		fdput(f);
+	if (!inode_storage_ptr(file_inode(fd_file(f))))
 		return -EBADF;
-	}
 
 	sdata = bpf_local_storage_update(file_inode(fd_file(f)),
 					 (struct bpf_local_storage_map *)map,
 					 value, map_flags, GFP_ATOMIC);
-	fdput(f);
 	return PTR_ERR_OR_ZERO(sdata);
 }
 
@@ -123,15 +119,11 @@ static int inode_storage_delete(struct inode *inode, struct bpf_map *map)
 
 static long bpf_fd_inode_storage_delete_elem(struct bpf_map *map, void *key)
 {
-	struct fd f = fdget_raw(*(int *)key);
-	int err;
+	CLASS(fd_raw, f)(*(int *)key);
 
-	if (!fd_file(f))
+	if (fd_empty(f))
 		return -EBADF;
-
-	err = inode_storage_delete(file_inode(fd_file(f)), map);
-	fdput(f);
-	return err;
+	return inode_storage_delete(file_inode(fd_file(f)), map);
 }
 
 /* *gfp_flags* is a hidden argument provided by the verifier */
-- 
2.43.5

[PATCH bpf-next 3/8] bpf: factor out fetching bpf_map from FD and adding it to used_maps list

[Andrii Nakryiko]

Factor out the logic to extract bpf_map instances from FD embedded in
bpf_insns, adding it to the list of used_maps (unless it's already
there, in which case we just reuse map's index). This simplifies the
logic in resolve_pseudo_ldimm64(), especially around `struct fd`
handling, as all that is now neatly contained in the helper and doesn't
leak into a dozen error handling paths.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 kernel/bpf/verifier.c | 115 ++++++++++++++++++++++++------------------
 1 file changed, 66 insertions(+), 49 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index df3be12096cf..14e4ef687a59 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -18865,6 +18865,58 @@ static bool bpf_map_is_cgroup_storage(struct bpf_map *map)
 		map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE);
 }
 
+/* Add map behind fd to used maps list, if it's not already there, and return
+ * its index. Also set *reused to true if this map was already in the list of
+ * used maps.
+ * Returns <0 on error, or >= 0 index, on success.
+ */
+static int add_used_map_from_fd(struct bpf_verifier_env *env, int fd, bool *reused)
+{
+	struct fd f = fdget(fd);
+	struct bpf_map *map;
+	int i;
+
+	map = __bpf_map_get(f);
+	if (IS_ERR(map)) {
+		verbose(env, "fd %d is not pointing to valid bpf_map\n", fd);
+		return PTR_ERR(map);
+	}
+
+	/* check whether we recorded this map already */
+	for (i = 0; i < env->used_map_cnt; i++) {
+		if (env->used_maps[i] == map) {
+			*reused = true;
+			fdput(f);
+			return i;
+		}
+	}
+
+	if (env->used_map_cnt >= MAX_USED_MAPS) {
+		verbose(env, "The total number of maps per program has reached the limit of %u\n",
+			MAX_USED_MAPS);
+		fdput(f);
+		return -E2BIG;
+	}
+
+	if (env->prog->sleepable)
+		atomic64_inc(&map->sleepable_refcnt);
+
+	/* hold the map. If the program is rejected by verifier,
+	 * the map will be released by release_maps() or it
+	 * will be used by the valid program until it's unloaded
+	 * and all maps are released in bpf_free_used_maps()
+	 */
+	bpf_map_inc(map);
+
+	*reused = false;
+	env->used_maps[env->used_map_cnt++] = map;
+
+	fdput(f);
+
+	return env->used_map_cnt - 1;
+
+}
+
 /* find and rewrite pseudo imm in ld_imm64 instructions:
  *
  * 1. if it accesses map FD, replace it with actual map pointer.
@@ -18876,7 +18928,7 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
 {
 	struct bpf_insn *insn = env->prog->insnsi;
 	int insn_cnt = env->prog->len;
-	int i, j, err;
+	int i, err;
 
 	err = bpf_prog_calc_tag(env->prog);
 	if (err)
@@ -18893,9 +18945,10 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
 		if (insn[0].code == (BPF_LD | BPF_IMM | BPF_DW)) {
 			struct bpf_insn_aux_data *aux;
 			struct bpf_map *map;
-			struct fd f;
+			int map_idx;
 			u64 addr;
 			u32 fd;
+			bool reused;
 
 			if (i == insn_cnt - 1 || insn[1].code != 0 ||
 			    insn[1].dst_reg != 0 || insn[1].src_reg != 0 ||
@@ -18956,20 +19009,18 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
 				break;
 			}
 
-			f = fdget(fd);
-			map = __bpf_map_get(f);
-			if (IS_ERR(map)) {
-				verbose(env, "fd %d is not pointing to valid bpf_map\n", fd);
-				return PTR_ERR(map);
-			}
+			map_idx = add_used_map_from_fd(env, fd, &reused);
+			if (map_idx < 0)
+				return map_idx;
+			map = env->used_maps[map_idx];
+
+			aux = &env->insn_aux_data[i];
+			aux->map_index = map_idx;
 
 			err = check_map_prog_compatibility(env, map, env->prog);
-			if (err) {
-				fdput(f);
+			if (err)
 				return err;
-			}
 
-			aux = &env->insn_aux_data[i];
 			if (insn[0].src_reg == BPF_PSEUDO_MAP_FD ||
 			    insn[0].src_reg == BPF_PSEUDO_MAP_IDX) {
 				addr = (unsigned long)map;
@@ -18978,13 +19029,11 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
 
 				if (off >= BPF_MAX_VAR_OFF) {
 					verbose(env, "direct value offset of %u is not allowed\n", off);
-					fdput(f);
 					return -EINVAL;
 				}
 
 				if (!map->ops->map_direct_value_addr) {
 					verbose(env, "no direct value access support for this map type\n");
-					fdput(f);
 					return -EINVAL;
 				}
 
@@ -18992,7 +19041,6 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
 				if (err) {
 					verbose(env, "invalid access to map value pointer, value_size=%u off=%u\n",
 						map->value_size, off);
-					fdput(f);
 					return err;
 				}
 
@@ -19003,70 +19051,39 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
 			insn[0].imm = (u32)addr;
 			insn[1].imm = addr >> 32;
 
-			/* check whether we recorded this map already */
-			for (j = 0; j < env->used_map_cnt; j++) {
-				if (env->used_maps[j] == map) {
-					aux->map_index = j;
-					fdput(f);
-					goto next_insn;
-				}
-			}
-
-			if (env->used_map_cnt >= MAX_USED_MAPS) {
-				verbose(env, "The total number of maps per program has reached the limit of %u\n",
-					MAX_USED_MAPS);
-				fdput(f);
-				return -E2BIG;
-			}
-
-			if (env->prog->sleepable)
-				atomic64_inc(&map->sleepable_refcnt);
-			/* hold the map. If the program is rejected by verifier,
-			 * the map will be released by release_maps() or it
-			 * will be used by the valid program until it's unloaded
-			 * and all maps are released in bpf_free_used_maps()
-			 */
-			bpf_map_inc(map);
-
-			aux->map_index = env->used_map_cnt;
-			env->used_maps[env->used_map_cnt++] = map;
+			/* proceed with extra checks only if its newly added used map */
+			if (reused)
+				goto next_insn;
 
 			if (bpf_map_is_cgroup_storage(map) &&
 			    bpf_cgroup_storage_assign(env->prog->aux, map)) {
 				verbose(env, "only one cgroup storage of each type is allowed\n");
-				fdput(f);
 				return -EBUSY;
 			}
 			if (map->map_type == BPF_MAP_TYPE_ARENA) {
 				if (env->prog->aux->arena) {
 					verbose(env, "Only one arena per program\n");
-					fdput(f);
 					return -EBUSY;
 				}
 				if (!env->allow_ptr_leaks || !env->bpf_capable) {
 					verbose(env, "CAP_BPF and CAP_PERFMON are required to use arena\n");
-					fdput(f);
 					return -EPERM;
 				}
 				if (!env->prog->jit_requested) {
 					verbose(env, "JIT is required to use arena\n");
-					fdput(f);
 					return -EOPNOTSUPP;
 				}
 				if (!bpf_jit_supports_arena()) {
 					verbose(env, "JIT doesn't support arena\n");
-					fdput(f);
 					return -EOPNOTSUPP;
 				}
 				env->prog->aux->arena = (void *)map;
 				if (!bpf_arena_get_user_vm_start(env->prog->aux->arena)) {
 					verbose(env, "arena's user address must be set via map_extra or mmap()\n");
-					fdput(f);
 					return -EINVAL;
 				}
 			}
 
-			fdput(f);
 next_insn:
 			insn++;
 			i++;
-- 
2.43.5

[PATCH bpf-next 4/8] bpf: switch maps to CLASS(fd, ...)

[Andrii Nakryiko]

From: Al Viro <viro@zeniv.linux.org.uk>

        Calling conventions for __bpf_map_get() would be more convenient
if it left fpdut() on failure to callers.  Makes for simpler logics
in the callers.

	Among other things, the proof of memory safety no longer has to
rely upon file->private_data never being ERR_PTR(...) for bpffs files.
Original calling conventions made it impossible for the caller to tell
whether __bpf_map_get() has returned ERR_PTR(-EINVAL) because it has found
the file not be a bpf map one (in which case it would've done fdput())
or because it found that ERR_PTR(-EINVAL) in file->private_data of a
bpf map file (in which case fdput() would _not_ have been done).

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 include/linux/bpf.h     |  11 +++-
 kernel/bpf/map_in_map.c |  38 ++++---------
 kernel/bpf/syscall.c    | 118 ++++++++++------------------------------
 kernel/bpf/verifier.c   |   7 +--
 net/core/sock_map.c     |  23 ++------
 5 files changed, 58 insertions(+), 139 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index b9425e410bcb..9f35df07e86d 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2241,7 +2241,16 @@ void __bpf_obj_drop_impl(void *p, const struct btf_record *rec, bool percpu);
 
 struct bpf_map *bpf_map_get(u32 ufd);
 struct bpf_map *bpf_map_get_with_uref(u32 ufd);
-struct bpf_map *__bpf_map_get(struct fd f);
+
+static inline struct bpf_map *__bpf_map_get(struct fd f)
+{
+	if (fd_empty(f))
+		return ERR_PTR(-EBADF);
+	if (unlikely(fd_file(f)->f_op != &bpf_map_fops))
+		return ERR_PTR(-EINVAL);
+	return fd_file(f)->private_data;
+}
+
 void bpf_map_inc(struct bpf_map *map);
 void bpf_map_inc_with_uref(struct bpf_map *map);
 struct bpf_map *__bpf_map_inc_not_zero(struct bpf_map *map, bool uref);
diff --git a/kernel/bpf/map_in_map.c b/kernel/bpf/map_in_map.c
index b4f18c85d7bc..645bd30bc9a9 100644
--- a/kernel/bpf/map_in_map.c
+++ b/kernel/bpf/map_in_map.c
@@ -11,24 +11,18 @@ struct bpf_map *bpf_map_meta_alloc(int inner_map_ufd)
 {
 	struct bpf_map *inner_map, *inner_map_meta;
 	u32 inner_map_meta_size;
-	struct fd f;
-	int ret;
+	CLASS(fd, f)(inner_map_ufd);
 
-	f = fdget(inner_map_ufd);
 	inner_map = __bpf_map_get(f);
 	if (IS_ERR(inner_map))
 		return inner_map;
 
 	/* Does not support >1 level map-in-map */
-	if (inner_map->inner_map_meta) {
-		ret = -EINVAL;
-		goto put;
-	}
+	if (inner_map->inner_map_meta)
+		return ERR_PTR(-EINVAL);
 
-	if (!inner_map->ops->map_meta_equal) {
-		ret = -ENOTSUPP;
-		goto put;
-	}
+	if (!inner_map->ops->map_meta_equal)
+		return ERR_PTR(-ENOTSUPP);
 
 	inner_map_meta_size = sizeof(*inner_map_meta);
 	/* In some cases verifier needs to access beyond just base map. */
@@ -36,10 +30,8 @@ struct bpf_map *bpf_map_meta_alloc(int inner_map_ufd)
 		inner_map_meta_size = sizeof(struct bpf_array);
 
 	inner_map_meta = kzalloc(inner_map_meta_size, GFP_USER);
-	if (!inner_map_meta) {
-		ret = -ENOMEM;
-		goto put;
-	}
+	if (!inner_map_meta)
+		return ERR_PTR(-ENOMEM);
 
 	inner_map_meta->map_type = inner_map->map_type;
 	inner_map_meta->key_size = inner_map->key_size;
@@ -53,8 +45,9 @@ struct bpf_map *bpf_map_meta_alloc(int inner_map_ufd)
 		 * invalid/empty/valid, but ERR_PTR in case of errors. During
 		 * equality NULL or IS_ERR is equivalent.
 		 */
-		ret = PTR_ERR(inner_map_meta->record);
-		goto free;
+		struct bpf_map *ret = ERR_CAST(inner_map_meta->record);
+		kfree(inner_map_meta);
+		return ret;
 	}
 	/* Note: We must use the same BTF, as we also used btf_record_dup above
 	 * which relies on BTF being same for both maps, as some members like
@@ -77,14 +70,7 @@ struct bpf_map *bpf_map_meta_alloc(int inner_map_ufd)
 		inner_array_meta->elem_size = inner_array->elem_size;
 		inner_map_meta->bypass_spec_v1 = inner_map->bypass_spec_v1;
 	}
-
-	fdput(f);
 	return inner_map_meta;
-free:
-	kfree(inner_map_meta);
-put:
-	fdput(f);
-	return ERR_PTR(ret);
 }
 
 void bpf_map_meta_free(struct bpf_map *map_meta)
@@ -110,9 +96,8 @@ void *bpf_map_fd_get_ptr(struct bpf_map *map,
 			 int ufd)
 {
 	struct bpf_map *inner_map, *inner_map_meta;
-	struct fd f;
+	CLASS(fd, f)(ufd);
 
-	f = fdget(ufd);
 	inner_map = __bpf_map_get(f);
 	if (IS_ERR(inner_map))
 		return inner_map;
@@ -123,7 +108,6 @@ void *bpf_map_fd_get_ptr(struct bpf_map *map,
 	else
 		inner_map = ERR_PTR(-EINVAL);
 
-	fdput(f);
 	return inner_map;
 }
 
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 4909e3f23065..ab0d94f41c48 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1418,21 +1418,6 @@ static int map_create(union bpf_attr *attr)
 	return err;
 }
 
-/* if error is returned, fd is released.
- * On success caller should complete fd access with matching fdput()
- */
-struct bpf_map *__bpf_map_get(struct fd f)
-{
-	if (!fd_file(f))
-		return ERR_PTR(-EBADF);
-	if (fd_file(f)->f_op != &bpf_map_fops) {
-		fdput(f);
-		return ERR_PTR(-EINVAL);
-	}
-
-	return fd_file(f)->private_data;
-}
-
 void bpf_map_inc(struct bpf_map *map)
 {
 	atomic64_inc(&map->refcnt);
@@ -1448,15 +1433,11 @@ EXPORT_SYMBOL_GPL(bpf_map_inc_with_uref);
 
 struct bpf_map *bpf_map_get(u32 ufd)
 {
-	struct fd f = fdget(ufd);
-	struct bpf_map *map;
-
-	map = __bpf_map_get(f);
-	if (IS_ERR(map))
-		return map;
+	CLASS(fd, f)(ufd);
+	struct bpf_map *map = __bpf_map_get(f);
 
-	bpf_map_inc(map);
-	fdput(f);
+	if (!IS_ERR(map))
+		bpf_map_inc(map);
 
 	return map;
 }
@@ -1464,15 +1445,11 @@ EXPORT_SYMBOL(bpf_map_get);
 
 struct bpf_map *bpf_map_get_with_uref(u32 ufd)
 {
-	struct fd f = fdget(ufd);
-	struct bpf_map *map;
-
-	map = __bpf_map_get(f);
-	if (IS_ERR(map))
-		return map;
+	CLASS(fd, f)(ufd);
+	struct bpf_map *map = __bpf_map_get(f);
 
-	bpf_map_inc_with_uref(map);
-	fdput(f);
+	if (!IS_ERR(map))
+		bpf_map_inc_with_uref(map);
 
 	return map;
 }
@@ -1537,11 +1514,9 @@ static int map_lookup_elem(union bpf_attr *attr)
 {
 	void __user *ukey = u64_to_user_ptr(attr->key);
 	void __user *uvalue = u64_to_user_ptr(attr->value);
-	int ufd = attr->map_fd;
 	struct bpf_map *map;
 	void *key, *value;
 	u32 value_size;
-	struct fd f;
 	int err;
 
 	if (CHECK_ATTR(BPF_MAP_LOOKUP_ELEM))
@@ -1550,26 +1525,20 @@ static int map_lookup_elem(union bpf_attr *attr)
 	if (attr->flags & ~BPF_F_LOCK)
 		return -EINVAL;
 
-	f = fdget(ufd);
+	CLASS(fd, f)(attr->map_fd);
 	map = __bpf_map_get(f);
 	if (IS_ERR(map))
 		return PTR_ERR(map);
-	if (!(map_get_sys_perms(map, f) & FMODE_CAN_READ)) {
-		err = -EPERM;
-		goto err_put;
-	}
+	if (!(map_get_sys_perms(map, f) & FMODE_CAN_READ))
+		return -EPERM;
 
 	if ((attr->flags & BPF_F_LOCK) &&
-	    !btf_record_has_field(map->record, BPF_SPIN_LOCK)) {
-		err = -EINVAL;
-		goto err_put;
-	}
+	    !btf_record_has_field(map->record, BPF_SPIN_LOCK))
+		return -EINVAL;
 
 	key = __bpf_copy_key(ukey, map->key_size);
-	if (IS_ERR(key)) {
-		err = PTR_ERR(key);
-		goto err_put;
-	}
+	if (IS_ERR(key))
+		return PTR_ERR(key);
 
 	value_size = bpf_map_value_size(map);
 
@@ -1600,8 +1569,6 @@ static int map_lookup_elem(union bpf_attr *attr)
 	kvfree(value);
 free_key:
 	kvfree(key);
-err_put:
-	fdput(f);
 	return err;
 }
 
@@ -1612,17 +1579,15 @@ static int map_update_elem(union bpf_attr *attr, bpfptr_t uattr)
 {
 	bpfptr_t ukey = make_bpfptr(attr->key, uattr.is_kernel);
 	bpfptr_t uvalue = make_bpfptr(attr->value, uattr.is_kernel);
-	int ufd = attr->map_fd;
 	struct bpf_map *map;
 	void *key, *value;
 	u32 value_size;
-	struct fd f;
 	int err;
 
 	if (CHECK_ATTR(BPF_MAP_UPDATE_ELEM))
 		return -EINVAL;
 
-	f = fdget(ufd);
+	CLASS(fd, f)(attr->map_fd);
 	map = __bpf_map_get(f);
 	if (IS_ERR(map))
 		return PTR_ERR(map);
@@ -1660,7 +1625,6 @@ static int map_update_elem(union bpf_attr *attr, bpfptr_t uattr)
 	kvfree(key);
 err_put:
 	bpf_map_write_active_dec(map);
-	fdput(f);
 	return err;
 }
 
@@ -1669,16 +1633,14 @@ static int map_update_elem(union bpf_attr *attr, bpfptr_t uattr)
 static int map_delete_elem(union bpf_attr *attr, bpfptr_t uattr)
 {
 	bpfptr_t ukey = make_bpfptr(attr->key, uattr.is_kernel);
-	int ufd = attr->map_fd;
 	struct bpf_map *map;
-	struct fd f;
 	void *key;
 	int err;
 
 	if (CHECK_ATTR(BPF_MAP_DELETE_ELEM))
 		return -EINVAL;
 
-	f = fdget(ufd);
+	CLASS(fd, f)(attr->map_fd);
 	map = __bpf_map_get(f);
 	if (IS_ERR(map))
 		return PTR_ERR(map);
@@ -1715,7 +1677,6 @@ static int map_delete_elem(union bpf_attr *attr, bpfptr_t uattr)
 	kvfree(key);
 err_put:
 	bpf_map_write_active_dec(map);
-	fdput(f);
 	return err;
 }
 
@@ -1726,30 +1687,24 @@ static int map_get_next_key(union bpf_attr *attr)
 {
 	void __user *ukey = u64_to_user_ptr(attr->key);
 	void __user *unext_key = u64_to_user_ptr(attr->next_key);
-	int ufd = attr->map_fd;
 	struct bpf_map *map;
 	void *key, *next_key;
-	struct fd f;
 	int err;
 
 	if (CHECK_ATTR(BPF_MAP_GET_NEXT_KEY))
 		return -EINVAL;
 
-	f = fdget(ufd);
+	CLASS(fd, f)(attr->map_fd);
 	map = __bpf_map_get(f);
 	if (IS_ERR(map))
 		return PTR_ERR(map);
-	if (!(map_get_sys_perms(map, f) & FMODE_CAN_READ)) {
-		err = -EPERM;
-		goto err_put;
-	}
+	if (!(map_get_sys_perms(map, f) & FMODE_CAN_READ))
+		return -EPERM;
 
 	if (ukey) {
 		key = __bpf_copy_key(ukey, map->key_size);
-		if (IS_ERR(key)) {
-			err = PTR_ERR(key);
-			goto err_put;
-		}
+		if (IS_ERR(key))
+			return PTR_ERR(key);
 	} else {
 		key = NULL;
 	}
@@ -1781,8 +1736,6 @@ static int map_get_next_key(union bpf_attr *attr)
 	kvfree(next_key);
 free_key:
 	kvfree(key);
-err_put:
-	fdput(f);
 	return err;
 }
 
@@ -2011,11 +1964,9 @@ static int map_lookup_and_delete_elem(union bpf_attr *attr)
 {
 	void __user *ukey = u64_to_user_ptr(attr->key);
 	void __user *uvalue = u64_to_user_ptr(attr->value);
-	int ufd = attr->map_fd;
 	struct bpf_map *map;
 	void *key, *value;
 	u32 value_size;
-	struct fd f;
 	int err;
 
 	if (CHECK_ATTR(BPF_MAP_LOOKUP_AND_DELETE_ELEM))
@@ -2024,7 +1975,7 @@ static int map_lookup_and_delete_elem(union bpf_attr *attr)
 	if (attr->flags & ~BPF_F_LOCK)
 		return -EINVAL;
 
-	f = fdget(ufd);
+	CLASS(fd, f)(attr->map_fd);
 	map = __bpf_map_get(f);
 	if (IS_ERR(map))
 		return PTR_ERR(map);
@@ -2094,7 +2045,6 @@ static int map_lookup_and_delete_elem(union bpf_attr *attr)
 	kvfree(key);
 err_put:
 	bpf_map_write_active_dec(map);
-	fdput(f);
 	return err;
 }
 
@@ -2102,27 +2052,22 @@ static int map_lookup_and_delete_elem(union bpf_attr *attr)
 
 static int map_freeze(const union bpf_attr *attr)
 {
-	int err = 0, ufd = attr->map_fd;
+	int err = 0;
 	struct bpf_map *map;
-	struct fd f;
 
 	if (CHECK_ATTR(BPF_MAP_FREEZE))
 		return -EINVAL;
 
-	f = fdget(ufd);
+	CLASS(fd, f)(attr->map_fd);
 	map = __bpf_map_get(f);
 	if (IS_ERR(map))
 		return PTR_ERR(map);
 
-	if (map->map_type == BPF_MAP_TYPE_STRUCT_OPS || !IS_ERR_OR_NULL(map->record)) {
-		fdput(f);
+	if (map->map_type == BPF_MAP_TYPE_STRUCT_OPS || !IS_ERR_OR_NULL(map->record))
 		return -ENOTSUPP;
-	}
 
-	if (!(map_get_sys_perms(map, f) & FMODE_CAN_WRITE)) {
-		fdput(f);
+	if (!(map_get_sys_perms(map, f) & FMODE_CAN_WRITE))
 		return -EPERM;
-	}
 
 	mutex_lock(&map->freeze_mutex);
 	if (bpf_map_write_active(map)) {
@@ -2137,7 +2082,6 @@ static int map_freeze(const union bpf_attr *attr)
 	WRITE_ONCE(map->frozen, true);
 err_put:
 	mutex_unlock(&map->freeze_mutex);
-	fdput(f);
 	return err;
 }
 
@@ -5175,14 +5119,13 @@ static int bpf_map_do_batch(const union bpf_attr *attr,
 			 cmd == BPF_MAP_LOOKUP_AND_DELETE_BATCH;
 	bool has_write = cmd != BPF_MAP_LOOKUP_BATCH;
 	struct bpf_map *map;
-	int err, ufd;
-	struct fd f;
+	int err;
 
 	if (CHECK_ATTR(BPF_MAP_BATCH))
 		return -EINVAL;
 
-	ufd = attr->batch.map_fd;
-	f = fdget(ufd);
+	CLASS(fd, f)(attr->batch.map_fd);
+
 	map = __bpf_map_get(f);
 	if (IS_ERR(map))
 		return PTR_ERR(map);
@@ -5210,7 +5153,6 @@ static int bpf_map_do_batch(const union bpf_attr *attr,
 		maybe_wait_bpf_programs(map);
 		bpf_map_write_active_dec(map);
 	}
-	fdput(f);
 	return err;
 }
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 14e4ef687a59..e3932f8ce10a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -18872,7 +18872,7 @@ static bool bpf_map_is_cgroup_storage(struct bpf_map *map)
  */
 static int add_used_map_from_fd(struct bpf_verifier_env *env, int fd, bool *reused)
 {
-	struct fd f = fdget(fd);
+	CLASS(fd, f)(fd);
 	struct bpf_map *map;
 	int i;
 
@@ -18886,7 +18886,6 @@ static int add_used_map_from_fd(struct bpf_verifier_env *env, int fd, bool *reus
 	for (i = 0; i < env->used_map_cnt; i++) {
 		if (env->used_maps[i] == map) {
 			*reused = true;
-			fdput(f);
 			return i;
 		}
 	}
@@ -18894,7 +18893,6 @@ static int add_used_map_from_fd(struct bpf_verifier_env *env, int fd, bool *reus
 	if (env->used_map_cnt >= MAX_USED_MAPS) {
 		verbose(env, "The total number of maps per program has reached the limit of %u\n",
 			MAX_USED_MAPS);
-		fdput(f);
 		return -E2BIG;
 	}
 
@@ -18911,10 +18909,7 @@ static int add_used_map_from_fd(struct bpf_verifier_env *env, int fd, bool *reus
 	*reused = false;
 	env->used_maps[env->used_map_cnt++] = map;
 
-	fdput(f);
-
 	return env->used_map_cnt - 1;
-
 }
 
 /* find and rewrite pseudo imm in ld_imm64 instructions:
diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index d3dbb92153f2..0f5f80f44d52 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -67,46 +67,39 @@ static struct bpf_map *sock_map_alloc(union bpf_attr *attr)
 
 int sock_map_get_from_fd(const union bpf_attr *attr, struct bpf_prog *prog)
 {
-	u32 ufd = attr->target_fd;
 	struct bpf_map *map;
-	struct fd f;
 	int ret;
 
 	if (attr->attach_flags || attr->replace_bpf_fd)
 		return -EINVAL;
 
-	f = fdget(ufd);
+	CLASS(fd, f)(attr->target_fd);
 	map = __bpf_map_get(f);
 	if (IS_ERR(map))
 		return PTR_ERR(map);
 	mutex_lock(&sockmap_mutex);
 	ret = sock_map_prog_update(map, prog, NULL, NULL, attr->attach_type);
 	mutex_unlock(&sockmap_mutex);
-	fdput(f);
 	return ret;
 }
 
 int sock_map_prog_detach(const union bpf_attr *attr, enum bpf_prog_type ptype)
 {
-	u32 ufd = attr->target_fd;
 	struct bpf_prog *prog;
 	struct bpf_map *map;
-	struct fd f;
 	int ret;
 
 	if (attr->attach_flags || attr->replace_bpf_fd)
 		return -EINVAL;
 
-	f = fdget(ufd);
+	CLASS(fd, f)(attr->target_fd);
 	map = __bpf_map_get(f);
 	if (IS_ERR(map))
 		return PTR_ERR(map);
 
 	prog = bpf_prog_get(attr->attach_bpf_fd);
-	if (IS_ERR(prog)) {
-		ret = PTR_ERR(prog);
-		goto put_map;
-	}
+	if (IS_ERR(prog))
+		return PTR_ERR(prog);
 
 	if (prog->type != ptype) {
 		ret = -EINVAL;
@@ -118,8 +111,6 @@ int sock_map_prog_detach(const union bpf_attr *attr, enum bpf_prog_type ptype)
 	mutex_unlock(&sockmap_mutex);
 put_prog:
 	bpf_prog_put(prog);
-put_map:
-	fdput(f);
 	return ret;
 }
 
@@ -1550,18 +1541,17 @@ int sock_map_bpf_prog_query(const union bpf_attr *attr,
 			    union bpf_attr __user *uattr)
 {
 	__u32 __user *prog_ids = u64_to_user_ptr(attr->query.prog_ids);
-	u32 prog_cnt = 0, flags = 0, ufd = attr->target_fd;
+	u32 prog_cnt = 0, flags = 0;
 	struct bpf_prog **pprog;
 	struct bpf_prog *prog;
 	struct bpf_map *map;
-	struct fd f;
 	u32 id = 0;
 	int ret;
 
 	if (attr->query.query_flags)
 		return -EINVAL;
 
-	f = fdget(ufd);
+	CLASS(fd, f)(attr->target_fd);
 	map = __bpf_map_get(f);
 	if (IS_ERR(map))
 		return PTR_ERR(map);
@@ -1593,7 +1583,6 @@ int sock_map_bpf_prog_query(const union bpf_attr *attr,
 	    copy_to_user(&uattr->query.prog_cnt, &prog_cnt, sizeof(prog_cnt)))
 		ret = -EFAULT;
 
-	fdput(f);
 	return ret;
 }
 
-- 
2.43.5

[PATCH bpf-next 5/8] bpf: trivial conversions for fdget()

[Andrii Nakryiko]

From: Al Viro <viro@zeniv.linux.org.uk>

fdget() is the first thing done in scope, all matching fdput() are
immediately followed by leaving the scope.

Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 kernel/bpf/btf.c     | 11 +++--------
 kernel/bpf/syscall.c | 10 +++-------
 kernel/bpf/token.c   |  9 +++------
 3 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index e34f58abc7e2..c4506d788c85 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7676,21 +7676,16 @@ int btf_new_fd(const union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
 struct btf *btf_get_by_fd(int fd)
 {
 	struct btf *btf;
-	struct fd f;
+	CLASS(fd, f)(fd);
 
-	f = fdget(fd);
-
-	if (!fd_file(f))
+	if (fd_empty(f))
 		return ERR_PTR(-EBADF);
 
-	if (fd_file(f)->f_op != &btf_fops) {
-		fdput(f);
+	if (fd_file(f)->f_op != &btf_fops)
 		return ERR_PTR(-EINVAL);
-	}
 
 	btf = fd_file(f)->private_data;
 	refcount_inc(&btf->refcnt);
-	fdput(f);
 
 	return btf;
 }
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ab0d94f41c48..d75e4e68801e 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3187,20 +3187,16 @@ int bpf_link_new_fd(struct bpf_link *link)
 
 struct bpf_link *bpf_link_get_from_fd(u32 ufd)
 {
-	struct fd f = fdget(ufd);
+	CLASS(fd, f)(ufd);
 	struct bpf_link *link;
 
-	if (!fd_file(f))
+	if (fd_empty(f))
 		return ERR_PTR(-EBADF);
-	if (fd_file(f)->f_op != &bpf_link_fops && fd_file(f)->f_op != &bpf_link_fops_poll) {
-		fdput(f);
+	if (fd_file(f)->f_op != &bpf_link_fops && fd_file(f)->f_op != &bpf_link_fops_poll)
 		return ERR_PTR(-EINVAL);
-	}
 
 	link = fd_file(f)->private_data;
 	bpf_link_inc(link);
-	fdput(f);
-
 	return link;
 }
 EXPORT_SYMBOL(bpf_link_get_from_fd);
diff --git a/kernel/bpf/token.c b/kernel/bpf/token.c
index 9a1d356e79ed..9b92cb886d49 100644
--- a/kernel/bpf/token.c
+++ b/kernel/bpf/token.c
@@ -232,19 +232,16 @@ int bpf_token_create(union bpf_attr *attr)
 
 struct bpf_token *bpf_token_get_from_fd(u32 ufd)
 {
-	struct fd f = fdget(ufd);
+	CLASS(fd, f)(ufd);
 	struct bpf_token *token;
 
-	if (!fd_file(f))
+	if (fd_empty(f))
 		return ERR_PTR(-EBADF);
-	if (fd_file(f)->f_op != &bpf_token_fops) {
-		fdput(f);
+	if (fd_file(f)->f_op != &bpf_token_fops)
 		return ERR_PTR(-EINVAL);
-	}
 
 	token = fd_file(f)->private_data;
 	bpf_token_inc(token);
-	fdput(f);
 
 	return token;
 }
-- 
2.43.5

[PATCH bpf-next 6/8] bpf: more trivial fdget() conversions

[Andrii Nakryiko]

From: Al Viro <viro@zeniv.linux.org.uk>

All failure exits prior to fdget() leave the scope, all matching fdput()
are immediately followed by leaving the scope.

Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 kernel/bpf/syscall.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index d75e4e68801e..65dcd92d0b2c 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -4901,33 +4901,25 @@ static int bpf_link_get_info_by_fd(struct file *file,
 static int bpf_obj_get_info_by_fd(const union bpf_attr *attr,
 				  union bpf_attr __user *uattr)
 {
-	int ufd = attr->info.bpf_fd;
-	struct fd f;
-	int err;
-
 	if (CHECK_ATTR(BPF_OBJ_GET_INFO_BY_FD))
 		return -EINVAL;
 
-	f = fdget(ufd);
-	if (!fd_file(f))
+	CLASS(fd, f)(attr->info.bpf_fd);
+	if (fd_empty(f))
 		return -EBADFD;
 
 	if (fd_file(f)->f_op == &bpf_prog_fops)
-		err = bpf_prog_get_info_by_fd(fd_file(f), fd_file(f)->private_data, attr,
+		return bpf_prog_get_info_by_fd(fd_file(f), fd_file(f)->private_data, attr,
 					      uattr);
 	else if (fd_file(f)->f_op == &bpf_map_fops)
-		err = bpf_map_get_info_by_fd(fd_file(f), fd_file(f)->private_data, attr,
+		return bpf_map_get_info_by_fd(fd_file(f), fd_file(f)->private_data, attr,
 					     uattr);
 	else if (fd_file(f)->f_op == &btf_fops)
-		err = bpf_btf_get_info_by_fd(fd_file(f), fd_file(f)->private_data, attr, uattr);
+		return bpf_btf_get_info_by_fd(fd_file(f), fd_file(f)->private_data, attr, uattr);
 	else if (fd_file(f)->f_op == &bpf_link_fops || fd_file(f)->f_op == &bpf_link_fops_poll)
-		err = bpf_link_get_info_by_fd(fd_file(f), fd_file(f)->private_data,
+		return bpf_link_get_info_by_fd(fd_file(f), fd_file(f)->private_data,
 					      attr, uattr);
-	else
-		err = -EINVAL;
-
-	fdput(f);
-	return err;
+	return -EINVAL;
 }
 
 #define BPF_BTF_LOAD_LAST_FIELD btf_token_fd
-- 
2.43.5

[PATCH bpf-next 7/8] security,bpf: constify struct path in bpf_token_create() LSM hook

[Andrii Nakryiko]

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>
---
 include/linux/lsm_hook_defs.h | 2 +-
 include/linux/security.h      | 4 ++--
 security/security.c           | 2 +-
 security/selinux/hooks.c      | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

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;
 
-- 
2.43.5

[PATCH bpf-next 8/8] bpf: convert bpf_token_create() to CLASS(fd, ...)

[Andrii Nakryiko]

From: Al Viro <viro@zeniv.linux.org.uk>

Keep file reference through the entire thing, don't bother with grabbing
struct path reference 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.

Reviewed-by: Christian Brauner <brauner@kernel.org>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 kernel/bpf/token.c | 65 ++++++++++++++++------------------------------
 1 file changed, 23 insertions(+), 42 deletions(-)

diff --git a/kernel/bpf/token.c b/kernel/bpf/token.c
index 9b92cb886d49..dcbec1a0dfb3 100644
--- a/kernel/bpf/token.c
+++ b/kernel/bpf/token.c
@@ -116,67 +116,52 @@ int bpf_token_create(union bpf_attr *attr)
 	struct user_namespace *userns;
 	struct inode *inode;
 	struct file *file;
+	CLASS(fd, f)(attr->token_create.bpffs_fd);
 	struct path path;
-	struct fd f;
+	struct super_block *sb;
 	umode_t mode;
 	int err, fd;
 
-	f = fdget(attr->token_create.bpffs_fd);
-	if (!fd_file(f))
+	if (fd_empty(f))
 		return -EBADF;
 
 	path = fd_file(f)->f_path;
-	path_get(&path);
-	fdput(f);
+	sb = path.dentry->d_sb;
 
-	if (path.dentry != path.mnt->mnt_sb->s_root) {
-		err = -EINVAL;
-		goto out_path;
-	}
-	if (path.mnt->mnt_sb->s_op != &bpf_super_ops) {
-		err = -EINVAL;
-		goto out_path;
-	}
+	if (path.dentry != sb->s_root)
+		return -EINVAL;
+	if (sb->s_op != &bpf_super_ops)
+		return -EINVAL;
 	err = path_permission(&path, MAY_ACCESS);
 	if (err)
-		goto out_path;
+		return err;
 
-	userns = path.dentry->d_sb->s_user_ns;
+	userns = sb->s_user_ns;
 	/*
 	 * Enforce that creators of BPF tokens are in the same user
 	 * namespace as the BPF FS instance. This makes reasoning about
 	 * permissions a lot easier and we can always relax this later.
 	 */
-	if (current_user_ns() != userns) {
-		err = -EPERM;
-		goto out_path;
-	}
-	if (!ns_capable(userns, CAP_BPF)) {
-		err = -EPERM;
-		goto out_path;
-	}
+	if (current_user_ns() != userns)
+		return -EPERM;
+	if (!ns_capable(userns, CAP_BPF))
+		return -EPERM;
 
 	/* Creating BPF token in init_user_ns doesn't make much sense. */
-	if (current_user_ns() == &init_user_ns) {
-		err = -EOPNOTSUPP;
-		goto out_path;
-	}
+	if (current_user_ns() == &init_user_ns)
+		return -EOPNOTSUPP;
 
-	mnt_opts = path.dentry->d_sb->s_fs_info;
+	mnt_opts = sb->s_fs_info;
 	if (mnt_opts->delegate_cmds == 0 &&
 	    mnt_opts->delegate_maps == 0 &&
 	    mnt_opts->delegate_progs == 0 &&
-	    mnt_opts->delegate_attachs == 0) {
-		err = -ENOENT; /* no BPF token delegation is set up */
-		goto out_path;
-	}
+	    mnt_opts->delegate_attachs == 0)
+		return -ENOENT; /* no BPF token delegation is set up */
 
 	mode = S_IFREG | ((S_IRUSR | S_IWUSR) & ~current_umask());
-	inode = bpf_get_inode(path.mnt->mnt_sb, NULL, mode);
-	if (IS_ERR(inode)) {
-		err = PTR_ERR(inode);
-		goto out_path;
-	}
+	inode = bpf_get_inode(sb, NULL, mode);
+	if (IS_ERR(inode))
+		return PTR_ERR(inode);
 
 	inode->i_op = &bpf_token_iops;
 	inode->i_fop = &bpf_token_fops;
@@ -185,8 +170,7 @@ int bpf_token_create(union bpf_attr *attr)
 	file = alloc_file_pseudo(inode, path.mnt, BPF_TOKEN_INODE_NAME, O_RDWR, &bpf_token_fops);
 	if (IS_ERR(file)) {
 		iput(inode);
-		err = PTR_ERR(file);
-		goto out_path;
+		return PTR_ERR(file);
 	}
 
 	token = kzalloc(sizeof(*token), GFP_USER);
@@ -218,15 +202,12 @@ int bpf_token_create(union bpf_attr *attr)
 	file->private_data = token;
 	fd_install(fd, file);
 
-	path_put(&path);
 	return fd;
 
 out_token:
 	bpf_token_free(token);
 out_file:
 	fput(file);
-out_path:
-	path_put(&path);
 	return err;
 }
 
-- 
2.43.5

Re: [PATCH bpf-next 3/8] bpf: factor out fetching bpf_map from FD and adding it to used_maps list

[Jiri Olsa]

On Tue, Aug 13, 2024 at 04:02:55PM -0700, Andrii Nakryiko wrote:
> Factor out the logic to extract bpf_map instances from FD embedded in
> bpf_insns, adding it to the list of used_maps (unless it's already
> there, in which case we just reuse map's index). This simplifies the
> logic in resolve_pseudo_ldimm64(), especially around `struct fd`
> handling, as all that is now neatly contained in the helper and doesn't
> leak into a dozen error handling paths.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  kernel/bpf/verifier.c | 115 ++++++++++++++++++++++++------------------
>  1 file changed, 66 insertions(+), 49 deletions(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index df3be12096cf..14e4ef687a59 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -18865,6 +18865,58 @@ static bool bpf_map_is_cgroup_storage(struct bpf_map *map)
>  		map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE);
>  }
>  
> +/* Add map behind fd to used maps list, if it's not already there, and return
> + * its index. Also set *reused to true if this map was already in the list of
> + * used maps.
> + * Returns <0 on error, or >= 0 index, on success.
> + */
> +static int add_used_map_from_fd(struct bpf_verifier_env *env, int fd, bool *reused)
> +{
> +	struct fd f = fdget(fd);

using 'CLASS(fd, f)(fd)' would remove few fdput lines below?

jirka

> +	struct bpf_map *map;
> +	int i;
> +
> +	map = __bpf_map_get(f);
> +	if (IS_ERR(map)) {
> +		verbose(env, "fd %d is not pointing to valid bpf_map\n", fd);
> +		return PTR_ERR(map);
> +	}
> +
> +	/* check whether we recorded this map already */
> +	for (i = 0; i < env->used_map_cnt; i++) {
> +		if (env->used_maps[i] == map) {
> +			*reused = true;
> +			fdput(f);
> +			return i;
> +		}
> +	}
> +
> +	if (env->used_map_cnt >= MAX_USED_MAPS) {
> +		verbose(env, "The total number of maps per program has reached the limit of %u\n",
> +			MAX_USED_MAPS);
> +		fdput(f);
> +		return -E2BIG;
> +	}
> +
> +	if (env->prog->sleepable)
> +		atomic64_inc(&map->sleepable_refcnt);
> +
> +	/* hold the map. If the program is rejected by verifier,
> +	 * the map will be released by release_maps() or it
> +	 * will be used by the valid program until it's unloaded
> +	 * and all maps are released in bpf_free_used_maps()
> +	 */
> +	bpf_map_inc(map);
> +
> +	*reused = false;
> +	env->used_maps[env->used_map_cnt++] = map;
> +
> +	fdput(f);
> +
> +	return env->used_map_cnt - 1;
> +
> +}
> +
>  /* find and rewrite pseudo imm in ld_imm64 instructions:
>   *
>   * 1. if it accesses map FD, replace it with actual map pointer.
> @@ -18876,7 +18928,7 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
>  {
>  	struct bpf_insn *insn = env->prog->insnsi;
>  	int insn_cnt = env->prog->len;
> -	int i, j, err;
> +	int i, err;
>  
>  	err = bpf_prog_calc_tag(env->prog);
>  	if (err)
> @@ -18893,9 +18945,10 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
>  		if (insn[0].code == (BPF_LD | BPF_IMM | BPF_DW)) {
>  			struct bpf_insn_aux_data *aux;
>  			struct bpf_map *map;
> -			struct fd f;
> +			int map_idx;
>  			u64 addr;
>  			u32 fd;
> +			bool reused;
>  
>  			if (i == insn_cnt - 1 || insn[1].code != 0 ||
>  			    insn[1].dst_reg != 0 || insn[1].src_reg != 0 ||
> @@ -18956,20 +19009,18 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
>  				break;
>  			}
>  
> -			f = fdget(fd);
> -			map = __bpf_map_get(f);
> -			if (IS_ERR(map)) {
> -				verbose(env, "fd %d is not pointing to valid bpf_map\n", fd);
> -				return PTR_ERR(map);
> -			}
> +			map_idx = add_used_map_from_fd(env, fd, &reused);
> +			if (map_idx < 0)
> +				return map_idx;
> +			map = env->used_maps[map_idx];
> +
> +			aux = &env->insn_aux_data[i];
> +			aux->map_index = map_idx;
>  
>  			err = check_map_prog_compatibility(env, map, env->prog);
> -			if (err) {
> -				fdput(f);
> +			if (err)
>  				return err;
> -			}
>  
> -			aux = &env->insn_aux_data[i];
>  			if (insn[0].src_reg == BPF_PSEUDO_MAP_FD ||
>  			    insn[0].src_reg == BPF_PSEUDO_MAP_IDX) {
>  				addr = (unsigned long)map;
> @@ -18978,13 +19029,11 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
>  
>  				if (off >= BPF_MAX_VAR_OFF) {
>  					verbose(env, "direct value offset of %u is not allowed\n", off);
> -					fdput(f);
>  					return -EINVAL;
>  				}
>  
>  				if (!map->ops->map_direct_value_addr) {
>  					verbose(env, "no direct value access support for this map type\n");
> -					fdput(f);
>  					return -EINVAL;
>  				}
>  
> @@ -18992,7 +19041,6 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
>  				if (err) {
>  					verbose(env, "invalid access to map value pointer, value_size=%u off=%u\n",
>  						map->value_size, off);
> -					fdput(f);
>  					return err;
>  				}
>  
> @@ -19003,70 +19051,39 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
>  			insn[0].imm = (u32)addr;
>  			insn[1].imm = addr >> 32;
>  
> -			/* check whether we recorded this map already */
> -			for (j = 0; j < env->used_map_cnt; j++) {
> -				if (env->used_maps[j] == map) {
> -					aux->map_index = j;
> -					fdput(f);
> -					goto next_insn;
> -				}
> -			}
> -
> -			if (env->used_map_cnt >= MAX_USED_MAPS) {
> -				verbose(env, "The total number of maps per program has reached the limit of %u\n",
> -					MAX_USED_MAPS);
> -				fdput(f);
> -				return -E2BIG;
> -			}
> -
> -			if (env->prog->sleepable)
> -				atomic64_inc(&map->sleepable_refcnt);
> -			/* hold the map. If the program is rejected by verifier,
> -			 * the map will be released by release_maps() or it
> -			 * will be used by the valid program until it's unloaded
> -			 * and all maps are released in bpf_free_used_maps()
> -			 */
> -			bpf_map_inc(map);
> -
> -			aux->map_index = env->used_map_cnt;
> -			env->used_maps[env->used_map_cnt++] = map;
> +			/* proceed with extra checks only if its newly added used map */
> +			if (reused)
> +				goto next_insn;
>  
>  			if (bpf_map_is_cgroup_storage(map) &&
>  			    bpf_cgroup_storage_assign(env->prog->aux, map)) {
>  				verbose(env, "only one cgroup storage of each type is allowed\n");
> -				fdput(f);
>  				return -EBUSY;
>  			}
>  			if (map->map_type == BPF_MAP_TYPE_ARENA) {
>  				if (env->prog->aux->arena) {
>  					verbose(env, "Only one arena per program\n");
> -					fdput(f);
>  					return -EBUSY;
>  				}
>  				if (!env->allow_ptr_leaks || !env->bpf_capable) {
>  					verbose(env, "CAP_BPF and CAP_PERFMON are required to use arena\n");
> -					fdput(f);
>  					return -EPERM;
>  				}
>  				if (!env->prog->jit_requested) {
>  					verbose(env, "JIT is required to use arena\n");
> -					fdput(f);
>  					return -EOPNOTSUPP;
>  				}
>  				if (!bpf_jit_supports_arena()) {
>  					verbose(env, "JIT doesn't support arena\n");
> -					fdput(f);
>  					return -EOPNOTSUPP;
>  				}
>  				env->prog->aux->arena = (void *)map;
>  				if (!bpf_arena_get_user_vm_start(env->prog->aux->arena)) {
>  					verbose(env, "arena's user address must be set via map_extra or mmap()\n");
> -					fdput(f);
>  					return -EINVAL;
>  				}
>  			}
>  
> -			fdput(f);
>  next_insn:
>  			insn++;
>  			i++;
> -- 
> 2.43.5
> 
> 

Re: [PATCH bpf-next 3/8] bpf: factor out fetching bpf_map from FD and adding it to used_maps list

[Andrii Nakryiko]

On Wed, Aug 14, 2024 at 2:39 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Tue, Aug 13, 2024 at 04:02:55PM -0700, Andrii Nakryiko wrote:
> > Factor out the logic to extract bpf_map instances from FD embedded in
> > bpf_insns, adding it to the list of used_maps (unless it's already
> > there, in which case we just reuse map's index). This simplifies the
> > logic in resolve_pseudo_ldimm64(), especially around `struct fd`
> > handling, as all that is now neatly contained in the helper and doesn't
> > leak into a dozen error handling paths.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> >  kernel/bpf/verifier.c | 115 ++++++++++++++++++++++++------------------
> >  1 file changed, 66 insertions(+), 49 deletions(-)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index df3be12096cf..14e4ef687a59 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -18865,6 +18865,58 @@ static bool bpf_map_is_cgroup_storage(struct bpf_map *map)
> >               map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE);
> >  }
> >
> > +/* Add map behind fd to used maps list, if it's not already there, and return
> > + * its index. Also set *reused to true if this map was already in the list of
> > + * used maps.
> > + * Returns <0 on error, or >= 0 index, on success.
> > + */
> > +static int add_used_map_from_fd(struct bpf_verifier_env *env, int fd, bool *reused)
> > +{
> > +     struct fd f = fdget(fd);
>
> using 'CLASS(fd, f)(fd)' would remove few fdput lines below?

That's done in the next patch once we change __bpf_map_get() behavior
to allow usage of CLASS(fd, ...)

>
> jirka
>
> > +     struct bpf_map *map;
> > +     int i;
> > +
> > +     map = __bpf_map_get(f);
> > +     if (IS_ERR(map)) {
> > +             verbose(env, "fd %d is not pointing to valid bpf_map\n", fd);
> > +             return PTR_ERR(map);
> > +     }
> > +

[...]

Re: [PATCH bpf-next 0/8] BPF follow ups to struct fd refactorings

[Andrii Nakryiko]

On Tue, Aug 13, 2024 at 4:03 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> This patch set extracts all the BPF-related changes done in [0] into
> a separate series based on top of stable-struct_fd branch ([1]) merged into
> bpf-next tree. There are also a few changes, additions, and adjustments:
>
>   - patch subjects adjusted to use "bpf: " prefix consistently;
>   - patch #2 is extracting bpf-related changes from original patch #19
>     ("fdget_raw() users: switch to CLASS(fd_raw, ...)") and is ordered a bit
>     earlier in this patch set;
>   - patch #3 is reimplemented and replaces original patch #17
>     ("bpf: resolve_pseudo_ldimm64(): take handling of a single ldimm64 insn into helper")
>     completely;
>   - in patch #4 ("bpf: switch maps to CLASS(fd, ...)"), which was originally
>     patch #18 ("bpf maps: switch to CLASS(fd, ...)"), I've combined
>     __bpf_get_map() and bpf_file_to_map() into __bpf_get_map(), as the latter
>     is only used from it and makes no sense to keep separate;
>   - as part of rebasing patch #4, I adjusted newly added in patch #3
>     add_used_map_from_fd() function to use CLASS(fd, ...), as now
>     __bpf_get_map() doesn't do its own fdput() anymore. This made unnecessary
>     any further bpf_map_inc() changes, because we still rely on struct fd to
>     keep map's file reference alive;
>   - patches #5 and #6 are BPF-specific bits extracted from original patch #23
>     ("fdget(), trivial conversions") and #24 ("fdget(), more trivial conversions");
>   - patch #7 constifies security_bpf_token_create() LSM hook;
>   - patch #8 is original patch #35 ("convert bpf_token_create()"), with
>     path_get()+path_put() removed now that LSM hook above was adjusted.
>
> All these patches were pushed into a separate bpf-next/struct_fd branch ([2]).
> They were also merged into bpf-next/for-next so they can get early testing in
> linux-next.
>
>   [0] https://lore.kernel.org/bpf/20240730050927.GC5334@ZenIV/
>   [1] https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git/log/?h=stable-struct_fd
>   [2] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/log/?h=struct_fd
>
> Al Viro (6):
>   bpf: convert __bpf_prog_get() to CLASS(fd, ...)
>   bpf: switch fdget_raw() uses to CLASS(fd_raw, ...)
>   bpf: switch maps to CLASS(fd, ...)
>   bpf: trivial conversions for fdget()
>   bpf: more trivial fdget() conversions
>   bpf: convert bpf_token_create() to CLASS(fd, ...)
>
> Andrii Nakryiko (2):
>   bpf: factor out fetching bpf_map from FD and adding it to used_maps
>     list
>   security,bpf: constify struct path in bpf_token_create() LSM hook
>

Al, Christian,

Can you guys please take a look and let us know if this looks sane and
fine to you? I kept Al's patches mostly intact (see my notes in the
cover letter above), and patch #3 does the refactoring I proposed
earlier, keeping explicit fdput() temporarily, until Al's
__bpf_map_get() refactoring which allows and nice and simple CLASS(fd)
conversion.

I think we end up at exactly what the end goal of the original series
is: using CLASS(fd, ...) throughout with all the benefits.

>  include/linux/bpf.h            |  11 +-
>  include/linux/lsm_hook_defs.h  |   2 +-
>  include/linux/security.h       |   4 +-
>  kernel/bpf/bpf_inode_storage.c |  24 ++---
>  kernel/bpf/btf.c               |  11 +-
>  kernel/bpf/map_in_map.c        |  38 ++-----
>  kernel/bpf/syscall.c           | 181 +++++++++------------------------
>  kernel/bpf/token.c             |  74 +++++---------
>  kernel/bpf/verifier.c          | 110 +++++++++++---------
>  net/core/sock_map.c            |  23 ++---
>  security/security.c            |   2 +-
>  security/selinux/hooks.c       |   2 +-
>  12 files changed, 179 insertions(+), 303 deletions(-)
>
> --
> 2.43.5
>

Re: [PATCH bpf-next 7/8] security,bpf: constify struct path in bpf_token_create() LSM hook

[Andrii Nakryiko]

On Tue, Aug 13, 2024 at 4:03 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> 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>
> ---
>  include/linux/lsm_hook_defs.h | 2 +-
>  include/linux/security.h      | 4 ++--
>  security/security.c           | 2 +-
>  security/selinux/hooks.c      | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
>

Paul,

I just realized that I originally forgot to cc you and
linux-security-modules@ on this entire patch set and I apologize for
that. You can find the entire series at [0], if you'd like to see a
bit wider context.

But if you can, please check this patch specifically and give your
ack, if it's fine with you.

Ideally we land this patch together with the rest of Al's and mine
refactorings, as it allows us to avoid that ugly path_get/path_put
workaround that was added by Al initially (see [1]). LSM-specific
changes are pretty trivial and hopefully are not controversial.

Thanks!

  [0] https://lore.kernel.org/bpf/20240813230300.915127-1-andrii@kernel.org/
  [1] https://lore.kernel.org/bpf/20240730051625.14349-35-viro@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;
>
> --
> 2.43.5
>

Re: [PATCH bpf-next 7/8] security,bpf: constify struct path in bpf_token_create() LSM hook

[Paul Moore]

On Tue, Aug 27, 2024 at 7:02 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
> On Tue, Aug 13, 2024 at 4:03 PM Andrii Nakryiko <andrii@kernel.org> wrote:
> >
> > 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>
> > ---
> >  include/linux/lsm_hook_defs.h | 2 +-
> >  include/linux/security.h      | 4 ++--
> >  security/security.c           | 2 +-
> >  security/selinux/hooks.c      | 2 +-
> >  4 files changed, 5 insertions(+), 5 deletions(-)
> >
>
> Paul,
>
> I just realized that I originally forgot to cc you and
> linux-security-modules@ on this entire patch set and I apologize for
> that. You can find the entire series at [0], if you'd like to see a
> bit wider context.
>
> But if you can, please check this patch specifically and give your
> ack, if it's fine with you.

Hi Andrii,

Thanks for sending an email about this, many maintainers don't
remember to CC the LSM list when making changes like this and I really
appreciate it when people do, so thank you for that (even if it is a
teeny bit late <g>).  To be honest, I saw this patch back on the 14th
as I've got some tools which watch for LSM/security related commits
hitting linux-next or Linus' tree that don't originate from one of the
LSM trees and I thought it looked okay, my ACK is below.

> Ideally we land this patch together with the rest of Al's and mine
> refactorings, as it allows us to avoid that ugly path_get/path_put
> workaround that was added by Al initially (see [1]). LSM-specific
> changes are pretty trivial and hopefully are not controversial.

Acked-by: Paul Moore <paul@paul-moore.com> (LSM/SELinux)

-- 
paul-moore.com

Re: [PATCH bpf-next 7/8] security,bpf: constify struct path in bpf_token_create() LSM hook

[Andrii Nakryiko]

On Tue, Aug 27, 2024 at 4:21 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Tue, Aug 27, 2024 at 7:02 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> > On Tue, Aug 13, 2024 at 4:03 PM Andrii Nakryiko <andrii@kernel.org> wrote:
> > >
> > > 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>
> > > ---
> > >  include/linux/lsm_hook_defs.h | 2 +-
> > >  include/linux/security.h      | 4 ++--
> > >  security/security.c           | 2 +-
> > >  security/selinux/hooks.c      | 2 +-
> > >  4 files changed, 5 insertions(+), 5 deletions(-)
> > >
> >
> > Paul,
> >
> > I just realized that I originally forgot to cc you and
> > linux-security-modules@ on this entire patch set and I apologize for
> > that. You can find the entire series at [0], if you'd like to see a
> > bit wider context.
> >
> > But if you can, please check this patch specifically and give your
> > ack, if it's fine with you.
>
> Hi Andrii,
>
> Thanks for sending an email about this, many maintainers don't
> remember to CC the LSM list when making changes like this and I really
> appreciate it when people do, so thank you for that (even if it is a
> teeny bit late <g>).  To be honest, I saw this patch back on the 14th

Yep, my bad, I will try to be less forgetful next time. Thanks for a
quick reply and your ack!

> as I've got some tools which watch for LSM/security related commits
> hitting linux-next or Linus' tree that don't originate from one of the
> LSM trees and I thought it looked okay, my ACK is below.
>
> > Ideally we land this patch together with the rest of Al's and mine
> > refactorings, as it allows us to avoid that ugly path_get/path_put
> > workaround that was added by Al initially (see [1]). LSM-specific
> > changes are pretty trivial and hopefully are not controversial.
>
> Acked-by: Paul Moore <paul@paul-moore.com> (LSM/SELinux)
>
> --
> paul-moore.com

Re: [PATCH bpf-next 0/8] BPF follow ups to struct fd refactorings

[Al Viro]

On Tue, Aug 27, 2024 at 03:55:28PM -0700, Andrii Nakryiko wrote:
> > They were also merged into bpf-next/for-next so they can get early testing in
> > linux-next.

Umm...  I see that stuff in bpf-next/struct_fd, but not in your for-next.

> Can you guys please take a look and let us know if this looks sane and
> fine to you? I kept Al's patches mostly intact (see my notes in the
> cover letter above), and patch #3 does the refactoring I proposed
> earlier, keeping explicit fdput() temporarily, until Al's
> __bpf_map_get() refactoring which allows and nice and simple CLASS(fd)
> conversion.
> 
> I think we end up at exactly what the end goal of the original series
> is: using CLASS(fd, ...) throughout with all the benefits.

Looks sane.

Re: [PATCH bpf-next 0/8] BPF follow ups to struct fd refactorings

[Andrii Nakryiko]

On Thu, Sep 12, 2024 at 4:57 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Tue, Aug 27, 2024 at 03:55:28PM -0700, Andrii Nakryiko wrote:
> > > They were also merged into bpf-next/for-next so they can get early testing in
> > > linux-next.
>
> Umm...  I see that stuff in bpf-next/struct_fd, but not in your for-next.

We have a new process with for-next and my merge was probably
accidentally dropped at some point... But there was definitely a
period of time when these patches were in for-next, so they got some
compile-testing already and should be good to go.

>
> > Can you guys please take a look and let us know if this looks sane and
> > fine to you? I kept Al's patches mostly intact (see my notes in the
> > cover letter above), and patch #3 does the refactoring I proposed
> > earlier, keeping explicit fdput() temporarily, until Al's
> > __bpf_map_get() refactoring which allows and nice and simple CLASS(fd)
> > conversion.
> >
> > I think we end up at exactly what the end goal of the original series
> > is: using CLASS(fd, ...) throughout with all the benefits.
>
> Looks sane.

Alright, good to know. I'll follow up with BPF maintainers on the best
way to land all that, thanks.

Re: [PATCH bpf-next 0/8] BPF follow ups to struct fd refactorings

[Al Viro]

On Thu, Sep 12, 2024 at 05:10:57PM -0700, Andrii Nakryiko wrote:
> On Thu, Sep 12, 2024 at 4:57 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > On Tue, Aug 27, 2024 at 03:55:28PM -0700, Andrii Nakryiko wrote:
> > > > They were also merged into bpf-next/for-next so they can get early testing in
> > > > linux-next.
> >
> > Umm...  I see that stuff in bpf-next/struct_fd, but not in your for-next.
> 
> We have a new process with for-next and my merge was probably
> accidentally dropped at some point... But there was definitely a
> period of time when these patches were in for-next, so they got some
> compile-testing already and should be good to go.

I should've pushed the base branch into #for-next; mea culpa...