diff --git a/kernel/bpf/diagnostics.c b/kernel/bpf/diagnostics.c index 99784d465881..c69160f656e9 100644 --- a/kernel/bpf/diagnostics.c +++ b/kernel/bpf/diagnostics.c @@ -20,6 +20,7 @@ #define MEMORY_SAFETY "Memory Safety" #define RESOURCE_LIFETIME_SAFETY "Resource Lifetime Safety" #define CALL_TYPE_SAFETY "Call Type Safety" +#define EXECUTION_CONTEXT_SAFETY "Execution Context Safety" #define BPF_DIAG_TEXT_WIDTH 100 #define BPF_DIAG_TEXT_INDENT " " @@ -163,6 +164,7 @@ static void diag_print_history(struct bpf_verifier_env *env, const struct bpf_diag_history_opts *opts); static bool diag_target_matches(const struct bpf_diag_mod_target *event_target, const struct bpf_diag_mod_target *target); +static const char *diag_context_name(enum bpf_diag_context_kind kind); struct disasm_line { char text[DISASM_LINE_LEN]; int idx; @@ -1029,6 +1031,167 @@ void bpf_diag_call_type(struct bpf_verifier_env *env, u32 insn_idx, int argno, i diag_suggestion(env, "%s", suggestion); } +static const char *diag_context_constraint(enum bpf_diag_context_kind kind) +{ + switch (kind) { + case BPF_DIAG_CONTEXT_RCU: + return "RCU read-side critical sections cannot call operations that may sleep"; + case BPF_DIAG_CONTEXT_PREEMPT: + return "preemption-disabled code cannot call operations that may sleep"; + case BPF_DIAG_CONTEXT_IRQ: + return "IRQ-disabled code cannot call operations that may sleep"; + case BPF_DIAG_CONTEXT_LOCK: + return "code holding a BPF spin lock cannot call operations that may sleep"; + case BPF_DIAG_CONTEXT_NONE: + default: + return NULL; + } +} + +static const char *diag_active_context(struct bpf_verifier_env *env, u32 depth, + const char *context) +{ + if (depth == 1) + return bpf_diag_fmt(env, "an active %s (depth 1)", context); + return bpf_diag_fmt(env, "%u active %ss (depth %u)", depth, context, depth); +} + +static u32 diag_context_depth(struct bpf_verifier_env *env, enum bpf_diag_context_kind kind) +{ + switch (kind) { + case BPF_DIAG_CONTEXT_RCU: + return env->cur_state->active_rcu_locks; + case BPF_DIAG_CONTEXT_PREEMPT: + return env->cur_state->active_preempt_locks; + case BPF_DIAG_CONTEXT_IRQ: + return bpf_diag_irq_depth(env->cur_state); + case BPF_DIAG_CONTEXT_LOCK: + return env->cur_state->active_locks; + case BPF_DIAG_CONTEXT_NONE: + default: + return 0; + } +} + +void bpf_diag_ctx_forbidden(struct bpf_verifier_env *env, u32 insn_idx, + const char *operation, const char *suggestion) +{ + struct bpf_diag_history_opts opts; + enum bpf_diag_context_kind ctx_kind; + const char *constraint, *context; + u32 depth; + + if (env->cur_state->active_rcu_locks) + ctx_kind = BPF_DIAG_CONTEXT_RCU; + else if (env->cur_state->active_preempt_locks) + ctx_kind = BPF_DIAG_CONTEXT_PREEMPT; + else if (env->cur_state->active_irq_id) + ctx_kind = BPF_DIAG_CONTEXT_IRQ; + else if (env->cur_state->active_locks) + ctx_kind = BPF_DIAG_CONTEXT_LOCK; + else + ctx_kind = BPF_DIAG_CONTEXT_NONE; + + depth = diag_context_depth(env, ctx_kind); + opts = (struct bpf_diag_history_opts) { + .scope = BPF_DIAG_HISTORY_SCOPE_CONTEXT, + .ctx_kind = ctx_kind, + .ctx_depth = depth, + }; + constraint = diag_context_constraint(ctx_kind); + context = diag_context_name(ctx_kind); + + bpf_diag_header(env, EXECUTION_CONTEXT_SAFETY, + "operation is not allowed in this context"); + if (constraint) { + if (depth) { + diag_reason( + env, "The operation %s cannot be used in %s because %s. This path is still inside %s.", + operation, context, constraint, diag_active_context(env, depth, context)); + } else { + diag_reason(env, "The operation %s cannot be used in %s because %s.", + operation, context, constraint); + } + } else { + diag_reason(env, "The operation %s cannot be used in %s.", operation, + context); + } + + diag_section(env, "At"); + bpf_diag_source(env, insn_idx, "error", "%s is not allowed in %s", operation, + context); + + if (ctx_kind != BPF_DIAG_CONTEXT_NONE) + diag_print_history(env, &opts); + + diag_suggestion(env, "%s", suggestion); +} + +void bpf_diag_ctx_active(struct bpf_verifier_env *env, u32 insn_idx, const char *operation, + enum bpf_diag_context_kind ctx_kind, const char *suggestion) +{ + u32 depth = diag_context_depth(env, ctx_kind); + struct bpf_diag_history_opts opts = { + .scope = BPF_DIAG_HISTORY_SCOPE_CONTEXT, + .ctx_kind = ctx_kind, + .ctx_depth = depth, + }; + const char *context = diag_context_name(ctx_kind); + + bpf_diag_header(env, EXECUTION_CONTEXT_SAFETY, + "operation is not allowed in this context"); + diag_reason( + env, "The operation %s cannot be used while this path is still inside %s. Leave the region before this operation.", + operation, diag_active_context(env, depth, context)); + + diag_section(env, "At"); + bpf_diag_source(env, insn_idx, "error", "%s is not allowed before leaving %s", + operation, context); + + diag_print_history(env, &opts); + + diag_suggestion(env, "%s", suggestion); +} + +void bpf_diag_ctx_required(struct bpf_verifier_env *env, u32 insn_idx, const char *operation, + enum bpf_diag_context_kind ctx_kind, const char *suggestion) +{ + const char *context = diag_context_name(ctx_kind); + + bpf_diag_header(env, EXECUTION_CONTEXT_SAFETY, "required context is not active"); + diag_reason(env, "The operation %s requires an active %s, but this path is outside one.", + operation, context); + + diag_section(env, "At"); + bpf_diag_source(env, insn_idx, "error", "%s requires %s", operation, context); + + diag_suggestion(env, "%s", suggestion); +} + +void bpf_diag_ctx_underflow(struct bpf_verifier_env *env, u32 insn_idx, + const char *operation, enum bpf_diag_context_kind ctx_kind, + const char *suggestion) +{ + struct bpf_diag_history_opts opts = { + .scope = BPF_DIAG_HISTORY_SCOPE_CONTEXT, + .ctx_kind = ctx_kind, + }; + const char *context = diag_context_name(ctx_kind); + + bpf_diag_header(env, EXECUTION_CONTEXT_SAFETY, "unmatched context exit"); + diag_reason( + env, "The operation %s tries to leave %s, but this path has no active %s to leave. The current depth is 0.", + operation, context, context); + + diag_section(env, "At"); + bpf_diag_source(env, insn_idx, "error", "%s has no matching enter on this path", + operation); + + diag_print_history(env, &opts); + + diag_suggestion(env, "%s", suggestion); +} + void bpf_diag_invalid_deref(struct bpf_verifier_env *env, u32 insn_idx, int regno, const char *reg_name, const struct bpf_reg_state *reg, enum bpf_diag_invalid_deref_kind kind, s64 offset) @@ -2057,7 +2220,7 @@ static const char *diag_context_name(enum bpf_diag_context_kind kind) return "lock region"; case BPF_DIAG_CONTEXT_NONE: default: - return "context"; + return "non-sleepable program"; } } diff --git a/kernel/bpf/diagnostics.h b/kernel/bpf/diagnostics.h index 4b85a7ad2019..95bc654e5b3e 100644 --- a/kernel/bpf/diagnostics.h +++ b/kernel/bpf/diagnostics.h @@ -80,6 +80,15 @@ void bpf_diag_leak(struct bpf_verifier_env *env, u32 ref_id, u32 alloc_insn, u32 void bpf_diag_call_type(struct bpf_verifier_env *env, u32 insn_idx, int argno, int regno, int stack_arg_slot, const char *call_name, const char *arg_name, const char *reason, const char *suggestion); +void bpf_diag_ctx_forbidden(struct bpf_verifier_env *env, u32 insn_idx, + const char *operation, const char *suggestion); +void bpf_diag_ctx_active(struct bpf_verifier_env *env, u32 insn_idx, const char *operation, + enum bpf_diag_context_kind ctx_kind, const char *suggestion); +void bpf_diag_ctx_required(struct bpf_verifier_env *env, u32 insn_idx, const char *operation, + enum bpf_diag_context_kind ctx_kind, const char *suggestion); +void bpf_diag_ctx_underflow(struct bpf_verifier_env *env, u32 insn_idx, + const char *operation, enum bpf_diag_context_kind ctx_kind, + const char *suggestion); 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 30e48d4b02f9..0f126f090a47 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -7739,6 +7739,9 @@ static int process_iter_arg(struct bpf_verifier_env *env, struct bpf_reg_state * return err; case -EPROTO: verbose(env, "expected an RCU CS when using %s\n", meta->func_name); + bpf_diag_ctx_required( + env, insn_idx, meta->func_name, BPF_DIAG_CONTEXT_RCU, + "Wrap iterator use in bpf_rcu_read_lock() and bpf_rcu_read_unlock(), keeping all exit paths balanced."); return err; default: return err; @@ -9838,17 +9841,24 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn, return err; if (bpf_subprog_is_global(env, subprog)) { const char *sub_name = bpf_subprog_name(env, subprog); + const char *operation; bool returns_void; if (env->cur_state->active_locks) { verbose(env, "global function calls are not allowed while holding a lock,\n" "use static function instead\n"); + operation = bpf_diag_fmt(env, "global function %s()", sub_name); + bpf_diag_ctx_active(env, *insn_idx, operation, BPF_DIAG_CONTEXT_LOCK, + "Release the lock before calling the global function, or use a static function instead."); return -EINVAL; } if (env->subprog_info[subprog].might_sleep && !in_sleepable_context(env)) { verbose(env, "sleepable global function %s() called in %s\n", sub_name, non_sleepable_context_description(env)); + operation = bpf_diag_fmt(env, "sleepable global function %s()", sub_name); + bpf_diag_ctx_forbidden(env, *insn_idx, operation, + "Move the call outside the critical section, or use a non-sleepable function."); return -EINVAL; } @@ -10495,6 +10505,8 @@ static int check_resource_leak(struct bpf_verifier_env *env, bool exception_exit if (check_lock && env->cur_state->active_locks) { verbose(env, "%s cannot be used inside bpf_spin_lock-ed region\n", prefix); + bpf_diag_ctx_active(env, env->insn_idx, prefix, BPF_DIAG_CONTEXT_LOCK, + "Release the BPF spin lock before this operation on every path."); return -EINVAL; } @@ -10506,16 +10518,23 @@ static int check_resource_leak(struct bpf_verifier_env *env, bool exception_exit if (check_lock && env->cur_state->active_irq_id) { verbose(env, "%s cannot be used inside bpf_local_irq_save-ed region\n", prefix); + bpf_diag_ctx_active(env, env->insn_idx, prefix, BPF_DIAG_CONTEXT_IRQ, + "Restore the saved IRQ state before this operation on every path."); return -EINVAL; } if (check_lock && env->cur_state->active_rcu_locks) { verbose(env, "%s cannot be used inside bpf_rcu_read_lock-ed region\n", prefix); + bpf_diag_ctx_active(env, env->insn_idx, prefix, BPF_DIAG_CONTEXT_RCU, + "Call bpf_rcu_read_unlock() before this operation on every path."); return -EINVAL; } if (check_lock && env->cur_state->active_preempt_locks) { verbose(env, "%s cannot be used inside bpf_preempt_disable-ed region\n", prefix); + bpf_diag_ctx_active( + env, env->insn_idx, prefix, BPF_DIAG_CONTEXT_PREEMPT, + "Call bpf_preempt_enable() before this operation on every path."); return -EINVAL; } @@ -10697,6 +10716,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn enum bpf_type_flag ret_flag; struct bpf_reg_state *regs; struct bpf_call_arg_meta meta; + const char *operation; int insn_idx = *insn_idx_p; bool changes_data; int i, err, func_id; @@ -10744,6 +10764,10 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn if (fn->might_sleep && !in_sleepable_context(env)) { verbose(env, "sleepable helper %s#%d in %s\n", func_id_name(func_id), func_id, non_sleepable_context_description(env)); + operation = bpf_diag_fmt(env, "sleepable helper %s#%d", + func_id_name(func_id), func_id); + bpf_diag_ctx_forbidden(env, insn_idx, operation, + "Move the helper call outside the critical section, or use a non-sleepable helper."); return -EINVAL; } @@ -13591,6 +13615,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, const struct btf_type *t, *ptr_type; struct bpf_call_arg_meta meta; struct bpf_insn_aux_data *insn_aux; + const char *operation; int err, insn_idx = *insn_idx_p; u32 i, nargs, ptr_type_id; struct bpf_kfunc_desc *desc; @@ -13656,6 +13681,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, sleepable = bpf_is_kfunc_sleepable(&meta); if (sleepable && !in_sleepable(env)) { verbose(env, "program must be sleepable to call sleepable kfunc %s\n", func_name); + operation = bpf_diag_fmt(env, "sleepable kfunc %s", func_name); + bpf_diag_ctx_forbidden(env, insn_idx, operation, + "Mark the program sleepable if the program type allows it, or use a non-sleepable kfunc."); return -EACCES; } @@ -13726,6 +13754,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, } else if (rcu_unlock) { if (env->cur_state->active_rcu_locks == 0) { verbose(env, "unmatched rcu read unlock (kernel function %s)\n", func_name); + bpf_diag_ctx_underflow( + env, insn_idx, func_name, BPF_DIAG_CONTEXT_RCU, + "Remove the extra bpf_rcu_read_unlock() call, or ensure this path first enters an RCU read lock region."); return -EINVAL; } env->cur_state->active_rcu_locks--; @@ -13740,6 +13771,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, } else if (preempt_enable) { if (env->cur_state->active_preempt_locks == 0) { verbose(env, "unmatched attempt to enable preemption (kernel function %s)\n", func_name); + bpf_diag_ctx_underflow( + env, insn_idx, func_name, BPF_DIAG_CONTEXT_PREEMPT, + "Remove the extra bpf_preempt_enable() call, or ensure this path first disables preemption."); return -EINVAL; } env->cur_state->active_preempt_locks--; @@ -13752,6 +13786,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, if (sleepable && !in_sleepable_context(env)) { verbose(env, "kernel func %s is sleepable within %s\n", func_name, non_sleepable_context_description(env)); + operation = bpf_diag_fmt(env, "sleepable kfunc %s", func_name); + bpf_diag_ctx_forbidden(env, insn_idx, operation, + "Move the kfunc call outside the critical section, or use a non-sleepable kfunc."); return -EACCES; } @@ -13762,6 +13799,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, if (is_kfunc_rcu_protected(&meta) && !in_rcu_cs(env)) { verbose(env, "kernel func %s requires RCU critical section protection\n", func_name); + bpf_diag_ctx_required( + env, insn_idx, func_name, BPF_DIAG_CONTEXT_RCU, + "Call this kfunc between bpf_rcu_read_lock() and bpf_rcu_read_unlock(), keeping all exit paths balanced."); return -EACCES; } @@ -18039,6 +18079,10 @@ static int do_check_insn(struct bpf_verifier_env *env, bool *do_print_state) !kfunc_spin_allowed(env, insn->imm, insn->off))) { verbose(env, "function calls are not allowed while holding a lock\n"); + bpf_diag_ctx_active( + env, env->insn_idx, + "function call", BPF_DIAG_CONTEXT_LOCK, + "Release the BPF spin lock before making this call, or move the call outside the locked region."); return -EINVAL; } }