Re: [PATCH] bpf: add KF_SPIN_LOCK flag for kfuncs under bpf_spin_lock

["Kumar Kartikeya Dwivedi" <memxor@gmail.com>]

On Tue Aug 4, 2026 at 11:37 AM CEST, Kaitao Cheng wrote:
> 在 2026/8/4 16:43, Kumar Kartikeya Dwivedi 写道:
>> On Tue Aug 4, 2026 at 10:32 AM CEST, Kaitao Cheng wrote:
>>> From: Kaitao Cheng <chengkaitao@kylinos.cn>
>>>
>>> Introduce the KF_SPIN_LOCK kfunc metadata flag in BTF so kfuncs may be
>>> explicitly marked as safe to call while holding bpf_spin_lock.
>>>
>>> Allow kfuncs defined in kernel modules to be marked with KF_SPIN_LOCK.
>>>
>>> Example: BTF_ID_FLAGS(func, $kfunc_name, KF_SPIN_LOCK)
>>>
>>> Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
>>> ---
>>>  include/linux/btf.h   |  1 +
>>>  kernel/bpf/verifier.c | 20 +++++++++++++++-----
>>>  2 files changed, 16 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/include/linux/btf.h b/include/linux/btf.h
>>> index c09b7994de4e..380dc10b6750 100644
>>> --- a/include/linux/btf.h
>>> +++ b/include/linux/btf.h
>>> @@ -79,6 +79,7 @@
>>>  #define KF_ARENA_ARG1   (1 << 14) /* kfunc takes an arena pointer as its first argument */
>>>  #define KF_ARENA_ARG2   (1 << 15) /* kfunc takes an arena pointer as its second argument */
>>>  #define KF_IMPLICIT_ARGS (1 << 16) /* kfunc has implicit arguments supplied by the verifier */
>>> +#define KF_SPIN_LOCK    (1 << 17) /* kfunc is allowed inside bpf_spin_lock-ed region */
>>>
>>>  /*
>>>   * Tag marking a kernel function as a kfunc. This is meant to minimize the
>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>>> index 7aa47342dc65..9f3feba3c2fe 100644
>>> --- a/kernel/bpf/verifier.c
>>> +++ b/kernel/bpf/verifier.c
>>> @@ -11657,11 +11657,21 @@ static bool is_bpf_stream_kfunc(u32 btf_id)
>>>  	       btf_id == special_kfunc_list[KF_bpf_stream_print_stack];
>>>  }
>>>
>>> -static bool kfunc_spin_allowed(u32 btf_id)
>>> +static bool kfunc_spin_allowed(struct bpf_verifier_env *env, s32 func_id, s16 offset)
>>>  {
>>> -	return is_bpf_graph_api_kfunc(btf_id) || is_bpf_iter_num_api_kfunc(btf_id) ||
>>> -	       is_bpf_res_spin_lock_kfunc(btf_id) || is_bpf_arena_kfunc(btf_id) ||
>>> -	       is_bpf_stream_kfunc(btf_id);
>>> +	struct bpf_kfunc_meta kfunc;
>>> +	int err;
>>> +
>>> +	if (is_bpf_graph_api_kfunc(func_id) || is_bpf_iter_num_api_kfunc(func_id) ||
>>> +	    is_bpf_res_spin_lock_kfunc(func_id) || is_bpf_arena_kfunc(func_id) ||
>>> +	    is_bpf_stream_kfunc(func_id))
>>> +		return true;
>>> +
>>> +	err = fetch_kfunc_meta(env, func_id, offset, &kfunc);
>>> +	if (err || !kfunc.flags)
>>> +		return false;
>>> +
>>> +	return *kfunc.flags & KF_SPIN_LOCK;
>>>  }
>>>
>>>  static bool is_sync_callback_calling_kfunc(u32 btf_id)
>>> @@ -17289,7 +17299,7 @@ static int do_check_insn(struct bpf_verifier_env *env, bool *do_print_state)
>>>  				     insn->imm != BPF_FUNC_spin_unlock &&
>>>  				     insn->imm != BPF_FUNC_kptr_xchg) ||
>>>  				    (insn->src_reg == BPF_PSEUDO_KFUNC_CALL &&
>>> -				     (insn->off != 0 || !kfunc_spin_allowed(insn->imm)))) {
>>> +				     !kfunc_spin_allowed(env, insn->imm, insn->off))) {
>>>  					verbose(env,
>>>  						"function calls are not allowed while holding a lock\n");
>>>  					return -EINVAL;
>>
>> Why not apply it to all of the existing kfuncs? Also, use [PATCH bpf-next] subject.
>
> I do not think all kfuncs are suitable for the KF_SPIN_LOCK context.
> Kfuncs marked with KF_SLEEPABLE are an obvious example. Developers
> should add KF_SPIN_LOCK with great care and ensure that the kfunc
> does not sleep and is safe to call in atomic context; otherwise, it
> could easily introduce deadlocks.
>
> However, the kfuncs explicitly allowed by kfunc_spin_allowed(), such
> as those covered by is_bpf_graph_api_kfunc, is_bpf_iter_num_api_kfunc,
> is_bpf_res_spin_lock_kfunc, is_bpf_arena_kfunc and is_bpf_stream_kfunc,
> can be marked with KF_SPIN_LOCK. We can then simplify kfunc_spin_allowed()
> to rely solely on the flag.

Yes, of course. I meant those that we maintain as a list in the verifier.
KF_SPIN_LOCK is not a very optimal name. Should we do KF_ALLOWED_IN_LOCK_CS?
A bit verbose but better captures the intended meaning.