All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Tissoires <bentiss@kernel.org>
To: Alexei Starovoitov <ast@kernel.org>,
	 Daniel Borkmann <daniel@iogearbox.net>,
	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>,
	 John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	 Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
	 Jiri Olsa <jolsa@kernel.org>, Mykola Lysenko <mykolal@fb.com>,
	 Shuah Khan <shuah@kernel.org>
Cc: bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
	 linux-kselftest@vger.kernel.org,
	Benjamin Tissoires <bentiss@kernel.org>
Subject: [PATCH bpf-next 15/18] bpf: wq: add bpf_wq_set_callback_impl
Date: Tue, 16 Apr 2024 16:08:28 +0200	[thread overview]
Message-ID: <20240416-bpf_wq-v1-15-c9e66092f842@kernel.org> (raw)
In-Reply-To: <20240416-bpf_wq-v1-0-c9e66092f842@kernel.org>

In the verifier, we need to teach it that the mentioned callback is
is an async call.
We look at the provided flags and set the callback to be sleepable if
BPF_F_WQ_SLEEPABLE is set.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
 kernel/bpf/helpers.c  | 22 +++++++++++++++++
 kernel/bpf/verifier.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 87 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 9ac1b8bb3a01..e5c8adc44619 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1362,6 +1362,13 @@ static int __bpf_async_set_callback(struct bpf_async_kern *async, void *callback
 		ret = -EINVAL;
 		goto out;
 	}
