Received-SPF: pass (google.com: domain of linux-kernel+bounces-153650-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Subject: Re: [PATCH] bpf: verifier: allow arrays of progs to be used in
 sleepable context
To: Benjamin Tissoires <bentiss@kernel.org>,
 Alexei Starovoitov <ast@kernel.org>,
 John Fastabend <john.fastabend@gmail.com>,
 Andrii Nakryiko <andrii@kernel.org>, Martin KaFai Lau
 <martin.lau@linux.dev>, Eduard Zingerman <eddyz87@gmail.com>,
 Song Liu <song@kernel.org>, Yonghong Song <yonghong.song@linux.dev>,
 KP Singh <kpsingh@kernel.org>, Stanislav Fomichev <sdf@google.com>,
 Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>
Cc: bpf@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20240422-sleepable_array_progs-v1-1-7c46ccbaa6e2@kernel.org>
From: Daniel Borkmann <daniel@iogearbox.net>
Message-ID: <7344022a-6f59-7cbf-ee45-6b7d59114be6@iogearbox.net>
Date: Mon, 22 Apr 2024 17:44:24 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <20240422-sleepable_array_progs-v1-1-7c46ccbaa6e2@kernel.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit

On 4/22/24 9:16 AM, Benjamin Tissoires wrote:
> Arrays of progs are underlying using regular arrays, but they can only
> be updated from a syscall.
> Therefore, they should be safe to use while in a sleepable context.
> 
> This is required to be able to call bpf_tail_call() from a sleepable
> tracing bpf program.
> 
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
> Hi,
> 
> a small patch to allow to have:
> 
> ```
> SEC("fmod_ret.s/__hid_bpf_tail_call_sleepable")
> int BPF_PROG(hid_tail_call_sleepable, struct hid_bpf_ctx *hctx)
> {
> 	bpf_tail_call(ctx, &hid_jmp_table, hctx->index);
> 
> 	return 0;
> }
> ```
> 
> This should allow me to add bpf hooks to functions that communicate with
> the hardware.

Could you also add selftests to it? In particular, I'm thinking that this is not
sufficient given also bpf_prog_map_compatible() needs to be extended to check on
prog->sleepable. For example we would need to disallow calling sleepable programs
in that map from non-sleepable context.

>   kernel/bpf/verifier.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 68cfd6fc6ad4..880b32795136 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -18171,6 +18171,7 @@ static int check_map_prog_compatibility(struct bpf_verifier_env *env,
>   		case BPF_MAP_TYPE_QUEUE:
>   		case BPF_MAP_TYPE_STACK:
>   		case BPF_MAP_TYPE_ARENA:
> +		case BPF_MAP_TYPE_PROG_ARRAY:
>   			break;
>   		default:
>   			verbose(env,
> 
> ---
> base-commit: 735f5b8a7ccf383e50d76f7d1c25769eee474812
> change-id: 20240422-sleepable_array_progs-e0c07b17cabb
> 
> Best regards,
>