Re: [PATCH bpf-next v1 2/3] bpf: Add support for KF_RET_RCU flag

[Andrea Righi <arighi@nvidia.com>]

Hi Kumar,

thanks for looking at this! Comment below.

On Mon, Sep 15, 2025 at 02:47:30AM +0000, Kumar Kartikeya Dwivedi wrote:
> Add a kfunc annotation 'KF_RET_RCU' to signal that the return type must
> be marked MEM_RCU, to return objects that are RCU protected. Naturally,
> this must imply that the kfunc is invoked in an RCU critical section,
> and thus the presence of this flag implies the presence of the
> KF_RCU_PROTECTED flag. Upcoming kfunc scx_bpf_cpu_curr() [0] will be
> made to make use of this flag.

I'm not sure we actually need two separate annotations, I can't think of a
case where KF_RCU_PROTECTED would be useful without also having KF_RET_RCU.

What I mean is: if the kfunc is only meant to be called inside an RCU
critical section, but doesn't return an RCU-protected pointer, then we can
simply add rcu_read_lock/unlock() internally in the kfunc. And for kfuncs
that take RCU-protected arguments we already have KF_RCU.

So it seems sufficient to have a single annotation that implements the
semantic "this kfunc returns an RCU-protected pointer".

What do you think?

Thanks,
-Andrea

> 
>   [0]: https://lore.kernel.org/all/20250903212311.369697-3-christian.loehle@arm.com
> 
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  Documentation/bpf/kfuncs.rst | 13 +++++++++++--
>  include/linux/btf.h          |  1 +
>  kernel/bpf/verifier.c        |  7 +++++++
>  3 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst
> index 18ba1f7c26b3..7d1b7009338b 100644
> --- a/Documentation/bpf/kfuncs.rst
> +++ b/Documentation/bpf/kfuncs.rst
> @@ -346,10 +346,19 @@ arguments are at least RCU protected pointers. This may transitively imply that
>  RCU protection is ensured, but it does not work in cases of kfuncs which require
>  RCU protection but do not take RCU protected arguments.
>  
> +2.4.9 KF_RET_RCU flag
> +---------------------
> +
> +The KF_RET_RCU flag is used for kfuncs which return pointers to RCU protected
> +objects. Since this only works when the invocation of the kfunc is made in an
> +active RCU critical section, the usage of this flag implies ``KF_RCU_PROTECTED``
> +flag automatically. This flag may be combined with other return value modifiers,
> +such as ``KF_RET_NULL``.
> +
>  .. _KF_deprecated_flag:
>  
> -2.4.9 KF_DEPRECATED flag
> -------------------------
> +2.4.10 KF_DEPRECATED flag
> +-------------------------
>  
>  The KF_DEPRECATED flag is used for kfuncs which are scheduled to be
>  changed or removed in a subsequent kernel release. A kfunc that is
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 9eda6b113f9b..97205b8a938c 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -79,6 +79,7 @@
>  #define KF_ARENA_RET    (1 << 13) /* kfunc returns an arena pointer */
>  #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_RET_RCU      ((1 << 16) | KF_RCU_PROTECTED) /* kfunc returns an RCU protected pointer, implies KF_RCU_PROTECTED */
>  
>  /*
>   * 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 aa7c82ab50b9..f1cc602ed556 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -12342,6 +12342,11 @@ static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
>  	return meta->kfunc_flags & KF_RET_NULL;
>  }
>  
> +static bool is_kfunc_ret_rcu(struct bpf_kfunc_call_arg_meta *meta)
> +{
> +	return meta->kfunc_flags & KF_RET_RCU;
> +}
> +
>  static bool is_kfunc_bpf_rcu_read_lock(struct bpf_kfunc_call_arg_meta *meta)
>  {
>  	return meta->func_id == special_kfunc_list[KF_bpf_rcu_read_lock];
> @@ -14042,6 +14047,8 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  
>  			if (meta.func_id == special_kfunc_list[KF_bpf_get_kmem_cache])
>  				regs[BPF_REG_0].type |= PTR_UNTRUSTED;
> +			else if (is_kfunc_ret_rcu(&meta))
> +				regs[BPF_REG_0].type |= MEM_RCU;
>  
>  			if (is_iter_next_kfunc(&meta)) {
>  				struct bpf_reg_state *cur_iter;
> -- 
> 2.51.0
>