[PATCH bpf-next v5 09/14] bpf: Report Memory Safety bounds errors

Kumar Kartikeya Dwivedi <[email protected]>
Newsgroups org.kernel.vger.bpf
Message-ID <[email protected]>
Augment selected memory-range verifier failures with Memory Safety reports
while preserving the existing terse verifier messages for compatibility.

Cover stack spill corruption, uninitialized stack reads, variable stack helper
accesses, and check_mem_region_access() range-proof failures. The bounds report
spells out the required offset + access_size <= object_size proof with concrete
values and uses scoped diagnostic history for causal context.

Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>
---
 kernel/bpf/diagnostics.c | 78 +++++++++++++++++++++++++++++++++++++++
 kernel/bpf/diagnostics.h |  6 +++
 kernel/bpf/verifier.c    | 79 ++++++++++++++++++++++++++++++++++++++--
 3 files changed, 159 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/diagnostics.c b/kernel/bpf/diagnostics.c
index 02399cad2fb0..058574a1411e 100644
--- a/kernel/bpf/diagnostics.c
+++ b/kernel/bpf/diagnostics.c
@@ -8,6 +8,7 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/seq_buf.h>
+#include <linux/overflow.h>
 #include <linux/slab.h>
 #include <linux/stdarg.h>
 #include <linux/string.h>
@@ -16,6 +17,7 @@
 #include "diagnostics.h"
 
 #define REGISTER_TYPE_SAFETY "Register Type Safety"
+#define MEMORY_SAFETY "Memory Safety"
 
 #define BPF_DIAG_TEXT_WIDTH 100
 #define BPF_DIAG_TEXT_INDENT "  "
@@ -1164,6 +1166,18 @@ void bpf_diag_stack_arg_uninit(struct bpf_verifier_env *env, u32 insn_idx, int n
 		env, "Write the outgoing stack argument after any operation that may invalidate stored pointer values, and before making this call.");
 }
 
