[PATCH bpf v2 7/7] selftests/bpf: Check callback map value lock identity

[Kumar Kartikeya Dwivedi <memxor@gmail.com>]

Add a verifier test which retains a map value from an outer callback and
then acquires a lock through an inner callback value before attempting to
release the outer callback value. Both values can denote different elements,
so the verifier must reject the mismatched unlock.

Also exercise two distinct one-element inner arrays. Their concrete map
instances share inner-map metadata, but their callback values must retain
distinct lock identities.

Keep a nested same-element lock/unlock program as a positive control. This
ensures assigning fresh identities to callback map values does not reject
balanced locking through one callback argument.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../bpf/progs/verifier_callback_lock.c        | 121 ++++++++++++++++++
 2 files changed, 123 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_callback_lock.c

diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index 64ac49ad67e6..5c572dd725e7 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -25,6 +25,7 @@
 #include "verifier_btf_ctx_access.skel.h"
 #include "verifier_btf_unreliable_prog.skel.h"
 #include "verifier_call_large_imm.skel.h"
+#include "verifier_callback_lock.skel.h"
 #include "verifier_cfg.skel.h"
 #include "verifier_cgroup_inv_retcode.skel.h"
 #include "verifier_cgroup_skb.skel.h"
@@ -188,6 +189,7 @@ void test_verifier_bswap(void)                { RUN(verifier_bswap); }
 void test_verifier_btf_ctx_access(void)       { RUN(verifier_btf_ctx_access); }
 void test_verifier_btf_unreliable_prog(void)  { RUN(verifier_btf_unreliable_prog); }
 void test_verifier_call_large_imm(void)       { RUN(verifier_call_large_imm); }
+void test_verifier_callback_lock(void)        { RUN(verifier_callback_lock); }
 void test_verifier_cfg(void)                  { RUN(verifier_cfg); }
 void test_verifier_cgroup_inv_retcode(void)   { RUN(verifier_cgroup_inv_retcode); }
 void test_verifier_cgroup_skb(void)           { RUN(verifier_cgroup_skb); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_callback_lock.c b/tools/testing/selftests/bpf/progs/verifier_callback_lock.c
new file mode 100644
index 000000000000..d23e13908ceb
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_callback_lock.c
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+struct bpf_map;
+
+struct lock_value {
+	struct bpf_spin_lock lock;
+};
+
+struct inner_lock_map {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 1);
+	__type(key, int);
+	__type(value, struct lock_value);
+} inner_lock_map_a SEC(".maps"), inner_lock_map_b SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
+	__uint(max_entries, 2);
+	__type(key, int);
+	__array(values, struct inner_lock_map);
+} lock_map_of_maps SEC(".maps") = {
+	.values = {
+		[0] = &inner_lock_map_a,
+		[1] = &inner_lock_map_b,
+	},
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 2);
+	__type(key, int);
+	__type(value, struct lock_value);
+} lock_map SEC(".maps");
+
+struct callback_ctx {
+	struct lock_value *value;
+};
+
+static long lock_different_value(struct bpf_map *map, int *key,
+				 struct lock_value *value, struct callback_ctx *ctx)
+{
+	bpf_spin_lock(&value->lock);
+	bpf_spin_unlock(&ctx->value->lock);
+	return 0;
+}
+
+static long nest_lock_different_value(struct bpf_map *map, int *key,
+				      struct lock_value *value, void *data)
+{
+	struct callback_ctx ctx = { .value = value };
+
+	bpf_for_each_map_elem(&lock_map, lock_different_value, &ctx, 0);
+	return 0;
+}
+
+SEC("?tc")
+__description("callback map value has a distinct lock identity")
+__failure __msg("bpf_spin_unlock of different lock")
+int callback_value_lock_identity(void *ctx)
+{
+	bpf_for_each_map_elem(&lock_map, nest_lock_different_value, NULL, 0);
+	return 0;
+}
+
+static long nest_lock_different_inner_value(struct bpf_map *map, int *key,
+					    struct lock_value *value, void *data)
+{
+	struct callback_ctx ctx = { .value = value };
+	int inner_key = 1;
+	void *inner_map;
+
+	inner_map = bpf_map_lookup_elem(&lock_map_of_maps, &inner_key);
+	if (!inner_map)
+		return 0;
+	bpf_for_each_map_elem(inner_map, lock_different_value, &ctx, 0);
+	return 0;
+}
+
+SEC("?tc")
+__description("distinct one-element inner maps have distinct lock identities")
+__failure __msg("bpf_spin_unlock of different lock")
+int callback_inner_map_value_lock_identity(void *ctx)
+{
+	int inner_key = 0;
+	void *inner_map;
+
+	inner_map = bpf_map_lookup_elem(&lock_map_of_maps, &inner_key);
+	if (!inner_map)
+		return 0;
+	bpf_for_each_map_elem(inner_map, nest_lock_different_inner_value, NULL, 0);
+	return 0;
+}
+
+static long lock_same_value(struct bpf_map *map, int *key,
+			    struct lock_value *value, void *data)
+{
+	bpf_spin_lock(&value->lock);
+	bpf_spin_unlock(&value->lock);
+	return 0;
+}
+
+static long nest_lock_same_value(struct bpf_map *map, int *key,
+				 struct lock_value *value, void *data)
+{
+	bpf_for_each_map_elem(&lock_map, lock_same_value, NULL, 0);
+	return 0;
+}
+
+SEC("?tc")
+__description("nested callback can lock its own map value")
+__success
+int callback_value_lock_identity_same(void *ctx)
+{
+	bpf_for_each_map_elem(&lock_map, nest_lock_same_value, NULL, 0);
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.53.0