Re: [PATCH 1/2] bpf: Allow bpf_for/bpf_repeat calls while holding a spinlock

[Eduard Zingerman <eddyz87@gmail.com>]

On Wed, 2025-01-01 at 15:37 -0500, Emil Tsalapatis wrote:
>  Add the bpf_iter_num_* kfuncs called by bpf_for in special_kfunc_list,
>  and allow the calls even while holding a spin lock.
> 
> Signed-off-by: Emil Tsalapatis (Meta) <emil@etsalapatis.com>
> ---

Reviewed-by: Eduard Zingerman <eddyz87@gmail.com>

[...]

> @@ -19048,7 +19066,7 @@ static int do_check(struct bpf_verifier_env *env)
>  				if (env->cur_state->active_locks) {
>  					if ((insn->src_reg == BPF_REG_0 && insn->imm != BPF_FUNC_spin_unlock) ||
>  					    (insn->src_reg == BPF_PSEUDO_KFUNC_CALL &&
> -					     (insn->off != 0 || !is_bpf_graph_api_kfunc(insn->imm)))) {
> +					     (insn->off != 0 || !kfunc_spin_allowed(insn->imm)))) {
>  						verbose(env, "function calls are not allowed while holding a lock\n");
>  						return -EINVAL;
>  					}


Nit: technically, 'bpf_loop' is a helper function independent of iter_num API.
     I suggest to change the name to is_bpf_iter_num_api_kfunc.
     Also, if we decide that loops are ok with spin locks,
     the condition above should be adjusted to allow calls to bpf_loop,
     e.g. to make the following test work:

--- 8< -------------------------------------
static int loop_cb(__u64 index, void *ctx)
{
	return 0;
}

SEC("tc")
__success __failure_unpriv __msg_unpriv("")
__retval(0)
int bpf_loop_inside_locked_region2(void)
{
	const int zero = 0;
	struct val *val;

	val = bpf_map_lookup_elem(&map_spin_lock, &zero);
	if (!val)
		return -1;

	bpf_spin_lock(&val->l);
	bpf_loop(10, loop_cb, NULL, 0);
	bpf_spin_unlock(&val->l);

	return 0;
}
------------------------------------- >8 ---