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 --- 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 #include #include +#include #include #include #include @@ -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