bpf: change logging calls from verbose() to bpf_log() and use log pointer
authorToke Høiland-Jørgensen <toke@redhat.com>
Fri, 25 Sep 2020 21:25:01 +0000 (23:25 +0200)
committerAlexei Starovoitov <ast@kernel.org>
Tue, 29 Sep 2020 00:09:59 +0000 (17:09 -0700)
In preparation for moving code around, change a bunch of references to
env->log (and the verbose() logging helper) to use bpf_log() and a direct
pointer to struct bpf_verifier_log. While we're touching the function
signature, mark the 'prog' argument to bpf_check_type_match() as const.

Also enhance the bpf_verifier_log_needed() check to handle NULL pointers
for the log struct so we can re-use the code with logging disabled.

Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
include/linux/bpf.h
include/linux/bpf_verifier.h
kernel/bpf/btf.c
kernel/bpf/verifier.c

index db6dcde..5176726 100644 (file)
@@ -1420,7 +1420,7 @@ int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
                             struct bpf_reg_state *regs);
 int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog,
                          struct bpf_reg_state *reg);
-int btf_check_type_match(struct bpf_verifier_env *env, struct bpf_prog *prog,
+int btf_check_type_match(struct bpf_verifier_log *log, const struct bpf_prog *prog,
                         struct btf *btf, const struct btf_type *t);
 
 struct bpf_prog *bpf_prog_by_id(u32 id);
index 2bb48a2..7bc9276 100644 (file)
@@ -347,8 +347,9 @@ static inline bool bpf_verifier_log_full(const struct bpf_verifier_log *log)
 
 static inline bool bpf_verifier_log_needed(const struct bpf_verifier_log *log)
 {
-       return (log->level && log->ubuf && !bpf_verifier_log_full(log)) ||
-               log->level == BPF_LOG_KERNEL;
+       return log &&
+               ((log->level && log->ubuf && !bpf_verifier_log_full(log)) ||
+                log->level == BPF_LOG_KERNEL);
 }
 
 #define BPF_MAX_SUBPROGS 256
index 5d3c36e..868c03a 100644 (file)
@@ -4388,7 +4388,7 @@ static int btf_check_func_type_match(struct bpf_verifier_log *log,
 }
 
 /* Compare BTFs of given program with BTF of target program */
-int btf_check_type_match(struct bpf_verifier_env *env, struct bpf_prog *prog,
+int btf_check_type_match(struct bpf_verifier_log *log, const struct bpf_prog *prog,
                         struct btf *btf2, const struct btf_type *t2)
 {
        struct btf *btf1 = prog->aux->btf;
@@ -4396,7 +4396,7 @@ int btf_check_type_match(struct bpf_verifier_env *env, struct bpf_prog *prog,
        u32 btf_id = 0;
 
        if (!prog->aux->func_info) {
-               bpf_log(&env->log, "Program extension requires BTF\n");
+               bpf_log(log, "Program extension requires BTF\n");
                return -EINVAL;
        }
 
@@ -4408,7 +4408,7 @@ int btf_check_type_match(struct bpf_verifier_env *env, struct bpf_prog *prog,
        if (!t1 || !btf_type_is_func(t1))
                return -EFAULT;
 
-       return btf_check_func_type_match(&env->log, btf1, t1, btf2, t2);
+       return btf_check_func_type_match(log, btf1, t1, btf2, t2);
 }
 
 /* Compare BTF of a function with given bpf_reg_state.
index 52649a5..d93489c 100644 (file)
@@ -11249,6 +11249,7 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
        struct bpf_prog *prog = env->prog;
        bool prog_extension = prog->type == BPF_PROG_TYPE_EXT;
        struct bpf_prog *tgt_prog = prog->aux->linked_prog;
+       struct bpf_verifier_log *log = &env->log;
        u32 btf_id = prog->aux->attach_btf_id;
        const char prefix[] = "btf_trace_";
        struct btf_func_model fmodel;
@@ -11276,23 +11277,23 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
                return 0;
 
        if (!btf_id) {
-               verbose(env, "Tracing programs must provide btf_id\n");
+               bpf_log(log, "Tracing programs must provide btf_id\n");
                return -EINVAL;
        }
        btf = bpf_prog_get_target_btf(prog);
        if (!btf) {
-               verbose(env,
+               bpf_log(log,
                        "FENTRY/FEXIT program can only be attached to another program annotated with BTF\n");
                return -EINVAL;
        }
        t = btf_type_by_id(btf, btf_id);
        if (!t) {
-               verbose(env, "attach_btf_id %u is invalid\n", btf_id);
+               bpf_log(log, "attach_btf_id %u is invalid\n", btf_id);
                return -EINVAL;
        }
        tname = btf_name_by_offset(btf, t->name_off);
        if (!tname) {
-               verbose(env, "attach_btf_id %u doesn't have a name\n", btf_id);
+               bpf_log(log, "attach_btf_id %u doesn't have a name\n", btf_id);
                return -EINVAL;
        }
        if (tgt_prog) {
@@ -11304,18 +11305,18 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
                                break;
                        }
                if (subprog == -1) {
-                       verbose(env, "Subprog %s doesn't exist\n", tname);
+                       bpf_log(log, "Subprog %s doesn't exist\n", tname);
                        return -EINVAL;
                }
                conservative = aux->func_info_aux[subprog].unreliable;
                if (prog_extension) {
                        if (conservative) {
-                               verbose(env,
+                               bpf_log(log,
                                        "Cannot replace static functions\n");
                                return -EINVAL;
                        }
                        if (!prog->jit_requested) {
-                               verbose(env,
+                               bpf_log(log,
                                        "Extension programs should be JITed\n");
                                return -EINVAL;
                        }
@@ -11323,7 +11324,7 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
                        prog->expected_attach_type = tgt_prog->expected_attach_type;
                }
                if (!tgt_prog->jited) {
-                       verbose(env, "Can attach to only JITed progs\n");
+                       bpf_log(log, "Can attach to only JITed progs\n");
                        return -EINVAL;
                }
                if (tgt_prog->type == prog->type) {
@@ -11331,7 +11332,7 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
                         * Cannot attach program extension to another extension.
                         * It's ok to attach fentry/fexit to extension program.
                         */
-                       verbose(env, "Cannot recursively attach\n");
+                       bpf_log(log, "Cannot recursively attach\n");
                        return -EINVAL;
                }
                if (tgt_prog->type == BPF_PROG_TYPE_TRACING &&
@@ -11353,13 +11354,13 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
                         * reasonable stack size. Hence extending fentry is not
                         * allowed.
                         */
-                       verbose(env, "Cannot extend fentry/fexit\n");
+                       bpf_log(log, "Cannot extend fentry/fexit\n");
                        return -EINVAL;
                }
                key = ((u64)aux->id) << 32 | btf_id;
        } else {
                if (prog_extension) {
-                       verbose(env, "Cannot replace kernel functions\n");
+                       bpf_log(log, "Cannot replace kernel functions\n");
                        return -EINVAL;
                }
                key = btf_id;
@@ -11368,17 +11369,17 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
        switch (prog->expected_attach_type) {
        case BPF_TRACE_RAW_TP:
                if (tgt_prog) {
-                       verbose(env,
+                       bpf_log(log,
                                "Only FENTRY/FEXIT progs are attachable to another BPF prog\n");
                        return -EINVAL;
                }
                if (!btf_type_is_typedef(t)) {
-                       verbose(env, "attach_btf_id %u is not a typedef\n",
+                       bpf_log(log, "attach_btf_id %u is not a typedef\n",
                                btf_id);
                        return -EINVAL;
                }
                if (strncmp(prefix, tname, sizeof(prefix) - 1)) {
-                       verbose(env, "attach_btf_id %u points to wrong type name %s\n",
+                       bpf_log(log, "attach_btf_id %u points to wrong type name %s\n",
                                btf_id, tname);
                        return -EINVAL;
                }
@@ -11401,7 +11402,7 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
                return 0;
        case BPF_TRACE_ITER:
                if (!btf_type_is_func(t)) {
-                       verbose(env, "attach_btf_id %u is not a function\n",
+                       bpf_log(log, "attach_btf_id %u is not a function\n",
                                btf_id);
                        return -EINVAL;
                }
@@ -11412,8 +11413,7 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
                prog->aux->attach_func_proto = t;
                if (!bpf_iter_prog_supported(prog))
                        return -EINVAL;
-               ret = btf_distill_func_proto(&env->log, btf, t,
-                                            tname, &fmodel);
+               ret = btf_distill_func_proto(log, btf, t, tname, &fmodel);
                return ret;
        default:
                if (!prog_extension)
@@ -11425,18 +11425,18 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
        case BPF_TRACE_FEXIT:
                prog->aux->attach_func_name = tname;
                if (prog->type == BPF_PROG_TYPE_LSM) {
-                       ret = bpf_lsm_verify_prog(&env->log, prog);
+                       ret = bpf_lsm_verify_prog(log, prog);
                        if (ret < 0)
                                return ret;
                }
 
                if (!btf_type_is_func(t)) {
-                       verbose(env, "attach_btf_id %u is not a function\n",
+                       bpf_log(log, "attach_btf_id %u is not a function\n",
                                btf_id);
                        return -EINVAL;
                }
                if (prog_extension &&
-                   btf_check_type_match(env, prog, btf, t))
+                   btf_check_type_match(log, prog, btf, t))
                        return -EINVAL;
                t = btf_type_by_id(btf, t->type);
                if (!btf_type_is_func_proto(t))
@@ -11455,7 +11455,7 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
                        prog->aux->attach_func_proto = NULL;
                        t = NULL;
                }
-               ret = btf_distill_func_proto(&env->log, btf, t,
+               ret = btf_distill_func_proto(log, btf, t,
                                             tname, &tr->func.model);
                if (ret < 0)
                        goto out;
@@ -11467,7 +11467,7 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
                } else {
                        addr = kallsyms_lookup_name(tname);
                        if (!addr) {
-                               verbose(env,
+                               bpf_log(log,
                                        "The address of function %s cannot be found\n",
                                        tname);
                                ret = -ENOENT;
@@ -11497,17 +11497,17 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
                                break;
                        }
                        if (ret)
-                               verbose(env, "%s is not sleepable\n",
+                               bpf_log(log, "%s is not sleepable\n",
                                        prog->aux->attach_func_name);
                } else if (prog->expected_attach_type == BPF_MODIFY_RETURN) {
                        if (tgt_prog) {
-                               verbose(env, "can't modify return codes of BPF programs\n");
+                               bpf_log(log, "can't modify return codes of BPF programs\n");
                                ret = -EINVAL;
                                goto out;
                        }
                        ret = check_attach_modify_return(prog, addr);
                        if (ret)
-                               verbose(env, "%s() is not modifiable\n",
+                               bpf_log(log, "%s() is not modifiable\n",
                                        prog->aux->attach_func_name);
                }
                if (ret)