Re: [PATCH bpf-next v2 09/17] bpf: Report Memory Safety bounds errors

["Alexei Starovoitov" <alexei.starovoitov@gmail.com>]

On Fri Jun 19, 2026 at 1:59 PM PDT, Kumar Kartikeya Dwivedi wrote:
> +
> +static void bpf_diag_format_access_offset(struct bpf_verifier_env *env,
> +					  char *buf, size_t size, int off,
> +					  const struct bpf_reg_state *reg)
> +{
> +	struct bpf_diag_scratch *scratch = bpf_diag_scratch(env);
> +	struct bpf_diag_reg_fmt *fmt;
> +	char *start;
> +
> +	if (tnum_is_const(reg->var_off)) {
> +		start = bpf_diag_scratch_buf(env, 2,
> +					     NULL);

unnecessary line wrap.

> +		if (!start) {
> +			scnprintf(buf, size, "constant");
> +			return;
> +		}
> +		bpf_diag_format_s64_sum(start, BPF_DIAG_SCRATCH_STR_LEN,
> +					(s64)reg->var_off.value, off);
> +		scnprintf(buf, size, "constant %s", start);
> +		return;
> +	}
> +
> +	if (tnum_is_unknown(reg->var_off) &&
> +	    bpf_diag_cnum64_unknown(reg->r64)) {

also

> +		scnprintf(buf, size, "unknown");
> +		return;
> +	}
> +
> +	fmt = &scratch->reg_fmt;
> +	memset(fmt, 0, sizeof(*fmt));
> +
> +	bpf_diag_format_scalar_range(fmt, fmt->range, sizeof(fmt->range),
> +				     reg->r64);

also doesn't need to wrap. Up to 100 chars is ok.

> +	if (off)
> +		scnprintf(buf, size,
> +			  "variable: known bits %#llx, unknown mask %#llx, plus fixed offset %d; %s",
> +			  (u64)reg->var_off.value, reg->var_off.mask, off,
> +			  fmt->range);
> +	else
> +		scnprintf(buf, size,
> +			  "variable: known bits %#llx, unknown mask %#llx; %s",
> +			  (u64)reg->var_off.value, reg->var_off.mask,
> +			  fmt->range);
> +}
> +
> +static u64 bpf_diag_mem_max_start(const struct bpf_reg_state *reg, int off)
> +{
> +	/* A negative fixed offset can clamp the maximum start to zero when
> +	 * the unsigned variable maximum is smaller than -off.
> +	 */
> +	if (off < 0 && reg_umax(reg) < (u64)-off)
> +		return 0;
> +	return reg_umax(reg) + off;
> +}
> +
> +void bpf_diag_report_mem_bounds(struct bpf_verifier_env *env, u32 insn_idx,
> +				int regno, const char *reg_name,
> +				const char *type_name,
> +				enum bpf_diag_mem_bounds_kind kind,
> +				int off, int size, u32 mem_size,
> +				const struct bpf_reg_state *reg)
> +{
> +	struct bpf_diag_history_opts opts = {
> +		.scope = BPF_DIAG_HISTORY_SCOPE_REG,
> +		.frameno = bpf_diag_current_frameno(env),
> +		.regno = regno,
> +	};
> +	char *offset_desc, *proof, *start;
> +	u64 max_start, max_end;
> +
> +	if (!bpf_diag_enabled(env))
> +		return;
> +
> +	offset_desc = bpf_diag_scratch_buf(env, 0, NULL);
> +	proof = bpf_diag_scratch_buf(env, 1, NULL);
> +	start = bpf_diag_scratch_buf(env, 2, NULL);
> +	if (!offset_desc || !proof || !start)
> +		return;

can they be NULL?

> +
> +	switch (kind) {
> +	case BPF_DIAG_MEM_NEGATIVE_MIN:
> +		bpf_diag_format_s64_sum(start, BPF_DIAG_SCRATCH_STR_LEN, reg_smin(reg),
> +					off);

no line wrap pls

> +		scnprintf(proof, BPF_DIAG_SCRATCH_STR_LEN,
> +			  "the smallest possible access starts at %s, below 0",
> +			  start);
> +		break;
> +	case BPF_DIAG_MEM_MIN_OUT_OF_RANGE:
> +		bpf_diag_format_s64_sum(start, BPF_DIAG_SCRATCH_STR_LEN, reg_smin(reg),
> +					off);

same

> +		scnprintf(proof, BPF_DIAG_SCRATCH_STR_LEN,
> +			  "the smallest possible access starts at %s, outside object_size %u",
> +			  start, mem_size);
> +		break;
> +	case BPF_DIAG_MEM_UNBOUNDED:
> +		scnprintf(proof, BPF_DIAG_SCRATCH_STR_LEN,
> +			  "%s has unsigned maximum %llu, which exceeds BPF_MAX_VAR_OFF %u",
> +			  reg_name, reg_umax(reg), BPF_MAX_VAR_OFF);
> +		break;
> +	case BPF_DIAG_MEM_MAX_OUT_OF_RANGE:
> +	default:
> +		max_start = bpf_diag_mem_max_start(reg, off);
> +		max_end = max_start + size;
> +		scnprintf(proof, BPF_DIAG_SCRATCH_STR_LEN,
> +			  "the largest possible access ends at %llu: start %llu + access_size %d, beyond object_size %u",
> +			  max_end, max_start, size, mem_size);
> +		break;
> +	}
> +
> +	bpf_diag_format_access_offset(env, offset_desc, BPF_DIAG_SCRATCH_STR_LEN,
> +				      off, reg);

same

> +
> +	bpf_diag_report_header(env, BPF_DIAG_CATEGORY_MEMORY_SAFETY,
> +			       "access outside bounds");
> +	bpf_diag_report_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);
> +
> +	bpf_diag_report_section(env, "At");
> +	bpf_diag_report_source(env, insn_idx, "error",
> +			       "access may be outside object bounds");

