bpf: Fix race in btf_resolve_helper_id()

btf_resolve_helper_id() caching logic is a bit racy, since under root the
verifier can verify several programs in parallel. Fix it with READ/WRITE_ONCE.
Fix the type as well, since error is also recorded.

Fixes: a7658e1a4164 ("bpf: Check types of arguments passed into helpers")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Song Liu <songliubraving@fb.com>
Acked-by: Andrii Nakryiko <andriin@fb.com>
Link: https://lore.kernel.org/bpf/20191114185720.1641606-15-ast@kernel.org

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 0d4c5c224d7987ee679eb3af68b3609893d1d25f..cb5a356381f5228fe4c5896e44ac034d22e3f853 100644 (file)
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -248,7 +248,7 @@ struct bpf_func_proto {
                };
                enum bpf_arg_type arg_type[5];
        };
-       u32 *btf_id; /* BTF ids of arguments */
+       int *btf_id; /* BTF ids of arguments */
 };
 
 /* bpf_context is intentionally undefined structure. Pointer to bpf_context is
@@ -881,7 +881,8 @@ int btf_struct_access(struct bpf_verifier_log *log,
                      const struct btf_type *t, int off, int size,
                      enum bpf_access_type atype,
                      u32 *next_btf_id);
-u32 btf_resolve_helper_id(struct bpf_verifier_log *log, void *, int);
+int btf_resolve_helper_id(struct bpf_verifier_log *log,
+                         const struct bpf_func_proto *fn, int);
 
 int btf_distill_func_proto(struct bpf_verifier_log *log,
                           struct btf *btf,

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 9e1164e5b429f3c1620a654d84fcdb28354d85c3..033d071eb59c52085c2c9aa7f584a62117f8602c 100644 (file)
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -3721,7 +3721,8 @@ again:
        return -EINVAL;
 }
 
-u32 btf_resolve_helper_id(struct bpf_verifier_log *log, void *fn, int arg)
+static int __btf_resolve_helper_id(struct bpf_verifier_log *log, void *fn,
+                                  int arg)
 {
        char fnname[KSYM_SYMBOL_LEN + 4] = "btf_";
        const struct btf_param *args;
@@ -3789,6 +3790,29 @@ u32 btf_resolve_helper_id(struct bpf_verifier_log *log, void *fn, int arg)
        return btf_id;
 }
 
+int btf_resolve_helper_id(struct bpf_verifier_log *log,
+                         const struct bpf_func_proto *fn, int arg)
+{
+       int *btf_id = &fn->btf_id[arg];
+       int ret;
+
+       if (fn->arg_type[arg] != ARG_PTR_TO_BTF_ID)
+               return -EINVAL;
+
+       ret = READ_ONCE(*btf_id);
+       if (ret)
+               return ret;
+       /* ok to race the search. The result is the same */
+       ret = __btf_resolve_helper_id(log, fn->func, arg);
+       if (!ret) {
+               /* Function argument cannot be type 'void' */
+               bpf_log(log, "BTF resolution bug\n");
+               return -EFAULT;
+       }
+       WRITE_ONCE(*btf_id, ret);
+       return ret;
+}
+
 static int __get_type_size(struct btf *btf, u32 btf_id,
                           const struct btf_type **bad_type)
 {

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 8f89cfa93e88672414d2fdb5dc0ed47ce6e86d51..e78ec7990767e85b180609f3575080b32beaab5b 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4147,11 +4147,9 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
        meta.func_id = func_id;
        /* check args */
        for (i = 0; i < 5; i++) {
-               if (fn->arg_type[i] == ARG_PTR_TO_BTF_ID) {
-                       if (!fn->btf_id[i])
-                               fn->btf_id[i] = btf_resolve_helper_id(&env->log, fn->func, i);
-                       meta.btf_id = fn->btf_id[i];
-               }
+               err = btf_resolve_helper_id(&env->log, fn, i);
+               if (err > 0)
+                       meta.btf_id = err;
                err = check_func_arg(env, BPF_REG_1 + i, fn->arg_type[i], &meta);
                if (err)
                        return err;

diff --git a/net/core/filter.c b/net/core/filter.c
index fc303abec8fa9c79d18f4a758edcb01069612b9c..f72face906594e0e7285cad5601e083590bce30c 100644 (file)
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3816,7 +3816,7 @@ static const struct bpf_func_proto bpf_skb_event_output_proto = {
        .arg5_type      = ARG_CONST_SIZE_OR_ZERO,
 };
 
-static u32 bpf_skb_output_btf_ids[5];
+static int bpf_skb_output_btf_ids[5];
 const struct bpf_func_proto bpf_skb_output_proto = {
        .func           = bpf_skb_event_output,
        .gpl_only       = true,