[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
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.