Commit dd4d3ef3 authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

Merge branch 'global-subprogs-in-rcu-preempt-irq-disabled-sections' 

Kumar Kartikeya Dwivedi says:

====================
Global subprogs in RCU/{preempt,irq}-disabled sections

Small change to allow non-sleepable global subprogs in
RCU, preempt-disabled, and irq-disabled sections. For
now, we don't lift the limitation for locks as it requires
more analysis, and will do this one resilient spin locks
land.

This surfaced a bug where sleepable global subprogs were
allowed in RCU read sections, that has been fixed. Tests
have been added to cover various cases.

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

  * Fix broken to_be_replaced argument in the selftest.
  * Adjust selftest program type.

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

  * Rename subprog_info[i].sleepable to might_sleep, which more
    accurately reflects the nature of the bit. 'sleepable' means whether
    a given context is allowed to, while might_sleep captures if it
    does.
  * Disallow extensions that might sleep to attach to targets that don't
    sleep, since they'd be permitted to be called in atomic contexts. (Eduard)
  * Add tests for mixing non-sleepable and sleepable global function
    calls, and extensions attaching to non-sleepable global functions. (Eduard)
  * Rename changes_pkt_data -> summarization
====================

Link: https://patch.msgid.link/20250301151846.1552362-1-memxor@gmail.com


Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents 93cf4e53 72ed076a

include/linux/bpf.h

