[PATCH bpf-next v1 08/14] bpf: Report non-sleepable kfunc programs accurately
Kumar Kartikeya Dwivedi <[email protected]>
| Newsgroups | org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
A sleepable kfunc call can fail either because the program is not sleepable or because an otherwise sleepable program has entered a non-sleepable critical section. check_kfunc_call() checks these conditions separately. The first check is only gated by in_sleepable(), so the shared diagnostic can blame an active RCU, preemption-disabled, IRQ-disabled, or locked region even though leaving that region would not make the program sleepable. Adding a second diagnostic entry point only to force the program context would duplicate the API. Reject the call once based on in_sleepable_context(). Teach the shared bpf_diag_ctx_forbidden() reporter to prefer the non-sleepable program when the current verifier state is not sleepable; otherwise preserve the RCU, preempt, IRQ, and lock priority for active contexts. Select the kfunc message and suggestion according to that cause, and align the helper and global-function descriptions with the same priority. This avoids a diagnostic-only wrapper while retaining context history for sleepable programs that enter a forbidden region. Link: https://lore.kernel.org/bpf/2e42a1a2bf45f4d2aba7495bdc9f147558055740e2f3c8b9dae255f6c57fc13c@mail.kernel.org/ Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]> --- kernel/bpf/diagnostics.c | 4 +++- kernel/bpf/verifier.c | 34 ++++++++++++++++++---------------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/kernel/bpf/diagnostics.c b/kernel/bpf/diagnostics.c index 2c475174a640..df9259fa0ea7 100644 --- a/kernel/bpf/diagnostics.c +++ b/kernel/bpf/diagnostics.c @@ -1083,7 +1083,9 @@ void bpf_diag_ctx_forbidden(struct bpf_verifier_env *env, u32 insn_idx, const char *constraint, *context; u32 depth; - if (env->cur_state->active_rcu_locks) + if (!env->cur_state->in_sleepable) + ctx_kind = BPF_DIAG_CONTEXT_NONE; + else 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; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index da2ec0655b17..3de9e4f617b6 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -9912,7 +9912,7 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn, 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."); + "Call the function from a sleepable program outside any critical section, or use a non-sleepable function."); return -EINVAL; } @@ -10731,6 +10731,8 @@ static inline bool in_sleepable_context(struct bpf_verifier_env *env) static const char *non_sleepable_context_description(struct bpf_verifier_env *env) { + if (!in_sleepable(env)) + return "non-sleepable prog"; if (env->cur_state->active_rcu_locks) return "rcu_read_lock region"; if (env->cur_state->active_preempt_locks) @@ -10739,7 +10741,7 @@ static const char *non_sleepable_context_description(struct bpf_verifier_env *en return "IRQ-disabled region"; if (env->cur_state->active_locks) return "lock region"; - return "non-sleepable prog"; + return "non-sleepable context"; } static int release_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, @@ -10835,7 +10837,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn 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."); + "Call the helper from a sleepable program outside any critical section, or use a non-sleepable helper."); return -EINVAL; } @@ -13760,11 +13762,20 @@ 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); + if (sleepable && !in_sleepable_context(env)) { + const char *suggestion; + + if (in_sleepable(env)) { + verbose(env, "kernel func %s is sleepable within %s\n", + func_name, non_sleepable_context_description(env)); + suggestion = "Move the kfunc call outside the critical section, or use a non-sleepable kfunc."; + } else { + verbose(env, "program must be sleepable to call sleepable kfunc %s\n", + func_name); + suggestion = "Mark the program sleepable if the program type allows it, or use a non-sleepable kfunc."; + } 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."); + bpf_diag_ctx_forbidden(env, insn_idx, operation, suggestion); return -EACCES; } @@ -13864,15 +13875,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, invalidate_rcu_protected_refs(env); } - 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; - } - if (in_rbtree_lock_required_cb(env) && (rcu_lock || rcu_unlock)) { verbose(env, "Calling bpf_rcu_read_{lock,unlock} in unnecessary rbtree callback\n"); return -EACCES; -- 2.53.0