On Fri, 22 Mar 2024 at 15:57, Benjamin Tissoires bentiss@kernel.org wrote:
In this patch, bpf_timer_set_sleepable_cb() is functionally equivalent to bpf_timer_set_callback(), to the exception that it enforces the timer to be started with BPF_F_TIMER_SLEEPABLE.
But given that bpf_timer_set_callback() is a helper when bpf_timer_set_sleepable_cb() is a kfunc, we need to teach the verifier about its attached callback. Marking that callback as sleepable will be done in a separate patch
Acked-by: Eduard Zingerman eddyz87@gmail.com Signed-off-by: Benjamin Tissoires bentiss@kernel.org
[...]
@@ -19548,6 +19582,28 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, desc->func_id == special_kfunc_list[KF_bpf_rdonly_cast]) { insn_buf[0] = BPF_MOV64_REG(BPF_REG_0, BPF_REG_1); *cnt = 1;
} else if (is_bpf_timer_set_sleepable_cb_impl_kfunc(desc->func_id)) {/* The verifier will process callback_fn as many times as necessary* with different maps and the register states prepared by* set_timer_callback_state will be accurate.** The following use case is valid:* map1 is shared by prog1, prog2, prog3.* prog1 calls bpf_timer_init for some map1 elements* prog2 calls bpf_timer_set_callback for some map1 elements.* Those that were not bpf_timer_init-ed will return -EINVAL.* prog3 calls bpf_timer_start for some map1 elements.* Those that were not both bpf_timer_init-ed and* bpf_timer_set_callback-ed will return -EINVAL.*/struct bpf_insn ld_addrs[2] = {BPF_LD_IMM64(BPF_REG_3, (long)env->prog->aux),};insn_buf[0] = ld_addrs[0];insn_buf[1] = ld_addrs[1];insn_buf[2] = *insn;*cnt = 3; }
Would be better to handle the fixup of all kfuncs in one place, i.e. fixup_kfunc_call.
return 0;}
-- 2.44.0