Merge branch 'fix-global-subprog-verification-context'

Kumar Kartikeya Dwivedi says:

====================
Fix global subprog verification context

Global subprogs can be invoked in both sleepable and non-sleepable
contexts, but are verified based on context assumptions for the program
type as a whole and not the precise context in which they are invoked.
Nicholas identified that this can violate verifier safety assumptions.
Verify global subprogs once for each observed sleepability context and add
workqueue callback regression tests.

Changelog:
----------
v2 -> v3
v2: https://lore.kernel.org/bpf/20260905051224.2325381-1-memxor@gmail.com

 * Clarify that globals are checked only in contexts from which they are
   reached, with two passes only when both contexts occur. (Alexei)
 * Implement in_sleepable_context() as !in_rcu_cs(). (Alexei, Eduard)
 * Keep the original subprogram traversal order and repeat until all
   called contexts have been verified. (Eduard)
 * Explain why instruction accounting needs the saved total and why the
   exception callback uses the main program's context. (Eduard)
 * Rename the do_check_common() parameter to is_sleepable. (BPF CI)
 * Preserve the harmless global calls with __weak __noinline and check
   their instruction totals across both contexts. Drop the redundant
   task-work global RCU test and retain the review acknowledgment. (Eduard)

v1 -> v2
v1: https://lore.kernel.org/bpf/20260905034018.2095649-1-memxor@gmail.com

 * Preserve the harmless global subprog calls in the positive test with
   barrier() so LLVM cannot eliminate the intended coverage.
 * Return zero explicitly from workqueue callbacks after global subprog calls.
 * Document cumulative instruction accounting across verification contexts.
 * Fix BPF multi-line comment formatting.
====================

Link: https://patch.msgid.link/20260914131923.2544250-1-memxor@gmail.com
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>

include/linux/bpf.h