+	if (type == BPF_ASYNC_TYPE_WQ) {
+		/* check that flags_k is consistent with work->flags */
+		if ((flags ^ cb->flags) & BPF_F_WQ_SLEEPABLE) {
+			ret = -EINVAL;
+			goto out;
+		}
+	}
 	if (!atomic64_read(&cb->map->usercnt)) {
 		/* maps with timers must be either held by user space
 		 * or pinned in bpffs. Otherwise timer might still be
@@ -2721,6 +2728,20 @@ __bpf_kfunc int bpf_wq_init(struct bpf_wq *wq, void *map, unsigned int flags)
 	return __bpf_async_init(async, map, flags, BPF_ASYNC_TYPE_WQ);
 }
 
+__bpf_kfunc int bpf_wq_set_callback_impl(struct bpf_wq *wq,
+					 int (callback_fn)(void *map, int *key, struct bpf_wq *wq),
+					 unsigned int flags__k,
+					 void *aux__ign)
+{
+	struct bpf_prog_aux *aux = (struct bpf_prog_aux *)aux__ign;
+	struct bpf_async_kern *async = (struct bpf_async_kern *)wq;
+
+	if (flags__k & ~BPF_F_WQ_SLEEPABLE)
+		return -EINVAL;
+
+	return __bpf_async_set_callback(async, callback_fn, aux, flags__k, BPF_ASYNC_TYPE_WQ);
+}
+
 __bpf_kfunc_end_defs();
 
 BTF_KFUNCS_START(generic_btf_ids)
@@ -2799,6 +2820,7 @@ BTF_ID_FLAGS(func, bpf_dynptr_size)
 BTF_ID_FLAGS(func, bpf_dynptr_clone)
 BTF_ID_FLAGS(func, bpf_modify_return_test_tp)
 BTF_ID_FLAGS(func, bpf_wq_init)
+BTF_ID_FLAGS(func, bpf_wq_set_callback_impl)
 BTF_KFUNCS_END(common_btf_ids)
 
 static const struct btf_kfunc_id_set common_kfunc_set = {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 6a45d88244c6..947cd544ffab 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -501,8 +501,12 @@ static bool is_dynptr_ref_function(enum bpf_func_id func_id)
 }
 
 static bool is_sync_callback_calling_kfunc(u32 btf_id);
+static bool is_async_callback_calling_kfunc(u32 btf_id);
+static bool is_callback_calling_kfunc(u32 btf_id);
 static bool is_bpf_throw_kfunc(struct bpf_insn *insn);
 
+static bool is_bpf_wq_set_callback_impl_kfunc(u32 btf_id);
+
 static bool is_sync_callback_calling_function(enum bpf_func_id func_id)
 {
 	return func_id == BPF_FUNC_for_each_map_elem ||
@@ -530,7 +534,8 @@ static bool is_sync_callback_calling_insn(struct bpf_insn *insn)
 
 static bool is_async_callback_calling_insn(struct bpf_insn *insn)
 {
-	return bpf_helper_call(insn) && is_async_callback_calling_function(insn->imm);
+	return (bpf_helper_call(insn) && is_async_callback_calling_function(insn->imm)) ||
+	       (bpf_pseudo_kfunc_call(insn) && is_async_callback_calling_kfunc(insn->imm));
 }
 
 static bool is_may_goto_insn(struct bpf_insn *insn)
@@ -9536,7 +9541,7 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins
 	 */
 	env->subprog_info[subprog].is_cb = true;
 	if (bpf_pseudo_kfunc_call(insn) &&
-	    !is_sync_callback_calling_kfunc(insn->imm)) {
+	    !is_callback_calling_kfunc(insn->imm)) {
 		verbose(env, "verifier bug: kfunc %s#%d not marked as callback-calling\n",
 			func_id_name(insn->imm), insn->imm);
 		return -EFAULT;
@@ -9550,7 +9555,7 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins
 	if (is_async_callback_calling_insn(insn)) {
 		struct bpf_verifier_state *async_cb;
 
-		/* there is no real recursion here. timer callbacks are async */
+		/* there is no real recursion here. timer and workqueue callbacks are async */
 		env->subprog_info[subprog].is_async_cb = true;
 		async_cb = push_async_cb(env, env->subprog_info[subprog].start,
 					 insn_idx, subprog, is_sleepable);
@@ -11042,6 +11047,7 @@ enum special_kfunc_type {
 	KF_bpf_throw,
 	KF_bpf_iter_css_task_new,
 	KF_bpf_wq_init,
+	KF_bpf_wq_set_callback_impl,
 };
 
 BTF_SET_START(special_kfunc_set)
@@ -11069,6 +11075,7 @@ BTF_ID(func, bpf_throw)
 BTF_ID(func, bpf_iter_css_task_new)
 #endif
 BTF_ID(func, bpf_wq_init)
+BTF_ID(func, bpf_wq_set_callback_impl)
 BTF_SET_END(special_kfunc_set)
 
 BTF_ID_LIST(special_kfunc_list)
@@ -11100,6 +11107,7 @@ BTF_ID(func, bpf_iter_css_task_new)
 BTF_ID_UNUSED
 #endif
 BTF_ID(func, bpf_wq_init)
+BTF_ID(func, bpf_wq_set_callback_impl)
 
 static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
 {
@@ -11431,12 +11439,28 @@ static bool is_sync_callback_calling_kfunc(u32 btf_id)
 	return btf_id == special_kfunc_list[KF_bpf_rbtree_add_impl];
 }
 
+static bool is_async_callback_calling_kfunc(u32 btf_id)
+{
+	return btf_id == special_kfunc_list[KF_bpf_wq_set_callback_impl];
+}
+
 static bool is_bpf_throw_kfunc(struct bpf_insn *insn)
 {
 	return bpf_pseudo_kfunc_call(insn) && insn->off == 0 &&
 	       insn->imm == special_kfunc_list[KF_bpf_throw];
 }
 
+static bool is_bpf_wq_set_callback_impl_kfunc(u32 btf_id)
+{
+	return btf_id == special_kfunc_list[KF_bpf_wq_set_callback_impl];
+}
+
+static bool is_callback_calling_kfunc(u32 btf_id)
+{
+	return is_sync_callback_calling_kfunc(btf_id) ||
+	       is_async_callback_calling_kfunc(btf_id);
+}
+
 static bool is_rbtree_lock_required_kfunc(u32 btf_id)
 {
 	return is_bpf_rbtree_api_kfunc(btf_id);
@@ -12244,6 +12268,22 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 		}
 	}
 
+	if (is_bpf_wq_set_callback_impl_kfunc(meta.func_id)) {
+		if (!meta.arg_constant.found) {
+			verbose(env, "kfunc %s#%d failed flags verification\n",
+				func_name, meta.func_id);
+			return -EINVAL;
+		}
+		err = push_callback_call(env, insn, insn_idx, meta.subprogno,
+					 !!(meta.arg_constant.value & BPF_F_WQ_SLEEPABLE),
+					 set_timer_callback_state);
+		if (err) {
+			verbose(env, "kfunc %s#%d failed callback verification\n",
+				func_name, meta.func_id);
+			return err;
+		}
+	}
+
 	rcu_lock = is_kfunc_bpf_rcu_read_lock(&meta);
 	rcu_unlock = is_kfunc_bpf_rcu_read_unlock(&meta);
 
@@ -19681,6 +19721,28 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 		   desc->func_id == special_kfunc_list[KF_bpf_rdonly_cast]) {
 		insn_buf[0] = BPF_MOV64_REG(BPF_REG_0, BPF_REG_1);
 		*cnt = 1;
+	} else if (is_bpf_wq_set_callback_impl_kfunc(desc->func_id)) {
+		/* The verifier will process callback_fn as many times as necessary
+		 * with different maps and the register states prepared by
+		 * set_timer_callback_state will be accurate.
+		 *
+		 * The following use case is valid:
+		 *   map1 is shared by prog1, prog2, prog3.
+		 *   prog1 calls bpf_timer_init for some map1 elements
+		 *   prog2 calls bpf_timer_set_callback for some map1 elements.
+		 *     Those that were not bpf_timer_init-ed will return -EINVAL.
+		 *   prog3 calls bpf_timer_start for some map1 elements.
+		 *     Those that were not both bpf_timer_init-ed and
+		 *     bpf_timer_set_callback-ed will return -EINVAL.
+		 */
+		struct bpf_insn ld_addrs[2] = {
+			BPF_LD_IMM64(BPF_REG_4, (long)env->prog->aux),
+		};
+
+		insn_buf[0] = ld_addrs[0];
+		insn_buf[1] = ld_addrs[1];
+		insn_buf[2] = *insn;
+		*cnt = 3;
 	}
 	return 0;
 }

