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@xxxxxxxxxxxxxxx>
> ---
Reviewed-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
[...]
> @@ -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 ---
