Redesign Verification Errors

include/linux/bpf.h

@@ -4142,8 +4142,19 @@ static inline bool bpf_is_subprog(const struct bpf_prog *prog)
 }
 
 const struct bpf_line_info *bpf_find_linfo(const struct bpf_prog *prog, u32 insn_off);
-void bpf_get_linfo_file_line(struct btf *btf, const struct bpf_line_info *linfo,
-			     const char **filep, const char **linep, int *nump);
+#define BPF_LINFO_LINE_TRIM 1
+struct bpf_linfo_source {
+	const char *file;
+	const char *line;
+	u32 file_name_off;
+	int line_num;
+	int line_col;
+};
+
+void bpf_get_linfo_source(struct btf *btf, const struct bpf_line_info *linfo,
+			  struct bpf_linfo_source *src, u32 flags);
+void bpf_get_linfo_file_line(struct btf *btf, const struct bpf_line_info *linfo, const char **filep,
+			     const char **linep, int *nump, u32 flags);
 int bpf_prog_get_file_line(struct bpf_prog *prog, unsigned long ip, const char **filep,
 			   const char **linep, int *nump);
 struct bpf_prog *bpf_prog_find_from_stack(void);

include/linux/bpf_verifier.h

@@ -835,6 +835,7 @@ static inline u16 bpf_in_stack_arg_cnt(const struct bpf_subprog_info *sub)
 	return 0;
 }
 
