bpf: Fix cgroup local storage prog tracking

Recently noticed that we're tracking programs related to local storage maps
through their prog pointer. This is a wrong assumption since the prog pointer
can still change throughout the verification process, for example, whenever
bpf_patch_insn_single() is called.

Therefore, the prog pointer that was assigned via bpf_cgroup_storage_assign()
is not guaranteed to be the same as we pass in bpf_cgroup_storage_release()
and the map would therefore remain in busy state forever. Fix this by using
the prog's aux pointer which is stable throughout verification and beyond.

Fixes: de9cbbaadba5 ("bpf: introduce cgroup storage maps")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Cc: Roman Gushchin <guro@fb.com>
Cc: Martin KaFai Lau <kafai@fb.com>
Link: https://lore.kernel.org/bpf/1471c69eca3022218666f909bc927a92388fd09e.1576580332.git.daniel@iogearbox.net

diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index 169fd25f6bc2d697f078abd3e9e430499fdb9aa7..9be71c195d7450ea40631e464c5f08beee8a0878 100644 (file)
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -157,8 +157,8 @@ void bpf_cgroup_storage_link(struct bpf_cgroup_storage *storage,
                             struct cgroup *cgroup,
                             enum bpf_attach_type type);
 void bpf_cgroup_storage_unlink(struct bpf_cgroup_storage *storage);
-int bpf_cgroup_storage_assign(struct bpf_prog *prog, struct bpf_map *map);
-void bpf_cgroup_storage_release(struct bpf_prog *prog, struct bpf_map *map);
+int bpf_cgroup_storage_assign(struct bpf_prog_aux *aux, struct bpf_map *map);
+void bpf_cgroup_storage_release(struct bpf_prog_aux *aux, struct bpf_map *map);
 
 int bpf_percpu_cgroup_storage_copy(struct bpf_map *map, void *key, void *value);
 int bpf_percpu_cgroup_storage_update(struct bpf_map *map, void *key,
@@ -360,9 +360,9 @@ static inline int cgroup_bpf_prog_query(const union bpf_attr *attr,
 
 static inline void bpf_cgroup_storage_set(
        struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE]) {}
-static inline int bpf_cgroup_storage_assign(struct bpf_prog *prog,
+static inline int bpf_cgroup_storage_assign(struct bpf_prog_aux *aux,
                                            struct bpf_map *map) { return 0; }
-static inline void bpf_cgroup_storage_release(struct bpf_prog *prog,
+static inline void bpf_cgroup_storage_release(struct bpf_prog_aux *aux,
                                              struct bpf_map *map) {}
 static inline struct bpf_cgroup_storage *bpf_cgroup_storage_alloc(
        struct bpf_prog *prog, enum bpf_cgroup_storage_type stype) { return NULL; }

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 6231858df7239913d77d6bae7ea0ca2634ec2aae..af6b738cf435cce5b7377eaf1aaf57e7ae8260a4 100644 (file)
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2043,8 +2043,7 @@ static void bpf_free_cgroup_storage(struct bpf_prog_aux *aux)
        for_each_cgroup_storage_type(stype) {
                if (!aux->cgroup_storage[stype])
                        continue;
-               bpf_cgroup_storage_release(aux->prog,
-                                          aux->cgroup_storage[stype]);
+               bpf_cgroup_storage_release(aux, aux->cgroup_storage[stype]);
        }
 }
 

diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c
index 2ba750725cb26d18fe19cac08a30e0e6f48a9e86..6bf605dd4b94f0b33f09384478d4e3379f269176 100644 (file)
--- a/kernel/bpf/local_storage.c
+++ b/kernel/bpf/local_storage.c
@@ -20,7 +20,7 @@ struct bpf_cgroup_storage_map {
        struct bpf_map map;
 
        spinlock_t lock;
-       struct bpf_prog *prog;
+       struct bpf_prog_aux *aux;
        struct rb_root root;
        struct list_head list;
 };
@@ -420,7 +420,7 @@ const struct bpf_map_ops cgroup_storage_map_ops = {
        .map_seq_show_elem = cgroup_storage_seq_show_elem,
 };
 
-int bpf_cgroup_storage_assign(struct bpf_prog *prog, struct bpf_map *_map)
+int bpf_cgroup_storage_assign(struct bpf_prog_aux *aux, struct bpf_map *_map)
 {
        enum bpf_cgroup_storage_type stype = cgroup_storage_type(_map);
        struct bpf_cgroup_storage_map *map = map_to_storage(_map);
@@ -428,14 +428,14 @@ int bpf_cgroup_storage_assign(struct bpf_prog *prog, struct bpf_map *_map)
 
        spin_lock_bh(&map->lock);
 
-       if (map->prog && map->prog != prog)
+       if (map->aux && map->aux != aux)
                goto unlock;
-       if (prog->aux->cgroup_storage[stype] &&
-           prog->aux->cgroup_storage[stype] != _map)
+       if (aux->cgroup_storage[stype] &&
+           aux->cgroup_storage[stype] != _map)
                goto unlock;
 
-       map->prog = prog;
-       prog->aux->cgroup_storage[stype] = _map;
+       map->aux = aux;
+       aux->cgroup_storage[stype] = _map;
        ret = 0;
 unlock:
        spin_unlock_bh(&map->lock);
@@ -443,16 +443,16 @@ unlock:
        return ret;
 }
 
-void bpf_cgroup_storage_release(struct bpf_prog *prog, struct bpf_map *_map)
+void bpf_cgroup_storage_release(struct bpf_prog_aux *aux, struct bpf_map *_map)
 {
        enum bpf_cgroup_storage_type stype = cgroup_storage_type(_map);
        struct bpf_cgroup_storage_map *map = map_to_storage(_map);
 
        spin_lock_bh(&map->lock);
-       if (map->prog == prog) {
-               WARN_ON(prog->aux->cgroup_storage[stype] != _map);
-               map->prog = NULL;
-               prog->aux->cgroup_storage[stype] = NULL;
+       if (map->aux == aux) {
+               WARN_ON(aux->cgroup_storage[stype] != _map);
+               map->aux = NULL;
+               aux->cgroup_storage[stype] = NULL;
        }
        spin_unlock_bh(&map->lock);
 }

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a1acdce77070902cf6c34b27965ed52683f52cfe..6ef71429d997f1e1c803894ac69fb60a38d35daf 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -8268,7 +8268,7 @@ static int replace_map_fd_with_map_ptr(struct bpf_verifier_env *env)
                        env->used_maps[env->used_map_cnt++] = map;
 
                        if (bpf_map_is_cgroup_storage(map) &&
-                           bpf_cgroup_storage_assign(env->prog, 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;