aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlexei Starovoitov <ast@kernel.org>2025-01-06 10:59:50 -0800
committerAlexei Starovoitov <ast@kernel.org>2025-01-06 10:59:56 -0800
commit859a7db4c7db84b06ca7fc9c60eb45a130c8fbec (patch)
treeed16b336308fe7e06470331a87eb492bfd7c9de0
parentbpf/tests: Add 32 bits only long conditional jump tests (diff)
parentselftests/bpf: test bpf_for within spin lock section (diff)
downloadlinux-rng-859a7db4c7db84b06ca7fc9c60eb45a130c8fbec.tar.xz
linux-rng-859a7db4c7db84b06ca7fc9c60eb45a130c8fbec.zip
Merge branch 'bpf-allow-bpf_for-bpf_repeat-while-holding-spin'
Emil Tsalapatis says: ==================== In BPF programs, kfunc calls while holding a lock are not allowed because kfuncs may sleep by default. The exception to this rule are the functions in special_kfunc_list, which are guaranteed to not sleep. The bpf_iter_num_* functions used by the bpf_for and bpf_repeat macros make no function calls themselves, and as such are guaranteed to not sleep. Add them to special_kfunc_list to allow them within BPF spinlock critical sections. Signed-off-by: Emil Tsalapatis (Meta) <emil@etsalapatis.com> ==================== Link: https://patch.msgid.link/20250104202528.882482-1-emil@etsalapatis.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
-rw-r--r--kernel/bpf/verifier.c20
-rw-r--r--tools/testing/selftests/bpf/progs/verifier_spin_lock.c26
2 files changed, 45 insertions, 1 deletions
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d77abb87ffb1..b8ca227c78af 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11690,6 +11690,9 @@ enum special_kfunc_type {
KF_bpf_get_kmem_cache,
KF_bpf_local_irq_save,
KF_bpf_local_irq_restore,
+ KF_bpf_iter_num_new,
+ KF_bpf_iter_num_next,
+ KF_bpf_iter_num_destroy,
};
BTF_SET_START(special_kfunc_set)
@@ -11765,6 +11768,9 @@ BTF_ID_UNUSED
BTF_ID(func, bpf_get_kmem_cache)
BTF_ID(func, bpf_local_irq_save)
BTF_ID(func, bpf_local_irq_restore)
+BTF_ID(func, bpf_iter_num_new)
+BTF_ID(func, bpf_iter_num_next)
+BTF_ID(func, bpf_iter_num_destroy)
static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
{
@@ -12151,12 +12157,24 @@ static bool is_bpf_rbtree_api_kfunc(u32 btf_id)
btf_id == special_kfunc_list[KF_bpf_rbtree_first];
}
+static bool is_bpf_iter_num_api_kfunc(u32 btf_id)
+{
+ return btf_id == special_kfunc_list[KF_bpf_iter_num_new] ||
+ btf_id == special_kfunc_list[KF_bpf_iter_num_next] ||
+ btf_id == special_kfunc_list[KF_bpf_iter_num_destroy];
+}
+
static bool is_bpf_graph_api_kfunc(u32 btf_id)
{
return is_bpf_list_api_kfunc(btf_id) || is_bpf_rbtree_api_kfunc(btf_id) ||
btf_id == special_kfunc_list[KF_bpf_refcount_acquire_impl];
}
+static bool kfunc_spin_allowed(u32 btf_id)
+{
+ return is_bpf_graph_api_kfunc(btf_id) || is_bpf_iter_num_api_kfunc(btf_id);
+}
+
static bool is_sync_callback_calling_kfunc(u32 btf_id)
{
return btf_id == special_kfunc_list[KF_bpf_rbtree_add_impl];
@@ -19048,7 +19066,7 @@ static int do_check(struct bpf_verifier_env *env)
if (env->cur_state->active_locks) {
if ((insn->src_reg == BPF_REG_0 && insn->imm != BPF_FUNC_spin_unlock) ||
(insn->src_reg == BPF_PSEUDO_KFUNC_CALL &&
- (insn->off != 0 || !is_bpf_graph_api_kfunc(insn->imm)))) {
+ (insn->off != 0 || !kfunc_spin_allowed(insn->imm)))) {
verbose(env, "function calls are not allowed while holding a lock\n");
return -EINVAL;
}
diff --git a/tools/testing/selftests/bpf/progs/verifier_spin_lock.c b/tools/testing/selftests/bpf/progs/verifier_spin_lock.c
index 25599eac9a70..d9d7b05cf6d2 100644
--- a/tools/testing/selftests/bpf/progs/verifier_spin_lock.c
+++ b/tools/testing/selftests/bpf/progs/verifier_spin_lock.c
@@ -530,4 +530,30 @@ l1_%=: exit; \
: __clobber_all);
}
+SEC("tc")
+__description("spin_lock: loop within a locked region")
+__success __failure_unpriv __msg_unpriv("")
+__retval(0)
+int bpf_loop_inside_locked_region(void)
+{
+ const int zero = 0;
+ struct val *val;
+ int i, j = 0;
+
+ val = bpf_map_lookup_elem(&map_spin_lock, &zero);
+ if (!val)
+ return -1;
+
+ bpf_spin_lock(&val->l);
+ bpf_for(i, 0, 10) {
+ j++;
+ /* Silence "unused variable" warnings. */
+ if (j == 10)
+ break;
+ }
+ bpf_spin_unlock(&val->l);
+
+ return 0;
+}
+
char _license[] SEC("license") = "GPL";