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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YvKRYRjJdXbAWL6Y@kernel.org>
Date:   Tue, 9 Aug 2022 19:54:57 +0300
From:   Jarkko Sakkinen <jarkko@...nel.org>
To:     Roberto Sassu <roberto.sassu@...wei.com>
Cc:     ast@...nel.org, daniel@...earbox.net, andrii@...nel.org,
        martin.lau@...ux.dev, song@...nel.org, yhs@...com,
        john.fastabend@...il.com, kpsingh@...nel.org, sdf@...gle.com,
        haoluo@...gle.com, jolsa@...nel.org, corbet@....net,
        dhowells@...hat.com, rostedt@...dmis.org, mingo@...hat.com,
        paul@...l-moore.com, jmorris@...ei.org, serge@...lyn.com,
        shuah@...nel.org, bpf@...r.kernel.org, linux-doc@...r.kernel.org,
        keyrings@...r.kernel.org, linux-security-module@...r.kernel.org,
        linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        Yosry Ahmed <yosryahmed@...gle.com>
Subject: Re: [PATCH v9 01/10] btf: Add a new kfunc flag which allows to mark
 a function to be sleepable

On Tue, Aug 09, 2022 at 03:45:54PM +0200, Roberto Sassu wrote:
> From: Benjamin Tissoires <benjamin.tissoires@...hat.com>
> 
> From: Benjamin Tissoires <benjamin.tissoires@...hat.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@...hat.com>
> Co-developed-by: Yosry Ahmed <yosryahmed@...gle.com>
> Signed-off-by: Yosry Ahmed <yosryahmed@...gle.com>
> Signed-off-by: Hao Luo <haoluo@...gle.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