bpf: Mutex protect used_maps array and count

To support modifying the used_maps array, we use a mutex to protect
the use of the counter and the array. The mutex is initialized right
after the prog aux is allocated, and destroyed right before prog
aux is freed. This way we guarantee it's initialized for both cBPF
and eBPF.

Signed-off-by: YiFei Zhu <zhuyifei@google.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Andrii Nakryiko <andriin@fb.com>
Cc: YiFei Zhu <zhuyifei1999@gmail.com>
Link: https://lore.kernel.org/bpf/20200915234543.3220146-2-sdf@google.com

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index ac02369174a991aef59d03c448928ef643a4a482..53851853562c63221afca3e0f2aa8be2aa40ec48 100644 (file)
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -111,7 +111,9 @@ static int
 nfp_map_ptrs_record(struct nfp_app_bpf *bpf, struct nfp_prog *nfp_prog,
                    struct bpf_prog *prog)
 {
-       int i, cnt, err;
+       int i, cnt, err = 0;
+
+       mutex_lock(&prog->aux->used_maps_mutex);
 
        /* Quickly count the maps we will have to remember */
        cnt = 0;
@@ -119,13 +121,15 @@ nfp_map_ptrs_record(struct nfp_app_bpf *bpf, struct nfp_prog *nfp_prog,
                if (bpf_map_offload_neutral(prog->aux->used_maps[i]))
                        cnt++;
        if (!cnt)
-               return 0;
+               goto out;
 
        nfp_prog->map_records = kmalloc_array(cnt,
                                              sizeof(nfp_prog->map_records[0]),
                                              GFP_KERNEL);
-       if (!nfp_prog->map_records)
-               return -ENOMEM;
+       if (!nfp_prog->map_records) {
+               err = -ENOMEM;
+               goto out;
+       }
 
        for (i = 0; i < prog->aux->used_map_cnt; i++)
                if (bpf_map_offload_neutral(prog->aux->used_maps[i])) {
@@ -133,12 +137,14 @@ nfp_map_ptrs_record(struct nfp_app_bpf *bpf, struct nfp_prog *nfp_prog,
                                                 prog->aux->used_maps[i]);
                        if (err) {
                                nfp_map_ptrs_forget(bpf, nfp_prog);
-                               return err;
+                               goto out;
                        }
                }
        WARN_ON(cnt != nfp_prog->map_records_cnt);
 
-       return 0;
+out:
+       mutex_unlock(&prog->aux->used_maps_mutex);
+       return err;
 }
 
 static int

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index c6d9f2c444f40a5ccfe334a6f74f32b34da6c146..5dcce036463428a321aac4602878a10de89b5f4c 100644 (file)
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -751,6 +751,7 @@ struct bpf_prog_aux {
        struct bpf_ksym ksym;
        const struct bpf_prog_ops *ops;
        struct bpf_map **used_maps;
+       struct mutex used_maps_mutex; /* mutex for used_maps and used_map_cnt */
        struct bpf_prog *prog;
        struct user_struct *user;
        u64 load_time; /* ns since boottime */

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ed0b3578867c083775190cd202cb6d316d236a73..2a20c2833996a233e39f787d396394eea197b58a 100644 (file)
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -98,6 +98,7 @@ struct bpf_prog *bpf_prog_alloc_no_stats(unsigned int size, gfp_t gfp_extra_flag
        fp->jit_requested = ebpf_jit_enabled();
 
        INIT_LIST_HEAD_RCU(&fp->aux->ksym.lnode);
+       mutex_init(&fp->aux->used_maps_mutex);
 
        return fp;
 }
@@ -253,6 +254,7 @@ struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size,
 void __bpf_prog_free(struct bpf_prog *fp)
 {
        if (fp->aux) {
+               mutex_destroy(&fp->aux->used_maps_mutex);
                free_percpu(fp->aux->stats);
                kfree(fp->aux->poke_tab);
                kfree(fp->aux);
@@ -1747,8 +1749,9 @@ bool bpf_prog_array_compatible(struct bpf_array *array,
 static int bpf_check_tail_call(const struct bpf_prog *fp)
 {
        struct bpf_prog_aux *aux = fp->aux;
-       int i;
+       int i, ret = 0;
 
+       mutex_lock(&aux->used_maps_mutex);
        for (i = 0; i < aux->used_map_cnt; i++) {
                struct bpf_map *map = aux->used_maps[i];
                struct bpf_array *array;
@@ -1757,11 +1760,15 @@ static int bpf_check_tail_call(const struct bpf_prog *fp)
                        continue;
 
                array = container_of(map, struct bpf_array, map);
-               if (!bpf_prog_array_compatible(array, fp))
-                       return -EINVAL;
+               if (!bpf_prog_array_compatible(array, fp)) {
+                       ret = -EINVAL;
+                       goto out;
+               }
        }
 
-       return 0;
+out:
+       mutex_unlock(&aux->used_maps_mutex);
+       return ret;
 }
 
 static void bpf_prog_select_func(struct bpf_prog *fp)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 4108ef3b828b30d1bcb2855664475fedad592d5c..a67b8c6746bebf02aecd36fdda9a3dfe76d564a5 100644 (file)
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3162,21 +3162,25 @@ static const struct bpf_map *bpf_map_from_imm(const struct bpf_prog *prog,
        const struct bpf_map *map;
        int i;
 
+       mutex_lock(&prog->aux->used_maps_mutex);
        for (i = 0, *off = 0; i < prog->aux->used_map_cnt; i++) {
                map = prog->aux->used_maps[i];
                if (map == (void *)addr) {
                        *type = BPF_PSEUDO_MAP_FD;
-                       return map;
+                       goto out;
                }
                if (!map->ops->map_direct_value_meta)
                        continue;
                if (!map->ops->map_direct_value_meta(map, addr, off)) {
                        *type = BPF_PSEUDO_MAP_VALUE;
-                       return map;
+                       goto out;
                }
        }
+       map = NULL;
 
-       return NULL;
+out:
+       mutex_unlock(&prog->aux->used_maps_mutex);
+       return map;
 }
 
 static struct bpf_insn *bpf_insn_prepare_dump(const struct bpf_prog *prog,
@@ -3294,6 +3298,7 @@ static int bpf_prog_get_info_by_fd(struct file *file,
        memcpy(info.tag, prog->tag, sizeof(prog->tag));
        memcpy(info.name, prog->aux->name, sizeof(prog->aux->name));
 
+       mutex_lock(&prog->aux->used_maps_mutex);
        ulen = info.nr_map_ids;
        info.nr_map_ids = prog->aux->used_map_cnt;
        ulen = min_t(u32, info.nr_map_ids, ulen);
@@ -3303,9 +3308,12 @@ static int bpf_prog_get_info_by_fd(struct file *file,
 
                for (i = 0; i < ulen; i++)
                        if (put_user(prog->aux->used_maps[i]->id,
-                                    &user_map_ids[i]))
+                                    &user_map_ids[i])) {
+                               mutex_unlock(&prog->aux->used_maps_mutex);
                                return -EFAULT;
+                       }
        }
+       mutex_unlock(&prog->aux->used_maps_mutex);
 
        err = set_info_rec_size(&info);
        if (err)

diff --git a/net/core/dev.c b/net/core/dev.c
index d42c9ea0c3c03764816d02bcdccc668f61042284..75d7f91337d9f386f901ecfe7346c81a5eb715fd 100644 (file)
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5441,15 +5441,20 @@ static int generic_xdp_install(struct net_device *dev, struct netdev_bpf *xdp)
        if (new) {
                u32 i;
 
+               mutex_lock(&new->aux->used_maps_mutex);
+
                /* generic XDP does not work with DEVMAPs that can
                 * have a bpf_prog installed on an entry
                 */
                for (i = 0; i < new->aux->used_map_cnt; i++) {
-                       if (dev_map_can_have_prog(new->aux->used_maps[i]))
-                               return -EINVAL;
-                       if (cpu_map_prog_allowed(new->aux->used_maps[i]))
+                       if (dev_map_can_have_prog(new->aux->used_maps[i]) ||
+                           cpu_map_prog_allowed(new->aux->used_maps[i])) {
+                               mutex_unlock(&new->aux->used_maps_mutex);
                                return -EINVAL;
+                       }
                }
+
+               mutex_unlock(&new->aux->used_maps_mutex);
        }
 
        switch (xdp->command) {