Re: [PATCH v9 01/10] btf: Add a new kfunc flag which allows to mark a function to be sleepable

[Jarkko Sakkinen <jarkko@kernel.org>]

On Tue, Aug 09, 2022 at 03:45:54PM +0200, Roberto Sassu wrote:
> From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> This allows to declare a kfunc as sleepable and prevents its use in
> a non sleepable program.

Nit: "Declare a kfunc as sleepable and prevent its use in a
non-sleepable program."

It's missing the part *how* the patch accomplishes its goals.

> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Co-developed-by: Yosry Ahmed <yosryahmed@google.com>
> Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> Signed-off-by: Hao Luo <haoluo@google.com>
> ---
>  Documentation/bpf/kfuncs.rst | 6 ++++++
>  include/linux/btf.h          | 1 +
>  kernel/bpf/btf.c             | 9 +++++++++
>  3 files changed, 16 insertions(+)
> 
> diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst
> index c0b7dae6dbf5..c8b21de1c772 100644
> --- a/Documentation/bpf/kfuncs.rst
> +++ b/Documentation/bpf/kfuncs.rst
> @@ -146,6 +146,12 @@ that operate (change some property, perform some operation) on an object that
>  was obtained using an acquire kfunc. Such kfuncs need an unchanged pointer to
>  ensure the integrity of the operation being performed on the expected object.
>  
> +2.4.6 KF_SLEEPABLE flag
> +-----------------------
> +
> +The KF_SLEEPABLE flag is used for kfuncs that may sleep. Such kfuncs can only
> +be called by sleepable BPF programs (BPF_F_SLEEPABLE).
> +
>  2.5 Registering the kfuncs
>  --------------------------
>  
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index cdb376d53238..976cbdd2981f 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -49,6 +49,7 @@
>   * for this case.
>   */
>  #define KF_TRUSTED_ARGS (1 << 4) /* kfunc only takes trusted pointer arguments */
> +#define KF_SLEEPABLE   (1 << 5) /* kfunc may sleep */
>  
>  struct btf;
>  struct btf_member;
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 7e64447659f3..d3e4c86b8fcd 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -6175,6 +6175,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
>  {
>  	enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
>  	bool rel = false, kptr_get = false, trusted_arg = false;
> +	bool sleepable = false;
>  	struct bpf_verifier_log *log = &env->log;
>  	u32 i, nargs, ref_id, ref_obj_id = 0;
>  	bool is_kfunc = btf_is_kernel(btf);
> @@ -6212,6 +6213,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
>  		rel = kfunc_flags & KF_RELEASE;
>  		kptr_get = kfunc_flags & KF_KPTR_GET;
>  		trusted_arg = kfunc_flags & KF_TRUSTED_ARGS;
> +		sleepable = kfunc_flags & KF_SLEEPABLE;
>  	}
>  
>  	/* check that BTF function arguments match actual types that the
> @@ -6419,6 +6421,13 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
>  			func_name);
>  		return -EINVAL;
>  	}
> +
> +	if (sleepable && !env->prog->aux->sleepable) {
> +		bpf_log(log, "kernel function %s is sleepable but the program is not\n",
> +			func_name);
> +		return -EINVAL;
> +	}
> +
>  	/* returns argument register number > 0 in case of reference release kfunc */
>  	return rel ? ref_regno : 0;
>  }
> -- 
> 2.25.1
> 

BR, Jarkko