+1 −0
Original line number Diff line number Diff line
@@ -1531,6 +1531,7 @@ struct bpf_prog_aux { 
	bool jits_use_priv_stack;

	bool priv_stack_requested;

	bool changes_pkt_data;

	bool might_sleep;

	u64 prog_array_member_cnt; /* counts how many times as member of prog_array */

	struct mutex ext_mutex; /* mutex for is_extended and prog_array_member_cnt */

	struct bpf_arena *arena;

include/linux/bpf_verifier.h

+1 −0
Original line number Diff line number Diff line
@@ -667,6 +667,7 @@ struct bpf_subprog_info { 
	/* true if bpf_fastcall stack region is used by functions that can't be inlined */

	bool keep_fastcall_stack: 1;

	bool changes_pkt_data: 1;

	bool might_sleep: 1;



	enum priv_stack_mode priv_stack_mode;

	u8 arg_cnt;

kernel/bpf/verifier.c

+48 −14
Original line number Diff line number Diff line
@@ -10317,23 +10317,18 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn, 
	if (subprog_is_global(env, subprog)) {

		const char *sub_name = subprog_name(env, subprog);









		/* Only global subprogs cannot be called with a lock held. */









		if (env->cur_state->active_locks) {









			verbose(env, "global function calls are not allowed while holding a lock,\n"









				     "use static function instead\n");









			return -EINVAL;









		}

















		/* Only global subprogs cannot be called with preemption disabled. */









		if (env->cur_state->active_preempt_locks) {









			verbose(env, "global function calls are not allowed with preemption disabled,\n"









				     "use static function instead\n");









			return -EINVAL;









		}

















		if (env->cur_state->active_irq_id) {









			verbose(env, "global function calls are not allowed with IRQs disabled,\n"









				     "use static function instead\n");









		if (env->subprog_info[subprog].might_sleep &&









		    (env->cur_state->active_rcu_lock || env->cur_state->active_preempt_locks ||









		     env->cur_state->active_irq_id || !in_sleepable(env))) {









			verbose(env, "global functions that may sleep are not allowed in non-sleepable context,\n"









				     "i.e., in a RCU/IRQ/preempt-disabled section, or in\n"









				     "a non-sleepable BPF program context\n");









			return -EINVAL;









		}
















@@ -16703,6 +16698,14 @@ static void mark_subprog_changes_pkt_data(struct bpf_verifier_env *env, int off)







	subprog->changes_pkt_data = true;









}

















static void mark_subprog_might_sleep(struct bpf_verifier_env *env, int off)









{









	struct bpf_subprog_info *subprog;

















	subprog = find_containing_subprog(env, off);









	subprog->might_sleep = true;









}

















/* 't' is an index of a call-site.









 * 'w' is a callee entry point.









 * Eventually this function would be called when env->cfg.insn_state[w] == EXPLORED.








@@ -16716,6 +16719,7 @@ static void merge_callee_effects(struct bpf_verifier_env *env, int t, int w)







	caller = find_containing_subprog(env, t);









	callee = find_containing_subprog(env, w);









	caller->changes_pkt_data |= callee->changes_pkt_data;









	caller->might_sleep |= callee->might_sleep;









}

















/* non-recursive DFS pseudo code








@@ -17183,9 +17187,20 @@ static int visit_insn(int t, struct bpf_verifier_env *env)







			mark_prune_point(env, t);









			mark_jmp_point(env, t);









		}









		if (bpf_helper_call(insn) && bpf_helper_changes_pkt_data(insn->imm))









		if (bpf_helper_call(insn)) {









			const struct bpf_func_proto *fp;

















			ret = get_helper_proto(env, insn->imm, &fp);









			/* If called in a non-sleepable context program will be









			 * rejected anyway, so we should end up with precise









			 * sleepable marks on subprogs, except for dead code









			 * elimination.









			 */









			if (ret == 0 && fp->might_sleep)









				mark_subprog_might_sleep(env, t);









			if (bpf_helper_changes_pkt_data(insn->imm))









				mark_subprog_changes_pkt_data(env, t);









		if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {









		} else if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {









			struct bpf_kfunc_call_arg_meta meta;

















			ret = fetch_kfunc_meta(env, insn, &meta, NULL);








@@ -17204,6 +17219,13 @@ static int visit_insn(int t, struct bpf_verifier_env *env)







				 */









				mark_force_checkpoint(env, t);









			}









			/* Same as helpers, if called in a non-sleepable context









			 * program will be rejected anyway, so we should end up









			 * with precise sleepable marks on subprogs, except for









			 * dead code elimination.









			 */









			if (ret == 0 && is_kfunc_sleepable(&meta))









				mark_subprog_might_sleep(env, t);









		}









		return visit_func_call_insn(t, insns, env, insn->src_reg == BPF_PSEUDO_CALL);
















@@ -17320,6 +17342,7 @@ static int check_cfg(struct bpf_verifier_env *env)







	}









	ret = 0; /* cfg looks good */









	env->prog->aux->changes_pkt_data = env->subprog_info[0].changes_pkt_data;









	env->prog->aux->might_sleep = env->subprog_info[0].might_sleep;

















err_free:









	kvfree(insn_state);








@@ -20845,6 +20868,7 @@ static int jit_subprogs(struct bpf_verifier_env *env)







		func[i]->aux->tail_call_reachable = env->subprog_info[i].tail_call_reachable;









		func[i]->aux->exception_cb = env->subprog_info[i].is_exception_cb;









		func[i]->aux->changes_pkt_data = env->subprog_info[i].changes_pkt_data;









		func[i]->aux->might_sleep = env->subprog_info[i].might_sleep;









		if (!i)









			func[i]->aux->exception_boundary = env->seen_exception;









		func[i] = bpf_int_jit_compile(func[i]);








@@ -22723,6 +22747,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,







	if (tgt_prog) {









		struct bpf_prog_aux *aux = tgt_prog->aux;









		bool tgt_changes_pkt_data;









		bool tgt_might_sleep;

















		if (bpf_prog_is_dev_bound(prog->aux) &&









		    !bpf_prog_dev_bound_match(prog, tgt_prog)) {








@@ -22765,6 +22790,15 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,







					"Extension program changes packet data, while original does not\n");









				return -EINVAL;









			}

















			tgt_might_sleep = aux->func









					  ? aux->func[subprog]->aux->might_sleep









					  : aux->might_sleep;









			if (prog->aux->might_sleep && !tgt_might_sleep) {









				bpf_log(log,









					"Extension program may sleep, while original does not\n");









				return -EINVAL;









			}









		}









		if (!tgt_prog->jited) {









			bpf_log(log, "Can attach to only JITed progs\n");

























tools/testing/selftests/bpf/prog_tests/rcu_read_lock.c



+3
−0






























Original line number
Diff line number
Diff line







@@ -81,6 +81,9 @@ static const char * const inproper_region_tests[] = {







	"nested_rcu_region",









	"rcu_read_lock_global_subprog_lock",









	"rcu_read_lock_global_subprog_unlock",









	"rcu_read_lock_sleepable_helper_global_subprog",









	"rcu_read_lock_sleepable_kfunc_global_subprog",









	"rcu_read_lock_sleepable_global_subprog_indirect",









};



















static void test_inproper_region(void)
































tools/testing/selftests/bpf/prog_tests/spin_lock.c



+3
−0






























Original line number
Diff line number
Diff line







@@ -50,6 +50,9 @@ static struct {







	{ "lock_id_mismatch_innermapval_mapval", "bpf_spin_unlock of different lock" },









	{ "lock_global_subprog_call1", "global function calls are not allowed while holding a lock" },









	{ "lock_global_subprog_call2", "global function calls are not allowed while holding a lock" },









	{ "lock_global_sleepable_helper_subprog", "global function calls are not allowed while holding a lock" },









	{ "lock_global_sleepable_kfunc_subprog", "global function calls are not allowed while holding a lock" },









	{ "lock_global_sleepable_subprog_indirect", "global function calls are not allowed while holding a lock" },









};



















static int match_regex(const char *pattern, const char *string)