+void bpf_diag_memory(struct bpf_verifier_env *env, u32 insn_idx, const char *problem,
+		     const char *reason, const char *suggestion)
+{
+	bpf_diag_header(env, MEMORY_SAFETY, problem);
+	diag_reason(env, "%s", reason);
+
+	diag_section(env, "At");
+	bpf_diag_source(env, insn_idx, "error", "%s", problem);
+
+	diag_suggestion(env, "%s", suggestion);
+}
+
 void bpf_diag_record_branch(struct bpf_verifier_env *env, u32 insn_idx, bool cond_true)
 {
 	struct bpf_diag_history_event event = {
@@ -1657,6 +1671,70 @@ static const char *diag_scalar_range(struct bpf_verifier_env *env, struct cnum64
 			    diag_u64_str(env, cnum64_umax(range)));
 }
 
+const char *bpf_diag_fmt_s64_sum(struct bpf_verifier_env *env, s64 value, int addend)
+{
+	s64 sum;
+
+	if (check_add_overflow(value, (s64)addend, &sum))
+		return bpf_diag_fmt(env, "%lld plus %d (%s)", value, addend,
+				    addend < 0 ? "below S64_MIN" : "above S64_MAX");
+
+	return bpf_diag_fmt(env, "%lld", sum);
+}
+
+static const char *diag_access_offset(struct bpf_verifier_env *env, int off,
+				      const struct bpf_reg_state *reg)
+{
+	if (tnum_is_const(reg->var_off))
+		return bpf_diag_fmt(env, "constant %s",
+				    bpf_diag_fmt_s64_sum(env, (s64)reg->var_off.value, off));
+
+	if (tnum_is_unknown(reg->var_off) && diag_cnum64_unknown(reg->r64))
+		return bpf_diag_fmt(env, "unbounded");
+
+	if (off)
+		return bpf_diag_fmt(env,
+			"variable: known bits %#llx, unknown mask %#llx, plus fixed offset %d; %s",
+			(u64)reg->var_off.value, reg->var_off.mask, off,
+			diag_scalar_range(env, reg->r64));
+	return bpf_diag_fmt(env, "variable: known bits %#llx, unknown mask %#llx; %s",
+			    (u64)reg->var_off.value, reg->var_off.mask,
+			    diag_scalar_range(env, reg->r64));
+}
+
+void bpf_diag_mem_bounds(struct bpf_verifier_env *env, u32 insn_idx, int regno,
+			 const char *reg_name, const char *type_name, const char *proof,
+			 int off, int size, u32 mem_size, const struct bpf_reg_state *reg)
+{
+	const struct bpf_func_state *frame = diag_current_frame(env);
+	struct bpf_diag_history_opts opts = {
+		.scope = BPF_DIAG_HISTORY_SCOPE_REG,
+		.frame_id = frame->diag_frame_id,
+		.frameno = frame->frameno,
+		.regno = regno,
+	};
+	const char *offset_desc;
+
+	if (!bpf_diag_enabled(env))
+		return;
+
+	offset_desc = diag_access_offset(env, off, reg);
+
+	bpf_diag_header(env, MEMORY_SAFETY, "access outside bounds");
+	diag_reason(
+		env, "The verifier cannot prove offset + access_size <= object_size. Here, %s. %s is %s; offset is %s; access_size is %d; object_size is %u.",
+		proof, reg_name, type_name, offset_desc, size, mem_size);
+
+	diag_section(env, "At");
+	bpf_diag_source(env, insn_idx, "error", "access may be outside object bounds");
+
+	if (regno >= 0)
+		diag_print_history(env, &opts);
+
+	diag_suggestion(
+		env, "Add or adjust a bounds check that proves offset + access_size stays within the object.");
+}
+
 static const char *diag_var_offset(struct bpf_verifier_env *env,
 				   const struct bpf_diag_reg_snapshot *snapshot)
 {
diff --git a/kernel/bpf/diagnostics.h b/kernel/bpf/diagnostics.h
index d2355c46dad1..b5feda71de3e 100644
--- a/kernel/bpf/diagnostics.h
+++ b/kernel/bpf/diagnostics.h
@@ -15,6 +15,7 @@ struct bpf_verifier_env;
 struct bpf_verifier_state;
 struct btf;
 
+const char *bpf_diag_fmt_s64_sum(struct bpf_verifier_env *env, s64 value, int addend);
 enum bpf_diag_mod_reason {
 	BPF_DIAG_MOD_WRITE,
 	BPF_DIAG_MOD_SPILL,
@@ -62,6 +63,11 @@ void bpf_diag_unreadable_reg(struct bpf_verifier_env *env, u32 insn_idx, int reg
 void bpf_diag_stack_arg_uninit(struct bpf_verifier_env *env, u32 insn_idx, int nargs,
 			       int stack_arg_slot, const char *callee_name,
 			       const char *arg_name);
+void bpf_diag_memory(struct bpf_verifier_env *env, u32 insn_idx, const char *problem,
+		     const char *reason, const char *suggestion);
+void bpf_diag_mem_bounds(struct bpf_verifier_env *env, u32 insn_idx, int regno,
+			 const char *reg_name, const char *type_name, const char *proof,
+			 int off, int size, u32 mem_size, const struct bpf_reg_state *reg);
 void bpf_diag_record_branch(struct bpf_verifier_env *env, u32 insn_idx, bool cond_true);
 void bpf_diag_mod_begin(struct bpf_verifier_env *env, const struct bpf_reg_state *reg,
 			const struct bpf_reg_state *origin, enum bpf_diag_mod_reason reason);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 962eb7b37e6b..cfc14167cad1 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3470,7 +3470,16 @@ static int check_stack_write_fixed_off(struct bpf_verifier_env *env,
 	    bpf_is_spilled_reg(&state->stack[spi]) &&
 	    !bpf_is_spilled_scalar_reg(&state->stack[spi]) &&
 	    size != BPF_REG_SIZE) {
+		const char *reason;
+
 		verbose(env, "attempt to corrupt spilled pointer on stack\n");
+		reason = bpf_diag_fmt(env,
+				      "This store writes %d bytes at stack offset %d into a stack slot that currently holds a spilled pointer. "
+			"Partial writes to spilled pointers are rejected because they can corrupt pointer metadata and leak kernel pointers.",
+			size, off);
+		bpf_diag_memory(
+			env, insn_idx, "stack spill corruption", reason,
+			"Write the full 8-byte spilled pointer slot, or use a separate stack slot for scalar data before overwriting only part of it.");
 		return -EACCES;
 	}
 
@@ -3762,6 +3771,21 @@ static int mark_reg_stack_read(struct bpf_verifier_env *env,
 	return 0;
 }
 
+static void bpf_diag_stack_read_uninit(struct bpf_verifier_env *env, int off, int i,
+				       int size)
+{
+	const char *reason;
+
+	reason = bpf_diag_fmt(env,
+			      "This rejected read uses %d bytes at stack offset %d, but byte %d in that range is uninitialized on this path. "
+		"Programs loaded with CAP_PERFMON can be allowed to read uninitialized stack bytes, but this program is being rejected without that allowance.",
+		size, off, i);
+	bpf_diag_memory(
+		env, env->insn_idx, "uninitialized stack read", reason,
+		"Initialize every byte in the stack range before reading it, adjust the offset and size so the read covers only initialized bytes, "
+		"or load with CAP_PERFMON if uninitialized stack reads are intended.");
+}
+
 /* Read the stack at 'off' and put the results into the register indicated by
  * 'dst_regno'. It handles reg filling if the addressed stack slot is a
  * spilled reg.
@@ -3851,6 +3875,8 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
 					} else {
 						verbose(env, "invalid read from stack off %d+%d size %d\n",
 							off, i, size);
+						bpf_diag_stack_read_uninit(env, off, i,
+									   size);
 					}
 					return -EACCES;
 				}
@@ -3909,6 +3935,7 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
 			} else {
 				verbose(env, "invalid read from stack off %d+%d size %d\n",
 					off, i, size);
+				bpf_diag_stack_read_uninit(env, off, i, size);
 			}
 			return -EACCES;
 		}
@@ -4001,11 +4028,19 @@ static int check_stack_read(struct bpf_verifier_env *env,
 	 * check_stack_read_fixed_off).
 	 */
 	if (dst_regno < 0 && var_off) {
+		const char *reason;
 		char tn_buf[48];
 
 		tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
 		verbose(env, "variable offset stack pointer cannot be passed into helper function; var_off=%s off=%d size=%d\n",
 			tn_buf, off, size);
+		reason = bpf_diag_fmt(env,
+				      "The helper would access the stack through variable offset %s plus fixed offset %d and size %d. "
+			"Helper stack memory arguments require a constant stack offset and a precise initialized range.",
+			tn_buf, off, size);
+		bpf_diag_memory(
+			env, env->insn_idx, "variable stack access", reason,
+			"Use a fixed stack offset for helper memory arguments, or copy the needed bytes into a fixed stack slot first.");
 		return -EACCES;
 	}
 	/* Variable offset is prohibited for unprivileged mode for simplicity
@@ -4247,6 +4282,9 @@ static int check_mem_region_access(struct bpf_verifier_env *env, struct bpf_reg_
 				   int off, int size, u32 mem_size,
 				   bool zero_size_allowed)
 {
+	const char *proof = "";
+	const char *start;
+	s64 max_start, max_end;
 	int err;
 
 	/* We may have adjusted the register pointing to memory region, so we
@@ -4265,14 +4303,28 @@ static int check_mem_region_access(struct bpf_verifier_env *env, struct bpf_reg_
 	      reg_smin(reg) + off < 0)) {
 		verbose(env, "%s min value is negative, either use unsigned index or do a if (index >=0) check.\n",
 			reg_arg_name(env, argno));
-		return -EACCES;
+		err = -EACCES;
+		if (bpf_diag_enabled(env)) {
+			start = bpf_diag_fmt_s64_sum(env, reg_smin(reg), off);
+			proof = bpf_diag_fmt(
+				env, "the minimal bound for a memory access is a negative value: %s",
+				start);
+		}
+		goto report_error;
 	}
+
 	err = __check_mem_access(env, reg, argno, reg_smin(reg) + off, size,
 				 mem_size, zero_size_allowed);
 	if (err) {
 		verbose(env, "%s min value is outside of the allowed memory range\n",
 			reg_arg_name(env, argno));
-		return err;
+		if (bpf_diag_enabled(env)) {
+			start = bpf_diag_fmt_s64_sum(env, reg_smin(reg), off);
+			proof = bpf_diag_fmt(
+				env, "the minimal bound for a memory access is %s and is outside of the object of size %u",
+				start, mem_size);
+		}
+		goto report_error;
 	}
 
 	/* If we haven't set a max value then we need to bail since we can't be
@@ -4282,17 +4334,36 @@ static int check_mem_region_access(struct bpf_verifier_env *env, struct bpf_reg_
 	if (reg_umax(reg) >= BPF_MAX_VAR_OFF) {
 		verbose(env, "%s unbounded memory access, make sure to bounds check any such access\n",
 			reg_arg_name(env, argno));
-		return -EACCES;
+		err = -EACCES;
+		if (bpf_diag_enabled(env))
+			proof = bpf_diag_fmt(
+				env, "the maximal bound for a memory access is %llu and exceeds maximum allowed offset of %u",
+				reg_umax(reg), BPF_MAX_VAR_OFF);
+		goto report_error;
 	}
+
 	err = __check_mem_access(env, reg, argno, reg_umax(reg) + off, size,
 				 mem_size, zero_size_allowed);
 	if (err) {
 		verbose(env, "%s max value is outside of the allowed memory range\n",
 			reg_arg_name(env, argno));
-		return err;
+		if (bpf_diag_enabled(env)) {
+			max_start = (s64)reg_umax(reg) + off;
+			max_end = max_start + size;
+			proof = bpf_diag_fmt(
+				env, "the maximal bound for a memory access is %lld: start %lld + access_size %d, beyond object_size %u",
+				max_end, max_start, size, mem_size);
+		}
+		goto report_error;
 	}
 
 	return 0;
+
+report_error:
+	bpf_diag_mem_bounds(env, env->insn_idx, reg_from_argno(argno),
+			    reg_arg_name(env, argno), reg_type_str(env, reg->type), proof,
+				   off, size, mem_size, reg);
+	return err;
 }
 
 static int __check_ptr_off_reg(struct bpf_verifier_env *env,
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.