+struct bpf_diag;
 struct bpf_verifier_env;
 
 struct backtrack_state {
@@ -951,6 +952,7 @@ struct bpf_verifier_env {
 	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_diag *diag;
 	struct bpf_subprog_info subprog_info[BPF_MAX_SUBPROGS + 2]; /* max + 2 for the fake and exception subprogs */
 	/* subprog indices sorted in topological order: leaves first, callers last */
 	int subprog_topo_order[BPF_MAX_SUBPROGS + 2];
@@ -1088,17 +1090,19 @@ __printf(3, 4) void verbose_linfo(struct bpf_verifier_env *env,
 				  u32 insn_off,
 				  const char *prefix_fmt, ...);
 
+__printf(2, 3) void bpf_verifier_log_bug(struct bpf_verifier_env *env, const char *fmt, ...);
+
 #define verifier_bug_if(cond, env, fmt, args...)						\
 	({											\
 		bool __cond = (cond);								\
 		if (unlikely(__cond))								\
 			verifier_bug(env, fmt " (" #cond ")", ##args);				\
 		(__cond);									\
 	})
-#define verifier_bug(env, fmt, args...)								\
-	({											\
-		BPF_WARN_ONCE(1, "verifier bug: " fmt "\n", ##args);				\
-		bpf_log(&env->log, "verifier bug: " fmt "\n", ##args);				\
+#define verifier_bug(env, fmt, args...)                              \
+	({                                                           \
+		BPF_WARN_ONCE(1, "verifier bug: " fmt "\n", ##args); \
+		bpf_verifier_log_bug(env, fmt, ##args);              \
 	})
 
 static inline void mark_prune_point(struct bpf_verifier_env *env, int idx)
@@ -1341,6 +1345,18 @@ static inline bool type_is_non_owning_ref(u32 type)
 	return type_is_ptr_alloc_obj(type) && type_flag(type) & NON_OWN_REF;
 }
 
+static inline bool type_is_map_ptr(enum bpf_reg_type type)
+{
+	switch (base_type(type)) {
+	case CONST_PTR_TO_MAP:
+	case PTR_TO_MAP_KEY:
+	case PTR_TO_MAP_VALUE:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static inline bool type_is_pkt_pointer(enum bpf_reg_type type)
 {
 	type = base_type(type);
@@ -1425,8 +1441,10 @@ void print_verifier_state(struct bpf_verifier_env *env, const struct bpf_verifie
 void print_insn_state(struct bpf_verifier_env *env, const struct bpf_verifier_state *vstate,
 		      u32 frameno);
 u32 bpf_vlog_alignment(u32 pos);
+const char *bpf_disasm_kfunc_name(void *data, const struct bpf_insn *insn);
 
 struct bpf_subprog_info *bpf_find_containing_subprog(struct bpf_verifier_env *env, int off);
+const char *bpf_verifier_subprog_name(const struct bpf_verifier_env *env, int subprog);
 int bpf_jmp_offset(struct bpf_insn *insn);
 struct bpf_iarray *bpf_insn_successors(struct bpf_verifier_env *env, u32 idx);
 void bpf_fmt_stack_mask(char *buf, ssize_t buf_sz, u64 stack_mask);

include/linux/btf.h

@@ -213,6 +213,7 @@ int btf_type_seq_show_flags(const struct btf *btf, u32 type_id, void *obj,
  */
 int btf_type_snprintf_show(const struct btf *btf, u32 type_id, void *obj,
 			   char *buf, int len, u64 flags);
+int btf_type_snprintf_show_name(const struct btf *btf, u32 type_id, char *buf, int len);
 
 int btf_get_fd_by_id(u32 id);
 u32 btf_obj_id(const struct btf *btf);

kernel/bpf/Makefile

@@ -6,7 +6,7 @@ cflags-nogcse-$(CONFIG_X86)$(CONFIG_CC_IS_GCC) := -fno-gcse
 endif
 CFLAGS_core.o += -Wno-override-init $(cflags-nogcse-yy)
 
-obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o cnum.o log.o token.o liveness.o const_fold.o
+obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o cnum.o log.o token.o liveness.o const_fold.o diagnostics.o
 obj-$(CONFIG_BPF_SYSCALL) += bpf_iter.o map_iter.o task_iter.o prog_iter.o link_iter.o
 obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o map_in_map.o bloom_filter.o
 obj-$(CONFIG_BPF_SYSCALL) += local_storage.o queue_stack_maps.o ringbuf.o bpf_insn_array.o

kernel/bpf/btf.c

@@ -8297,6 +8297,16 @@ int btf_type_snprintf_show(const struct btf *btf, u32 type_id, void *obj,
 	return ssnprintf.len;
 }
 
+int btf_type_snprintf_show_name(const struct btf *btf, u32 type_id, char *buf, int len)
+{
+	struct btf_show show = {
+		.btf = btf,
+		.state.type_id = type_id,
+	};
+
+	return snprintf(buf, len, "%s", btf_show_name(&show));
+}
+
 #ifdef CONFIG_PROC_FS
 static void bpf_btf_show_fdinfo(struct seq_file *m, struct file *filp)
 {

kernel/bpf/cfg.c

@@ -5,6 +5,8 @@
 #include <linux/filter.h>
 #include <linux/sort.h>
 
+#include "diagnostics.h"
+
 #define verbose(env, fmt, args...) bpf_verifier_log_write(env, fmt, ##args)
 
 /* non-recursive DFS pseudo code
@@ -113,6 +115,12 @@ static int push_insn(int t, int w, int e, struct bpf_verifier_env *env)
 	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);
+		bpf_diag_report_program_structure(env, t, "jump out of range",
+						  "Keep branch targets inside the program.",
+						  "Instruction %d jumps to instruction %d, but the "
+						  "program only contains instructions 0 through "
+						  "%d.",
+						  t, w, env->prog->len - 1);
 		return -EINVAL;
 	}
 
@@ -136,6 +144,15 @@ static int push_insn(int t, int w, int e, struct bpf_verifier_env *env)
 		verbose_linfo(env, t, "%d: ", t);
 		verbose_linfo(env, w, "%d: ", w);
 		verbose(env, "back-edge from insn %d to %d\n", t, w);
+		bpf_diag_report_program_structure(env, t, "back-edge is not allowed",
+						  "Load with privileges that allow this back-edge, "
+						  "or rewrite "
+						  "the control flow so it does not branch "
+						  "backward.",
+						  "Instruction %d branches back to instruction %d. "
+						  "This program is being rejected without the "
+						  "privilege needed for this back-edge.",
+						  t, w);
 		return -EINVAL;
 	} else if (insn_state[w] == EXPLORED) {
 		/* forward- or cross-edge */
@@ -316,6 +333,14 @@ static struct bpf_iarray *jt_from_subprog(struct bpf_verifier_env *env,
 
 	if (!jt) {
 		verbose(env, "no jump tables found for subprog starting at %u\n", subprog_start);
+		bpf_diag_report_program_structure(env, subprog_start, "missing jump table",
+						  "Make sure subprograms containing gotox "
+						  "instructions are "
+						  "accompanied by jump tables referencing these "
+						  "subprograms.",
+						  "No jump table was found for the subprogram that "
+						  "starts at instruction %u.",
+						  subprog_start);
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -343,6 +368,13 @@ create_jt(int t, struct bpf_verifier_env *env)
 		if (jt->items[i] < subprog_start || jt->items[i] >= subprog_end) {
 			verbose(env, "jump table for insn %d points outside of the subprog [%u,%u]\n",
 					t, subprog_start, subprog_end);
+			bpf_diag_report_program_structure(env, t, "jump table target out of range",
+							  "Keep every jump-table target inside the "
+							  "same subprogram.",
+							  "The jump table for instruction %d "
+							  "points outside subprogram range "
+							  "[%u,%u).",
+							  t, subprog_start, subprog_end);
 			kvfree(jt);
 			return ERR_PTR(-EINVAL);
 		}
@@ -374,6 +406,13 @@ static int visit_gotox_insn(int t, struct bpf_verifier_env *env)
 		w = jt->items[i];
 		if (w < 0 || w >= env->prog->len) {
 			verbose(env, "indirect jump out of range from insn %d to %d\n", t, w);
+			bpf_diag_report_program_structure(env, t, "indirect jump out of range",
+							  "Keep indirect jump targets inside the "
+							  "program.",
+							  "Instruction %d can jump indirectly to "
+							  "instruction %d, but the program only "
+							  "contains instructions 0 through %d.",
+							  t, w, env->prog->len - 1);
 			return -EINVAL;
 		}
 
@@ -624,12 +663,28 @@ int bpf_check_cfg(struct bpf_verifier_env *env)
 
 		if (insn_state[i] != EXPLORED) {
 			verbose(env, "unreachable insn %d\n", i);
+			bpf_diag_report_program_structure(env, i, "unreachable instruction",
+							  "Remove the unreachable instruction or "
+							  "add valid control flow that reaches it.",
+							  "Instruction %d is not reachable from "
+							  "the program entry point.",
+							  i);
 			ret = -EINVAL;
 			goto err_free;
 		}
 		if (bpf_is_ldimm64(insn)) {
 			if (insn_state[i + 1] != 0) {
 				verbose(env, "jump into the middle of ldimm64 insn %d\n", i);
+				bpf_diag_report_program_structure(env, i,
+								  "jump into ldimm64 immediate",
+								  "Target the first instruction of "
+								  "the ldimm64 pair, or "
+								  "restructure the jump target.",
+								  "Control flow reaches the second "
+								  "half of the ldimm64 instruction "
+								  "pair that starts at instruction "
+								  "%d.",
+								  i);
 				ret = -EINVAL;
 				goto err_free;
 			}

kernel/bpf/core.c

@@ -3460,24 +3460,30 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(xdp_bulk_tx);
 
 #ifdef CONFIG_BPF_SYSCALL
 
-void bpf_get_linfo_file_line(struct btf *btf, const struct bpf_line_info *linfo,
-			     const char **filep, const char **linep, int *nump)
+void bpf_get_linfo_source(struct btf *btf, const struct bpf_line_info *linfo,
+			  struct bpf_linfo_source *src, u32 flags)
 {
-	/* Get base component of the file path. */
-	if (filep) {
-		*filep = btf_name_by_offset(btf, linfo->file_name_off);
-		*filep = kbasename(*filep);
-	}
+	src->file = kbasename(btf_name_by_offset(btf, linfo->file_name_off));
+	src->line = btf_name_by_offset(btf, linfo->line_off);
+	while ((flags & BPF_LINFO_LINE_TRIM) && isspace(*src->line))
+		src->line++;
+	src->file_name_off = linfo->file_name_off;
+	src->line_num = BPF_LINE_INFO_LINE_NUM(linfo->line_col);
+	src->line_col = BPF_LINE_INFO_LINE_COL(linfo->line_col);
+}
 
-	/* Obtain the source line, and strip whitespace in prefix. */
-	if (linep) {
-		*linep = btf_name_by_offset(btf, linfo->line_off);
-		while (isspace(**linep))
-			*linep += 1;
-	}
+void bpf_get_linfo_file_line(struct btf *btf, const struct bpf_line_info *linfo, const char **filep,
+			     const char **linep, int *nump, u32 flags)
+{
+	struct bpf_linfo_source src;
 
+	bpf_get_linfo_source(btf, linfo, &src, flags);
+	if (filep)
+		*filep = src.file;
+	if (linep)
+		*linep = src.line;
 	if (nump)
-		*nump = BPF_LINE_INFO_LINE_NUM(linfo->line_col);
+		*nump = src.line_num;
 }
 
 const struct bpf_line_info *bpf_find_linfo(const struct bpf_prog *prog, u32 insn_off)
@@ -3551,7 +3557,7 @@ int bpf_prog_get_file_line(struct bpf_prog *prog, unsigned long ip, const char *
 	if (idx == -1)
 		return -ENOENT;
 
-	bpf_get_linfo_file_line(btf, &linfo[idx], filep, linep, nump);
+	bpf_get_linfo_file_line(btf, &linfo[idx], filep, linep, nump, BPF_LINFO_LINE_TRIM);
 	return 0;
 }