[PATCH v2] bpf: add diagnostics for rejected memory and map accesses

[Suchit Karunakaran <suchitkarunakaran@gmail.com>]

Emit structured verifier diagnostics when BPF programs are rejected for
unsupported or prohibited memory operations. Cover sign-extending arena
loads without JIT support, read/write access restrictions on maps, writes
through read-only pointers, direct packet writes, and modifying helpers
used with BPF_F_RDONLY_PROG.

Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
---
Changes since v1:
- Modified the diagnostics messages to better explain the issues to the
  user
---
 kernel/bpf/fixups.c   |  6 ++++++
 kernel/bpf/verifier.c | 24 ++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
index e9c2d6c06218..4216b359bf28 100644
--- a/kernel/bpf/fixups.c
+++ b/kernel/bpf/fixups.c
@@ -10,6 +10,7 @@
 #include <linux/perf_event.h>
 #include <linux/sched/signal.h>
 #include <net/xdp.h>
+#include "diagnostics.h"
 #include "disasm.h"
 
 #define verbose(env, fmt, args...) bpf_verifier_log_write(env, fmt, ##args)
@@ -1055,6 +1056,11 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env)
 			if (BPF_MODE(insn->code) == BPF_MEMSX) {
 				if (!bpf_jit_supports_insn(insn, true)) {
 					verbose(env, "sign extending loads from arena are not supported yet\n");
+					bpf_diag_policy(
+						env, i + delta,
+						"sign-extending arena load",
+						"this JIT backend doesn't support it yet",
+						"Recompile the BPF program with Clang's -mcpu=v3 to avoid generating sign-extending load instructions.");
 					return -EOPNOTSUPP;
 				}
 				insn->code = BPF_CLASS(insn->code) | BPF_PROBE_MEM32SX | BPF_SIZE(insn->code);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 5c8626215ee6..bbe88c259fd0 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4472,12 +4472,22 @@ static int check_map_access_type(struct bpf_verifier_env *env, struct bpf_reg_st
 	if (type == BPF_WRITE && !(cap & BPF_MAP_CAN_WRITE)) {
 		verbose(env, "write into map forbidden, value_size=%d off=%lld size=%d\n",
 			map->value_size, reg_smin(reg) + off, size);
+		bpf_diag_policy(env, env->insn_idx,
+				bpf_diag_fmt(env, "write to map '%s'",
+					map->name[0] ? map->name : "unnamed"),
+				"this map was created with BPF_F_RDONLY_PROG, which allows BPF programs to only read it",
+				"Remove the write, or create the map without BPF_F_RDONLY_PROG if BPF programs need to write to it.");
 		return -EACCES;
 	}
 
 	if (type == BPF_READ && !(cap & BPF_MAP_CAN_READ)) {
 		verbose(env, "read from map forbidden, value_size=%d off=%lld size=%d\n",
 			map->value_size, reg_smin(reg) + off, size);
+		bpf_diag_policy(env, env->insn_idx,
+				bpf_diag_fmt(env, "read from map '%s'",
+					map->name[0] ? map->name : "unnamed"),
+				"this map was created with BPF_F_WRONLY_PROG, which allows BPF programs to only write it",
+				"Remove the read, or create the map without BPF_F_WRONLY_PROG if BPF programs need to read from it.");
 		return -EACCES;
 	}
 
@@ -6944,6 +6954,10 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b
 		if (t == BPF_WRITE && rdonly_mem) {
 			verbose(env, "%s cannot write into %s\n",
 				reg_arg_name(env, argno), reg_type_str(env, reg->type));
+			bpf_diag_policy(env, insn_idx, "memory write",
+					bpf_diag_fmt(env, "%s points to read-only memory",
+						reg_arg_name(env, argno)),
+					"Use a writable destination, or copy the data into a writable buffer before modifying it.");
 			return -EACCES;
 		}
 
@@ -7032,6 +7046,10 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b
 	} else if (reg_is_pkt_pointer(reg)) {
 		if (t == BPF_WRITE && !may_access_direct_pkt_data(env, NULL, t)) {
 			verbose(env, "cannot write into packet\n");
+			bpf_diag_policy(env, insn_idx,
+					"direct packet write",
+					"packet data is read-only through direct access for this program type",
+					"Remove the direct write, or perform the modification in a program type and hook that support packet writes.");
 			return -EACCES;
 		}
 		if (t == BPF_WRITE && value_regno >= 0 &&
@@ -11805,6 +11823,12 @@ record_func_map(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta,
 	     func_id == BPF_FUNC_map_push_elem ||
 	     func_id == BPF_FUNC_map_pop_elem)) {
 		verbose(env, "write into map forbidden\n");
+		bpf_diag_policy(env, insn_idx,
+				bpf_diag_fmt(env, "call to %s on map '%s'",
+					func_id_name(func_id),
+					map->name[0] ? map->name : "unnamed"),
+				"the map was created with BPF_F_RDONLY_PROG, which blocks modifications from BPF programs",
+				"Remove the modifying helper call, or use a map created without BPF_F_RDONLY_PROG if BPF programs need to modify it.");
 		return -EACCES;
 	}
 
-- 
2.55.0