bpf: Move map/prog compatibility checks

Move some inlined map/prog compatibility checks from the
resolve_pseudo_ldimm64() function to the dedicated
check_map_prog_compatibility() function. Call the latter function
from the add_used_map_from_fd() function directly.

This simplifies code and optimizes logic a bit, as before these
changes the check_map_prog_compatibility() function was executed on
every map usage, which doesn't make sense, as it doesn't include any
per-instruction checks, only map type vs. prog type.

(This patch also simplifies a consequent patch which will call the
add_used_map_from_fd() function from another code path.)

Signed-off-by: Anton Protopopov <aspsk@isovalent.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/20241213130934.1087929-3-aspsk@isovalent.com

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index c855e7905c356ae4cb50a9ae8ef73c7e7d0de690..89bba0de853f1ae8707cc6a154b7b1c6d8700abf 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -19368,6 +19368,12 @@ static bool is_tracing_prog_type(enum bpf_prog_type type)
        }
 }
 
+static bool bpf_map_is_cgroup_storage(struct bpf_map *map)
+{
+       return (map->map_type == BPF_MAP_TYPE_CGROUP_STORAGE ||
+               map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE);
+}
+
 static int check_map_prog_compatibility(struct bpf_verifier_env *env,
                                        struct bpf_map *map,
                                        struct bpf_prog *prog)
@@ -19446,25 +19452,48 @@ static int check_map_prog_compatibility(struct bpf_verifier_env *env,
                        return -EINVAL;
                }
 
-       return 0;
-}
+       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");
+               return -EBUSY;
+       }
 
-static bool bpf_map_is_cgroup_storage(struct bpf_map *map)
-{
-       return (map->map_type == BPF_MAP_TYPE_CGROUP_STORAGE ||
-               map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE);
+       if (map->map_type == BPF_MAP_TYPE_ARENA) {
+               if (env->prog->aux->arena) {
+                       verbose(env, "Only one arena per program\n");
+                       return -EBUSY;
+               }
+               if (!env->allow_ptr_leaks || !env->bpf_capable) {
+                       verbose(env, "CAP_BPF and CAP_PERFMON are required to use arena\n");
+                       return -EPERM;
+               }
+               if (!env->prog->jit_requested) {
+                       verbose(env, "JIT is required to use arena\n");
+                       return -EOPNOTSUPP;
+               }
+               if (!bpf_jit_supports_arena()) {
+                       verbose(env, "JIT doesn't support arena\n");
+                       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");
+                       return -EINVAL;
+               }
+       }
+
+       return 0;
 }
 
 /* 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.
+ * its index.
  * 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)
+static int add_used_map_from_fd(struct bpf_verifier_env *env, int fd)
 {
        CLASS(fd, f)(fd);
        struct bpf_map *map;
-       int i;
+       int i, err;
 
        map = __bpf_map_get(f);
        if (IS_ERR(map)) {
@@ -19473,12 +19502,9 @@ static int add_used_map_from_fd(struct bpf_verifier_env *env, int fd, bool *reus
        }
 
        /* check whether we recorded this map already */
-       for (i = 0; i < env->used_map_cnt; i++) {
-               if (env->used_maps[i] == map) {
-                       *reused = true;
+       for (i = 0; i < env->used_map_cnt; i++)
+               if (env->used_maps[i] == map)
                        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",
@@ -19486,6 +19512,10 @@ static int add_used_map_from_fd(struct bpf_verifier_env *env, int fd, bool *reus
                return -E2BIG;
        }
 
+       err = check_map_prog_compatibility(env, map, env->prog);
+       if (err)
+               return err;
+
        if (env->prog->sleepable)
                atomic64_inc(&map->sleepable_refcnt);
 
@@ -19496,7 +19526,6 @@ static int add_used_map_from_fd(struct bpf_verifier_env *env, int fd, bool *reus
         */
        bpf_map_inc(map);
 
-       *reused = false;
        env->used_maps[env->used_map_cnt++] = map;
 
        return env->used_map_cnt - 1;
@@ -19533,7 +19562,6 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
                        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 ||
@@ -19594,7 +19622,7 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
                                break;
                        }
 
-                       map_idx = add_used_map_from_fd(env, fd, &reused);
+                       map_idx = add_used_map_from_fd(env, fd);
                        if (map_idx < 0)
                                return map_idx;
                        map = env->used_maps[map_idx];
@@ -19602,10 +19630,6 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
                        aux = &env->insn_aux_data[i];
                        aux->map_index = map_idx;
 
-                       err = check_map_prog_compatibility(env, map, env->prog);
-                       if (err)
-                               return err;
-
                        if (insn[0].src_reg == BPF_PSEUDO_MAP_FD ||
                            insn[0].src_reg == BPF_PSEUDO_MAP_IDX) {
                                addr = (unsigned long)map;
@@ -19636,39 +19660,6 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
                        insn[0].imm = (u32)addr;
                        insn[1].imm = addr >> 32;
 
-                       /* 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");
-                               return -EBUSY;
-                       }
-                       if (map->map_type == BPF_MAP_TYPE_ARENA) {
-                               if (env->prog->aux->arena) {
-                                       verbose(env, "Only one arena per program\n");
-                                       return -EBUSY;
-                               }
-                               if (!env->allow_ptr_leaks || !env->bpf_capable) {
-                                       verbose(env, "CAP_BPF and CAP_PERFMON are required to use arena\n");
-                                       return -EPERM;
-                               }
-                               if (!env->prog->jit_requested) {
-                                       verbose(env, "JIT is required to use arena\n");
-                                       return -EOPNOTSUPP;
-                               }
-                               if (!bpf_jit_supports_arena()) {
-                                       verbose(env, "JIT doesn't support arena\n");
-                                       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");
-                                       return -EINVAL;
-                               }
-                       }
-
 next_insn:
                        insn++;
                        i++;