same

> +
> +	if (regno >= 0)
> +		bpf_diag_print_history(env, &opts);
> +
> +	bpf_diag_report_suggestion(env,
> +				   "Add or adjust a bounds check that proves offset + access_size stays within the object.");
> +}
> +
>  static void bpf_diag_format_var_offset(struct bpf_diag_reg_fmt *fmt,
>  				       char *buf, size_t size,
>  				       const struct bpf_diag_reg_snapshot *snapshot)
> diff --git a/kernel/bpf/diagnostics.h b/kernel/bpf/diagnostics.h
> index d6e858cd39f2..fea7731d431c 100644
> --- a/kernel/bpf/diagnostics.h
> +++ b/kernel/bpf/diagnostics.h
> @@ -124,6 +124,13 @@ enum bpf_diag_invalid_deref_kind {
>  	BPF_DIAG_DEREF_INVALID_PTR,
>  };
>  
> +enum bpf_diag_mem_bounds_kind {
> +	BPF_DIAG_MEM_NEGATIVE_MIN,
> +	BPF_DIAG_MEM_MIN_OUT_OF_RANGE,
> +	BPF_DIAG_MEM_UNBOUNDED,
> +	BPF_DIAG_MEM_MAX_OUT_OF_RANGE,
> +};
> +
>  struct bpf_diag_history_opts {
>  	enum bpf_diag_history_scope scope;
>  	u32 frameno;
> @@ -171,6 +178,15 @@ void bpf_diag_report_stack_arg_uninit(struct bpf_verifier_env *env,
>  				      u32 insn_idx, int nargs,
>  				      int stack_arg_slot,
>  				      const char *callee_name);
> +void bpf_diag_report_memory(struct bpf_verifier_env *env, u32 insn_idx,
> +			    const char *problem, const char *reason,
> +			    const char *suggestion);
> +void bpf_diag_report_mem_bounds(struct bpf_verifier_env *env, u32 insn_idx,
> +				int regno, const char *reg_name,
> +				const char *type_name,
> +				enum bpf_diag_mem_bounds_kind kind,
> +				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_record_reg_mod(struct bpf_verifier_env *env, u32 insn_idx,
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 2c5f24528071..af04709c5178 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3527,6 +3527,13 @@ static int check_stack_write_fixed_off(struct bpf_verifier_env *env,
>  	    !bpf_is_spilled_scalar_reg(&state->stack[spi]) &&
>  	    size != BPF_REG_SIZE) {
>  		verbose(env, "attempt to corrupt spilled pointer on stack\n");
> +		bpf_diag_report_memory(env, insn_idx,
> +				       "stack spill corruption",
> +				       bpf_diag_scratch_printf(env,
> +							       0,
> +							       "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),
> +				       "Write the full 8-byte spilled pointer slot, or use a separate stack slot for scalar data before overwriting only part of it.");

but these are way too long.
introduce const chart *fmt = "This store.."
at the top of the function?

>  		return -EACCES;
>  	}
>  
> @@ -3811,6 +3818,18 @@ static void mark_reg_stack_read(struct bpf_verifier_env *env,
>  	}
>  }
>  
> +static void bpf_diag_report_stack_read_uninit(struct bpf_verifier_env *env,
> +					      int off, int i, int size)
> +{
> +	bpf_diag_report_memory(env, env->insn_idx,
> +			       "uninitialized stack read",
> +			       bpf_diag_scratch_printf(env,
> +						       0,
> +						       "This rejected read uses %d bytes at stack offset %d, but byte %d in that range is uninitialized on this path. Programs with sufficient privilege can be allowed to read uninitialized stack bytes, but this program is being rejected without that allowance.",
> +						       size, off, i),
> +			       "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 the privilege needed for uninitialized stack reads.");

also way too long.

> +}
> +
>  /* 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.
> @@ -3896,9 +3915,12 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
>  					if (type == STACK_POISON) {
>  						verbose(env, "reading from stack off %d+%d size %d, slot poisoned by dead code elimination\n",
>  							off, i, size);
> +						return -EFAULT;
>  					} else {
>  						verbose(env, "invalid read from stack off %d+%d size %d\n",
>  							off, i, size);
> +						bpf_diag_report_stack_read_uninit(env, off, i,
> +										  size);
>  					}
>  					return -EACCES;
>  				}
> @@ -3951,9 +3973,12 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
>  			if (type == STACK_POISON) {
>  				verbose(env, "reading from stack off %d+%d size %d, slot poisoned by dead code elimination\n",
>  					off, i, size);
> +				return -EFAULT;
>  			} else {
>  				verbose(env, "invalid read from stack off %d+%d size %d\n",
>  					off, i, size);
> +				bpf_diag_report_stack_read_uninit(env, off, i,
> +								  size);
>  			}
>  			return -EACCES;
>  		}
> @@ -4045,6 +4070,13 @@ static int check_stack_read(struct bpf_verifier_env *env,
>  		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);
> +		bpf_diag_report_memory(env, env->insn_idx,
> +				       "variable stack access",
> +				       bpf_diag_scratch_printf(env,
> +							       0,
> +							       "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),
> +				       "Use a fixed stack offset for helper memory arguments, or copy the needed bytes into a fixed stack slot first.");

and here too. Pls find a way to reduce indent of bpf_diag_report_memory( ...bpf_diag_scratch_printf
I'd think bpf_diag_scratch_printf() can be done first and result passed into bpf_diag_report_memory()