@@ -1651,8 +1651,9 @@ static inline void bpf_trampoline_set_flags(struct bpf_trampoline *tr, u32 flags
 struct bpf_func_info_aux {
 	u16 linkage;
 	bool unreliable;
-	bool called : 1;
-	bool verified : 1;
+	/* Indexed by in_sleepable. */
+	bool called[2];
+	bool verified[2];
 };
 
 enum bpf_jit_poke_reason {

kernel/bpf/verifier.c

@@ -9931,6 +9931,7 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 	if (err == -EFAULT)
 		return err;
 	if (bpf_subprog_is_global(env, subprog)) {
+		struct bpf_func_info_aux *sub_aux = subprog_aux(env, subprog);
 		const char *sub_name = bpf_subprog_name(env, subprog);
 		const char *operation;
 		bool returns_void;
@@ -9962,11 +9963,10 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 		if (env->log.level & BPF_LOG_LEVEL)
 			verbose(env, "Func#%d ('%s') is global and assumed valid.\n",
 				subprog, sub_name);
+		sub_aux->called[in_sleepable_context(env)] = true;
 		returns_void = subprog_returns_void(env, subprog);
 		if (env->subprog_info[subprog].changes_pkt_data)
 			clear_all_pkt_pointers(env);
-		/* mark global subprog for verifying after main prog */
-		subprog_aux(env, subprog)->called = true;
 		if (returns_void)
 			bpf_diag_record_scrub(env, &caller->regs[BPF_REG_0], BPF_DIAG_MOD_CALLER_SAVED);
 		else
@@ -10784,11 +10784,7 @@ int bpf_get_helper_proto(struct bpf_verifier_env *env, int func_id,
 /* Check if we're in a sleepable context. */
 static inline bool in_sleepable_context(struct bpf_verifier_env *env)
 {
-	return !env->cur_state->active_rcu_locks &&
-	       !env->cur_state->active_preempt_locks &&
-	       !env->cur_state->active_locks &&
-	       !env->cur_state->active_irq_id &&
-	       in_sleepable(env);
+	return !in_rcu_cs(env);
 }
 
 static const char *non_sleepable_context_description(struct bpf_verifier_env *env)
@@ -19447,13 +19443,14 @@ static void free_states(struct bpf_verifier_env *env)
 	}
 }
 
-static int do_check_common(struct bpf_verifier_env *env, int subprog)
+static int do_check_common(struct bpf_verifier_env *env, int subprog, bool is_sleepable)
 {
 	bool pop_log = !(env->log.level & BPF_LOG_LEVEL2);
 	struct bpf_subprog_info *sub = subprog_info(env, subprog);
 	struct bpf_prog_aux *aux = env->prog->aux;
 	struct bpf_verifier_state *state;
 	struct bpf_reg_state *regs;
+	u32 old_insns_total = sub->insns_total;
 	u32 insn_processed = env->insn_processed;
 	int ret, i;
 
@@ -19466,7 +19463,7 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
 	state->curframe = 0;
 	state->speculative = false;
 	state->branches = 1;
-	state->in_sleepable = env->prog->sleepable;
+	state->in_sleepable = is_sleepable;
 	state->frame[0] = kzalloc_obj(struct bpf_func_state, GFP_KERNEL_ACCOUNT);
 	if (!state->frame[0]) {
 		kfree(state);
@@ -19607,8 +19604,10 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
 	 * not accounted as callees by account_current_path().
 	 * Accumulate their total counts as total counts of the main or
 	 * global subprog hosting the async call.
+	 * Start from the saved total of earlier contexts: adding to the current
+	 * total would count this pass's synchronous paths twice.
 	 */
-	env->subprog_info[subprog].insns_total = env->insn_processed - insn_processed;
+	sub->insns_total = old_insns_total + (env->insn_processed - insn_processed);
 	return ret;
 }
 
@@ -19636,14 +19635,19 @@ static int do_check_subprogs(struct bpf_verifier_env *env)
 {
 	struct bpf_prog_aux *aux = env->prog->aux;
 	struct bpf_func_info_aux *sub_aux;
-	int i, ret, new_cnt;
+	int context, i, ret, new_cnt;
 
 	if (!aux->func_info)
 		return 0;
 
-	/* exception callback is presumed to be always called */
-	if (env->exception_callback_subprog)
-		subprog_aux(env, env->exception_callback_subprog)->called = true;
+	/*
+	 * Callbacks cannot throw, so the exception callback always runs in the
+	 * main program's context. It is presumed to be always called.
+	 */
+	if (env->exception_callback_subprog) {
+		sub_aux = subprog_aux(env, env->exception_callback_subprog);
+		sub_aux->called[env->prog->sleepable] = true;
+	}
 
 again:
 	new_cnt = 0;
@@ -19652,29 +19656,28 @@ static int do_check_subprogs(struct bpf_verifier_env *env)
 			continue;
 
 		sub_aux = subprog_aux(env, i);
-		if (!sub_aux->called || sub_aux->verified)
-			continue;
+		for (context = 0; context < ARRAY_SIZE(sub_aux->called); context++) {
+			if (!sub_aux->called[context] || sub_aux->verified[context])
+				continue;
 
-		env->insn_idx = env->subprog_info[i].start;
-		WARN_ON_ONCE(env->insn_idx == 0);
-		ret = do_check_common(env, i);
-		if (ret) {
-			return ret;
-		} else if (env->log.level & BPF_LOG_LEVEL) {
-			verbose(env, "Func#%d ('%s') is safe for any args that match its prototype\n",
-				i, bpf_subprog_name(env, i));
+			env->insn_idx = env->subprog_info[i].start;
+			WARN_ON_ONCE(env->insn_idx == 0);
+			ret = do_check_common(env, i, context);
+			if (ret)
+				return ret;
+			if (env->log.level & BPF_LOG_LEVEL)
+				verbose(env, "Func#%d ('%s') is safe for any args "
+					"that match its prototype\n",
+					i, bpf_subprog_name(env, i));
+
+			sub_aux->verified[context] = true;
+			new_cnt++;
 		}
-
-		/* We verified new global subprog, it might have called some
-		 * more global subprogs that we haven't verified yet, so we
-		 * need to do another pass over subprogs to verify those.
-		 */
-		sub_aux->verified = true;
-		new_cnt++;
 	}
 
-	/* We can't loop forever as we verify at least one global subprog on
-	 * each pass.
+	/*
+	 * We can't loop forever as each pass verifies at least one new context,
+	 * and there are only two contexts per global subprog.
 	 */
 	if (new_cnt)
 		goto again;
@@ -19687,7 +19690,7 @@ static int do_check_main(struct bpf_verifier_env *env)
 	int ret;
 
 	env->insn_idx = 0;
-	ret = do_check_common(env, 0);
+	ret = do_check_common(env, 0, env->prog->sleepable);
 	if (!ret)
 		env->prog->aux->stack_depth = env->subprog_info[0].stack_depth;
 	return ret;

tools/testing/selftests/bpf/progs/verifier_async_cb_context.c

@@ -9,6 +9,11 @@
 
 char _license[] SEC("license") = "GPL";
 
+struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym;
+void bpf_task_release(struct task_struct *p) __ksym;
+void bpf_rcu_read_lock(void) __ksym;
+void bpf_rcu_read_unlock(void) __ksym;
+
 /* Timer tests */
 
 struct timer_elem {
@@ -164,6 +169,7 @@ int syscall_btf_find_prog(void *ctx)
 
 struct wq_elem {
 	struct bpf_wq w;
+	struct task_struct __kptr *task;
 };
 
 struct {
@@ -217,6 +223,106 @@ int wq_sleepable_prog(void *ctx)
 	return 0;
 }
 
+__noinline int wq_global_acquire(void)
+{
+	struct task_struct *task, *acquired;
+	struct wq_elem *val;
+	int key = 0;
+
+	val = bpf_map_lookup_elem(&wq_map, &key);
+	if (!val)
+		return 0;
+
+	task = val->task;
+	if (!task)
+		return 0;
+
+	acquired = bpf_task_acquire(task);
+	if (acquired)
+		bpf_task_release(acquired);
+	return 0;
+}
+
+static int wq_global_rcu_cb(void *map, int *key, void *value)
+{
+	wq_global_acquire();
+	return 0;
+}
+
+SEC("fentry/bpf_fentry_test1")
+__failure __msg("R1 must be a rcu pointer")
+int wq_global_rcu_prog(void *ctx)
+{
+	struct wq_elem *val;
+	int key = 0;
+
+	val = bpf_map_lookup_elem(&wq_map, &key);
+	if (!val)
+		return 0;
+
+	bpf_wq_init(&val->w, &wq_map, 0);
+	bpf_wq_set_callback(&val->w, wq_global_rcu_cb, 0);
+	return 0;
+}
+
+static int wq_global_rcu_lock_cb(void *map, int *key, void *value)
+{
+	bpf_rcu_read_lock();
+	wq_global_acquire();
+	bpf_rcu_read_unlock();
+	return 0;
+}
+
+SEC("fentry/bpf_fentry_test1")
+__success
+int wq_global_rcu_lock_prog(void *ctx)
+{
+	struct wq_elem *val;
+	int key = 0;
+
+	/* Verify the same global subprog in non-sleepable and protected contexts. */
+	wq_global_acquire();
+
+	val = bpf_map_lookup_elem(&wq_map, &key);
+	if (!val)
+		return 0;
+
+	bpf_wq_init(&val->w, &wq_map, 0);
+	bpf_wq_set_callback(&val->w, wq_global_rcu_lock_cb, 0);
+	return 0;
+}
+
+__weak __noinline int wq_global_no_rcu(void)
+{
+	return 0;
+}
+
+static int wq_global_no_rcu_cb(void *map, int *key, void *value)
+{
+	wq_global_no_rcu();
+	return 0;
+}
+
+SEC("fentry/bpf_fentry_test1")
+__success __log_level(4)
+__msg("subprog {{[0-9]+}} (wq_global_no_rcu) global insns_self 4 insns_total 4 stack 0")
+int wq_global_no_rcu_prog(void *ctx)
+{
+	struct wq_elem *val;
+	int key = 0;
+
+	/* Verify the same global in non-sleepable and unprotected contexts. */
+	wq_global_no_rcu();
+
+	val = bpf_map_lookup_elem(&wq_map, &key);
+	if (!val)
+		return 0;
+
+	bpf_wq_init(&val->w, &wq_map, 0);
+	bpf_wq_set_callback(&val->w, wq_global_no_rcu_cb, 0);
+	return 0;
+}
+
 /* Task work tests */
 
 struct task_work_elem {