bpf: verbose log bpf_line_info in verifier

This patch adds bpf_line_info during the verifier's verbose.
It can give error context for debug purpose.

~~~~~~~~~~
Here is the verbose log for backedge:
	while (a) {
		a += bpf_get_smp_processor_id();
		bpf_trace_printk(fmt, sizeof(fmt), a);
	}

~> bpftool prog load ./test_loop.o /sys/fs/bpf/test_loop type tracepoint
13: while (a) {
3: a += bpf_get_smp_processor_id();
back-edge from insn 13 to 3

~~~~~~~~~~
Here is the verbose log for invalid pkt access:
Modification to test_xdp_noinline.c:

	data = (void *)(long)xdp->data;
	data_end = (void *)(long)xdp->data_end;
/*
	if (data + 4 > data_end)
		return XDP_DROP;
*/
	*(u32 *)data = dst->dst;

~> bpftool prog load ./test_xdp_noinline.o /sys/fs/bpf/test_xdp_noinline type xdp
; data = (void *)(long)xdp->data;
224: (79) r2 = *(u64 *)(r10 -112)
225: (61) r2 = *(u32 *)(r2 +0)
; *(u32 *)data = dst->dst;
226: (63) *(u32 *)(r2 +0) = r1
invalid access to packet, off=0 size=4, R2(id=0,off=0,r=0)
R2 offset is outside of the packet

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index c736945be7c5ed30462b58fb1d9b570ff9a6772f..548dcbdb711145006b24f49494687abb1c1927fc 100644 (file)
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -224,6 +224,7 @@ struct bpf_verifier_env {
        bool allow_ptr_leaks;
        bool seen_direct_write;
        struct bpf_insn_aux_data *insn_aux_data; /* array of per-insn state */
+       const struct bpf_line_info *prev_linfo;
        struct bpf_verifier_log log;
        struct bpf_subprog_info subprog_info[BPF_MAX_SUBPROGS + 1];
        u32 subprog_cnt;

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 89ce2613fdb0617d349d131557664939bf4cb0cb..ba8e3134bbc214994f096a3c8141eb9a153df3a7 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -26,6 +26,7 @@
 #include <linux/bsearch.h>
 #include <linux/sort.h>
 #include <linux/perf_event.h>
+#include <linux/ctype.h>
 
 #include "disasm.h"
 
@@ -216,6 +217,27 @@ struct bpf_call_arg_meta {
 
 static DEFINE_MUTEX(bpf_verifier_lock);
 
+static const struct bpf_line_info *
+find_linfo(const struct bpf_verifier_env *env, u32 insn_off)
+{
+       const struct bpf_line_info *linfo;
+       const struct bpf_prog *prog;
+       u32 i, nr_linfo;
+
+       prog = env->prog;
+       nr_linfo = prog->aux->nr_linfo;
+
+       if (!nr_linfo || insn_off >= prog->len)
+               return NULL;
+
+       linfo = prog->aux->linfo;
+       for (i = 1; i < nr_linfo; i++)
+               if (insn_off < linfo[i].insn_off)
+                       break;
+
+       return &linfo[i - 1];
+}
+
 void bpf_verifier_vlog(struct bpf_verifier_log *log, const char *fmt,
                       va_list args)
 {
@@ -266,6 +288,42 @@ __printf(2, 3) static void verbose(void *private_data, const char *fmt, ...)
        va_end(args);
 }
 
+static const char *ltrim(const char *s)
+{
+       while (isspace(*s))
+               s++;
+
+       return s;
+}
+
+__printf(3, 4) static void verbose_linfo(struct bpf_verifier_env *env,
+                                        u32 insn_off,
+                                        const char *prefix_fmt, ...)
+{
+       const struct bpf_line_info *linfo;
+
+       if (!bpf_verifier_log_needed(&env->log))
+               return;
+
+       linfo = find_linfo(env, insn_off);
+       if (!linfo || linfo == env->prev_linfo)
+               return;
+
+       if (prefix_fmt) {
+               va_list args;
+
+               va_start(args, prefix_fmt);
+               bpf_verifier_vlog(&env->log, prefix_fmt, args);
+               va_end(args);
+       }
+
+       verbose(env, "%s\n",
+               ltrim(btf_name_by_offset(env->prog->aux->btf,
+                                        linfo->line_off)));
+
+       env->prev_linfo = linfo;
+}
+
 static bool type_is_pkt_pointer(enum bpf_reg_type type)
 {
        return type == PTR_TO_PACKET ||
@@ -4561,6 +4619,7 @@ static int push_insn(int t, int w, int e, struct bpf_verifier_env *env)
                return 0;
 
        if (w < 0 || w >= env->prog->len) {
+               verbose_linfo(env, t, "%d: ", t);
                verbose(env, "jump out of range from insn %d to %d\n", t, w);
                return -EINVAL;
        }
@@ -4578,6 +4637,8 @@ static int push_insn(int t, int w, int e, struct bpf_verifier_env *env)
                insn_stack[cur_stack++] = w;
                return 1;
        } else if ((insn_state[w] & 0xF0) == DISCOVERED) {
+               verbose_linfo(env, t, "%d: ", t);
+               verbose_linfo(env, w, "%d: ", w);
                verbose(env, "back-edge from insn %d to %d\n", t, w);
                return -EINVAL;
        } else if (insn_state[w] == EXPLORED) {
@@ -4600,10 +4661,6 @@ static int check_cfg(struct bpf_verifier_env *env)
        int ret = 0;
        int i, t;
 
-       ret = check_subprogs(env);
-       if (ret < 0)
-               return ret;
-
        insn_state = kcalloc(insn_cnt, sizeof(int), GFP_KERNEL);
        if (!insn_state)
                return -ENOMEM;
@@ -5448,6 +5505,8 @@ static int do_check(struct bpf_verifier_env *env)
        int insn_processed = 0;
        bool do_print_state = false;
 
+       env->prev_linfo = NULL;
+
        state = kzalloc(sizeof(struct bpf_verifier_state), GFP_KERNEL);
        if (!state)
                return -ENOMEM;
@@ -5521,6 +5580,7 @@ static int do_check(struct bpf_verifier_env *env)
                                .private_data   = env,
                        };
 
+                       verbose_linfo(env, insn_idx, "; ");
                        verbose(env, "%d: ", insn_idx);
                        print_bpf_insn(&cbs, insn, env->allow_ptr_leaks);
                }
@@ -6755,7 +6815,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
 
        env->allow_ptr_leaks = capable(CAP_SYS_ADMIN);
 
-       ret = check_cfg(env);
+       ret = check_subprogs(env);
        if (ret < 0)
                goto skip_full_check;
 
@@ -6763,6 +6823,10 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
        if (ret < 0)
                goto skip_full_check;
 
+       ret = check_cfg(env);
+       if (ret < 0)
+               goto skip_full_check;
+
        ret = do_check(env);
        if (env->cur_state) {
                free_verifier_state(env->cur_state, true);