Merge branch 'fix __retval() being always ignored'

Eduard Zingerman says:

====================

Florian Westphal found a bug in test_loader.c processing of __retval tag.
Because of this bug the function test_loader.c:do_prog_test_run()
never executed and all __retval test tags were ignored. See [1].

Fix for this bug uncovers two additional bugs:
- During test_verifier tests migration to inline assembly (see [2])
  I missed the fact that some tests require maps to contain mock values;
- Some issue with a new refcounted_kptr test, which causes kernel to
  produce dead lock and refcount saturation warnings when subject to
  libbpf's bpf_test_run_opts().

This series fixes the bug in __retval() processing, and address the
issue with test maps not being populated. The issue in refcounted_kptr
is not addressed, __retval tags in those tests are commented out.

I found that the following tests depend on test maps being populated:
- progs/verifier_array_access.c
- verifier/value_ptr_arith.c (planned for migration to inline assembly)

Given the small amount of these tests I decided to opt for simple
non-generic solution (see patch #4).

[1] https://lore.kernel.org/bpf/f4c4aee644425842ee6aa8edf1da68f0a8260e7c.camel@gmail.com/T/
[2] https://lore.kernel.org/bpf/20230325025524.144043-1-eddyz87@gmail.com/
====================
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

tools/testing/selftests/bpf/prog_tests/verifier.c

@@ -44,8 +44,17 @@
 #include "verifier_xdp.skel.h"
 #include "verifier_xdp_direct_packet_access.skel.h"
 
+#define MAX_ENTRIES 11
+
+struct test_val {
+	unsigned int index;
+	int foo[MAX_ENTRIES];
+};
+
 __maybe_unused
-static void run_tests_aux(const char *skel_name, skel_elf_bytes_fn elf_bytes_factory)
+static void run_tests_aux(const char *skel_name,
+			  skel_elf_bytes_fn elf_bytes_factory,
+			  pre_execution_cb pre_execution_cb)
 {
 	struct test_loader tester = {};
 	__u64 old_caps;
@@ -58,6 +67,7 @@ static void run_tests_aux(const char *skel_name, skel_elf_bytes_fn elf_bytes_fac
 		return;
 	}
 
+	test_loader__set_pre_execution_cb(&tester, pre_execution_cb);
 	test_loader__run_subtests(&tester, skel_name, elf_bytes_factory);
 	test_loader_fini(&tester);
 
@@ -66,10 +76,9 @@ static void run_tests_aux(const char *skel_name, skel_elf_bytes_fn elf_bytes_fac
 		PRINT_FAIL("failed to restore CAP_SYS_ADMIN: %i, %s\n", err, strerror(err));
 }
 
-#define RUN(skel) run_tests_aux(#skel, skel##__elf_bytes)
+#define RUN(skel) run_tests_aux(#skel, skel##__elf_bytes, NULL)
 
 void test_verifier_and(void)                  { RUN(verifier_and); }
-void test_verifier_array_access(void)         { RUN(verifier_array_access); }
 void test_verifier_basic_stack(void)          { RUN(verifier_basic_stack); }
 void test_verifier_bounds_deduction(void)     { RUN(verifier_bounds_deduction); }
 void test_verifier_bounds_deduction_non_const(void)     { RUN(verifier_bounds_deduction_non_const); }
@@ -108,3 +117,30 @@ void test_verifier_var_off(void)              { RUN(verifier_var_off); }
 void test_verifier_xadd(void)                 { RUN(verifier_xadd); }
 void test_verifier_xdp(void)                  { RUN(verifier_xdp); }
 void test_verifier_xdp_direct_packet_access(void) { RUN(verifier_xdp_direct_packet_access); }
+
+static int init_array_access_maps(struct bpf_object *obj)
+{
+	struct bpf_map *array_ro;
+	struct test_val value = {
+		.index = (6 + 1) * sizeof(int),
+		.foo[6] = 0xabcdef12,
+	};
+	int err, key = 0;
+
+	array_ro = bpf_object__find_map_by_name(obj, "map_array_ro");
+	if (!ASSERT_OK_PTR(array_ro, "lookup map_array_ro"))
+		return -EINVAL;
+
+	err = bpf_map_update_elem(bpf_map__fd(array_ro), &key, &value, 0);
+	if (!ASSERT_OK(err, "map_array_ro update"))
+		return err;
+
+	return 0;
+}
+
+void test_verifier_array_access(void)
+{
+	run_tests_aux("verifier_array_access",
+		      verifier_array_access__elf_bytes,
+		      init_array_access_maps);
+}

tools/testing/selftests/bpf/progs/refcounted_kptr.c

@@ -219,7 +219,7 @@ static long __read_from_unstash(int idx)
 #define INSERT_READ_BOTH(rem_tree, rem_list, desc)			\
 SEC("tc")								\
 __description(desc)							\
-__success __retval(579)							\
+__success /* __retval(579) temporarily disabled */			\
 long insert_and_remove_tree_##rem_tree##_list_##rem_list(void *ctx)	\
 {									\
 	long err, tree_data, list_data;					\
@@ -258,7 +258,7 @@ INSERT_READ_BOTH(false, true, "insert_read_both: remove from list");
 #define INSERT_READ_BOTH(rem_tree, rem_list, desc)			\
 SEC("tc")								\
 __description(desc)							\
-__success __retval(579)							\
+__success /* __retval(579) temporarily disabled */			\
 long insert_and_remove_lf_tree_##rem_tree##_list_##rem_list(void *ctx)	\
 {									\
 	long err, tree_data, list_data;					\
@@ -296,7 +296,7 @@ INSERT_READ_BOTH(false, true, "insert_read_both_list_first: remove from list");
 #define INSERT_DOUBLE_READ_AND_DEL(read_fn, read_root, desc)		\
 SEC("tc")								\
 __description(desc)							\
-__success __retval(-1)							\
+__success /* temporarily __retval(-1) disabled */			\
 long insert_double_##read_fn##_and_del_##read_root(void *ctx)		\
 {									\
 	long err, list_data;						\
@@ -329,7 +329,7 @@ INSERT_DOUBLE_READ_AND_DEL(__read_from_list, head, "insert_double_del: 2x read-a
 #define INSERT_STASH_READ(rem_tree, desc)				\
 SEC("tc")								\
 __description(desc)							\
-__success __retval(84)							\
+__success /* __retval(84) temporarily disabled */			\
 long insert_rbtree_and_stash__del_tree_##rem_tree(void *ctx)		\
 {									\
 	long err, tree_data, map_data;					\

tools/testing/selftests/bpf/test_loader.c

@@ -587,9 +587,17 @@ void run_subtest(struct test_loader *tester,
 		/* For some reason test_verifier executes programs
 		 * with all capabilities restored. Do the same here.
 		 */
-		if (!restore_capabilities(&caps))
+		if (restore_capabilities(&caps))
 			goto tobj_cleanup;
 
+		if (tester->pre_execution_cb) {
+			err = tester->pre_execution_cb(tobj);
+			if (err) {
+				PRINT_FAIL("pre_execution_cb failed: %d\n", err);
+				goto tobj_cleanup;
+			}
+		}
+
 		do_prog_test_run(bpf_program__fd(tprog), &retval);
 		if (retval != subspec->retval && subspec->retval != POINTER_VALUE) {
 			PRINT_FAIL("Unexpected retval: %d != %d\n", retval, subspec->retval);

tools/testing/selftests/bpf/test_progs.h

@@ -424,14 +424,23 @@ int get_bpf_max_tramp_links(void);
 
 #define BPF_TESTMOD_TEST_FILE "/sys/kernel/bpf_testmod"
 
+typedef int (*pre_execution_cb)(struct bpf_object *obj);
+
 struct test_loader {
 	char *log_buf;
 	size_t log_buf_sz;
 	size_t next_match_pos;
+	pre_execution_cb pre_execution_cb;
 
 	struct bpf_object *obj;
 };
 
+static inline void test_loader__set_pre_execution_cb(struct test_loader *tester,
+						     pre_execution_cb cb)
+{
+	tester->pre_execution_cb = cb;
+}
+
 typedef const void *(*skel_elf_bytes_fn)(size_t *sz);
 
 extern void test_loader__run_subtests(struct test_loader *tester,