-- 
2.44.0


  parent reply	other threads:[~2024-04-16 14:10 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-16 14:08 [PATCH bpf-next 00/18] Introduce bpf_wq Benjamin Tissoires
2024-04-16 14:08 ` [PATCH bpf-next 01/18] bpf: trampoline: export __bpf_prog_enter/exit_recur Benjamin Tissoires
2024-04-16 14:08 ` [PATCH bpf-next 02/18] bpf: make timer data struct more generic Benjamin Tissoires
2024-04-16 14:08 ` [PATCH bpf-next 03/18] bpf: replace bpf_timer_init with a generic helper Benjamin Tissoires
2024-04-16 14:08 ` [PATCH bpf-next 04/18] bpf: replace bpf_timer_set_callback " Benjamin Tissoires
2024-04-16 14:08 ` [PATCH bpf-next 05/18] bpf: replace bpf_timer_cancel_and_free " Benjamin Tissoires
2024-04-16 14:08 ` [PATCH bpf-next 06/18] bpf: add support for bpf_wq user type Benjamin Tissoires
2024-04-19  6:02   ` Alexei Starovoitov
2024-04-16 14:08 ` [PATCH bpf-next 07/18] tools: sync include/uapi/linux/bpf.h Benjamin Tissoires
2024-04-16 14:08 ` [PATCH bpf-next 08/18] bpf: add support for KF_ARG_PTR_TO_WORKQUEUE Benjamin Tissoires
2024-04-19  6:00   ` Alexei Starovoitov
2024-04-16 14:08 ` [PATCH bpf-next 09/18] bpf: allow struct bpf_wq to be embedded in arraymaps and hashmaps Benjamin Tissoires
2024-04-19  6:05   ` Alexei Starovoitov
2024-04-16 14:08 ` [PATCH bpf-next 10/18] selftests/bpf: add bpf_wq tests Benjamin Tissoires
2024-04-16 14:08 ` [PATCH bpf-next 11/18] bpf: wq: add bpf_wq_init Benjamin Tissoires
2024-04-19  5:25   ` Alexei Starovoitov
2024-04-19 15:12     ` Benjamin Tissoires
2024-04-19 15:34       ` Alexei Starovoitov
2024-04-19 15:55         ` Benjamin Tissoires
2024-04-16 14:08 ` [PATCH bpf-next 12/18] tools: sync include/uapi/linux/bpf.h Benjamin Tissoires
2024-04-16 14:08 ` [PATCH bpf-next 13/18] selftests/bpf: wq: add bpf_wq_init() checks Benjamin Tissoires
2024-04-16 14:08 ` [PATCH bpf-next 14/18] bpf/verifier: add is_sleepable argument to push_callback_call Benjamin Tissoires
2024-04-16 14:08 ` Benjamin Tissoires [this message]
2024-04-16 14:08 ` [PATCH bpf-next 16/18] selftests/bpf: add checks for bpf_wq_set_callback() Benjamin Tissoires
2024-04-18  3:25   ` Song Liu
2024-04-18  8:55     ` Benjamin Tissoires
2024-04-16 14:08 ` [PATCH bpf-next 17/18] bpf: add bpf_wq_start Benjamin Tissoires
2024-04-19  6:18   ` Alexei Starovoitov
2024-04-19 15:14     ` Benjamin Tissoires
2024-04-19 15:49       ` Alexei Starovoitov
2024-04-19 16:01         ` Benjamin Tissoires
2024-04-16 14:08 ` [PATCH bpf-next 18/18] selftests/bpf: wq: add bpf_wq_start() checks Benjamin Tissoires

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240416-bpf_wq-v1-15-c9e66092f842@kernel.org \
    --to=bentiss@kernel.org \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=mykolal@fb.com \
    --cc=sdf@google.com \
    --cc=shuah@kernel.org \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.