[PATCH bpf-next v1 5/7] selftests/bpf: don't report arena tests as passing with an empty body

[Vineet Gupta <vineet.gupta@linux.dev>]

Some arena tests put the __BPF_FEATURE_ADDR_SPACE_CAST guard inside the
function rather than around it, so without arena support the program
still exists, its body is empty, and __success __retval(0) is met by
doing nothing.  A reader sees

  #584/1	 verifier_arena/basic_alloc1_nosleep:OK

and concludes arena works.  That is worse than the tests which vanish
entirely, since nothing hints anything is missing.

The guard was never needed to compile those bodies: without the
feature __arena is still btf_type_tag("arena"), which every compiler
accepts.  What it avoided was the load failure that follows from no
addr_space_cast being emitted — but only for a body that dereferences
an arena pointer.  The arena kfuncs take one as a plain scalar:

  static const struct bpf_reg_types arena_types = {
	  .types = { PTR_TO_ARENA, SCALAR_VALUE }

so a test that only passes pointers to bpf_arena_{alloc,free,reserve}_pages()
needs no cast and runs fine.

test_loader checks skip_reason before it opens anything, so the body no
longer has to be empty to stay loadable: drop all 19 in-body guards.
They do not all need a tag in their place, which is why 19 guards go
and 7 tags arrive:

  6   dereference, so they get __skip_without_arena and report SKIP:
      basic_alloc1{,_nosleep}, basic_alloc2{,_nosleep}, big_alloc1,
      access_reserved
  11  never dereference, so they get no tag and run for real instead
      of reporting a vacuous OK
  2   big_alloc3 and big_alloc4, already inside the file's outer
      whole-program guard, so the in-body one was redundant

The 7th tag goes on check_arena_arg_ret(), which has no guard at all
and is the one verifier_arena subtest that really fails under a stock
GCC.

Measured on bpf-next with a stock bpf-gcc, test_progs before and after:

  verifier_arena/basic_alloc1            OK   -> SKIP
  verifier_arena/basic_alloc1_nosleep    OK   -> SKIP
  verifier_arena/basic_alloc2            OK   -> SKIP
  verifier_arena/basic_alloc2_nosleep    OK   -> SKIP
  verifier_arena_large/big_alloc1        OK   -> SKIP
  verifier_arena_large/access_reserved   OK   -> SKIP
  verifier_arena/check_arena_arg_ret     FAIL -> SKIP

the six that dereference stop claiming to pass, and check_arena_arg_ret
stops failing.  The eleven that do not dereference stay OK, having gone
from an empty body to actually running.

Removing the guards exposes the arena_base() calls they were hiding.
The previous patch made that macro opaque to GCC's -Warray-bounds, so
nothing is needed here beyond the guards going away.

Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
---
 .../selftests/bpf/progs/verifier_arena.c      | 31 +++----------------
 .../bpf/progs/verifier_arena_large.c          | 16 ++--------
 2 files changed, 7 insertions(+), 40 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/verifier_arena.c b/tools/testing/selftests/bpf/progs/verifier_arena.c
index 3b3410802825..21b66d51c61e 100644
--- a/tools/testing/selftests/bpf/progs/verifier_arena.c
+++ b/tools/testing/selftests/bpf/progs/verifier_arena.c
@@ -27,9 +27,9 @@ struct {
 
 SEC("socket")
 __success __retval(0)
+__skip_without_arena
 int basic_alloc1_nosleep(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	volatile int __arena *page1, *page2, *no_page;
 
 	page1 = bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
@@ -52,15 +52,14 @@ int basic_alloc1_nosleep(void *ctx)
 		return 6;
 	if (*page2 != 0 && *page2 != 2) /* use-after-free should return 0 or the stored value */
 		return 7;
-#endif
 	return 0;
 }
 
 SEC("syscall")
 __success __retval(0)
+__skip_without_arena
 int basic_alloc1(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	volatile int __arena *page1, *page2, *no_page, *page3;
 
 	page1 = bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
@@ -91,7 +90,6 @@ int basic_alloc1(void *ctx)
 		return 9;
 	if (*page1 != 1)
 		return 10;
-#endif
 	return 0;
 }
 
@@ -125,9 +123,9 @@ int free_scalar_below_arena(void *ctx)
 
 SEC("socket")
 __success __retval(0)
+__skip_without_arena
 int basic_alloc2_nosleep(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	volatile char __arena *page1, *page2, *page3, *page4;
 
 	page1 = bpf_arena_alloc_pages(&arena, NULL, 2, NUMA_NO_NODE, 0);
@@ -157,15 +155,14 @@ int basic_alloc2_nosleep(void *ctx)
 		return 7;
 	if (*page4 != 0)
 		return 8;
-#endif
 	return 0;
 }
 
 SEC("syscall")
 __success __retval(0)
+__skip_without_arena
 int basic_alloc2(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	volatile char __arena *page1, *page2, *page3, *page4;
 
 	page1 = bpf_arena_alloc_pages(&arena, NULL, 2, NUMA_NO_NODE, 0);
@@ -195,7 +192,6 @@ int basic_alloc2(void *ctx)
 		return 7;
 	if (*page4 != 0)
 		return 8;
-#endif
 	return 0;
 }
 
@@ -233,7 +229,6 @@ SEC("socket")
 __success __retval(0)
 int basic_reserve1_nosleep(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	char __arena *page;
 	int ret;
 
@@ -257,7 +252,6 @@ int basic_reserve1_nosleep(void *ctx)
 	page = bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
 	if (page)
 		return 4;
-#endif
 	return 0;
 }
 
@@ -265,7 +259,6 @@ SEC("syscall")
 __success __retval(0)
 int basic_reserve1(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	char __arena *page;
 	int ret;
 
@@ -289,7 +282,6 @@ int basic_reserve1(void *ctx)
 	page = bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
 	if (page)
 		return 4;
-#endif
 	return 0;
 }
 
@@ -297,7 +289,6 @@ SEC("socket")
 __success __retval(0)
 int basic_reserve2_nosleep(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	char __arena *page;
 	int ret;
 
@@ -309,7 +300,6 @@ int basic_reserve2_nosleep(void *ctx)
 	page = bpf_arena_alloc_pages(&arena, page, 1, NUMA_NO_NODE, 0);
 	if ((u64)page)
 		return 2;
-#endif
 	return 0;
 }
 
@@ -317,7 +307,6 @@ SEC("syscall")
 __success __retval(0)
 int basic_reserve2(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	char __arena *page;
 	int ret;
 
@@ -329,7 +318,6 @@ int basic_reserve2(void *ctx)
 	page = bpf_arena_alloc_pages(&arena, page, 1, NUMA_NO_NODE, 0);
 	if ((u64)page)
 		return 2;
-#endif
 	return 0;
 }
 
@@ -338,7 +326,6 @@ SEC("socket")
 __success __retval(0)
 int reserve_twice_nosleep(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	char __arena *page;
 	int ret;
 
@@ -351,7 +338,6 @@ int reserve_twice_nosleep(void *ctx)
 	ret = bpf_arena_reserve_pages(&arena, page, 1);
 	if (ret != -EBUSY)
 		return 2;
-#endif
 	return 0;
 }
 
@@ -359,7 +345,6 @@ SEC("syscall")
 __success __retval(0)
 int reserve_twice(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	char __arena *page;
 	int ret;
 
@@ -372,7 +357,6 @@ int reserve_twice(void *ctx)
 	ret = bpf_arena_reserve_pages(&arena, page, 1);
 	if (ret != -EBUSY)
 		return 2;
-#endif
 	return 0;
 }
 
@@ -381,7 +365,6 @@ SEC("socket")
 __success __retval(0)
 int reserve_invalid_region_nosleep(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	char __arena *page;
 	int ret;
 
@@ -403,7 +386,6 @@ int reserve_invalid_region_nosleep(void *ctx)
 	ret = bpf_arena_reserve_pages(&arena, page, (1ULL << 32) - 1);
 	if (ret != -EINVAL)
 		return 4;
-#endif
 	return 0;
 }
 
@@ -411,7 +393,6 @@ SEC("syscall")
 __success __retval(0)
 int reserve_invalid_region(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	char __arena *page;
 	int ret;
 
@@ -433,7 +414,6 @@ int reserve_invalid_region(void *ctx)
 	ret = bpf_arena_reserve_pages(&arena, page, (1ULL << 32) - 1);
 	if (ret != -EINVAL)
 		return 4;
-#endif
 	return 0;
 }
 
@@ -478,7 +458,6 @@ SEC("syscall")
 __success __retval(0)
 int arena_kfuncs_under_bpf_lock(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	char __arena *page;
 	int ret;
 
@@ -503,7 +482,6 @@ int arena_kfuncs_under_bpf_lock(void *ctx)
 	bpf_arena_free_pages(&arena, page, 1);
 
 	bpf_spin_unlock(&arena_bpf_test_lock);
-#endif
 
 	return 0;
 }
@@ -765,6 +743,7 @@ u32 __arena volatile *check_arena_arg_quals2(u32 volatile __arena *arg1, u32 __a
 
 SEC("syscall")
 __success __retval(0)
+__skip_without_arena
 int check_arena_arg_ret(void *ctx)
 {
 	u32 __arena *page = bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
diff --git a/tools/testing/selftests/bpf/progs/verifier_arena_large.c b/tools/testing/selftests/bpf/progs/verifier_arena_large.c
index 34f0a0f88e12..6ef99793cac1 100644
--- a/tools/testing/selftests/bpf/progs/verifier_arena_large.c
+++ b/tools/testing/selftests/bpf/progs/verifier_arena_large.c
@@ -22,9 +22,9 @@ struct {
 
 SEC("syscall")
 __success __retval(0)
+__skip_without_arena
 int big_alloc1(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	volatile char __arena *page1, *page2, *no_page, *page3;
 	u64 base;
 
@@ -79,16 +79,15 @@ int big_alloc1(void *ctx)
 		return 13;
 	if (*(page2 - PAGE_SIZE) != 0)
 		return 14;
-#endif
 	return 0;
 }
 
 /* Try to access a reserved page. Behavior should be identical with accessing unallocated pages. */
 SEC("syscall")
 __success __retval(0)
+__skip_without_arena
 int access_reserved(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	volatile char __arena *page;
 	char __arena *base;
 	const size_t len = 4;
@@ -118,7 +117,6 @@ int access_reserved(void *ctx)
 		if (*page)
 			return 2 + 2 * i + 1;
 	}
-#endif
 	return 0;
 }
 
@@ -127,7 +125,6 @@ SEC("syscall")
 __success __retval(0)
 int request_partially_reserved(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	volatile char __arena *page;
 	char __arena *base;
 	int ret;
@@ -142,7 +139,6 @@ int request_partially_reserved(void *ctx)
 	page = bpf_arena_alloc_pages(&arena, base, 5, NUMA_NO_NODE, 0);
 	if ((u64)page != 0ULL)
 		return 2;
-#endif
 	return 0;
 }
 
@@ -150,7 +146,6 @@ SEC("syscall")
 __success __retval(0)
 int free_reserved(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	char __arena *addr;
 	char __arena *page;
 	int ret;
@@ -177,7 +172,6 @@ int free_reserved(void *ctx)
 	page = bpf_arena_alloc_pages(&arena, addr + __PAGE_SIZE, 2, NUMA_NO_NODE, 0);
 	if (!page)
 		return 3;
-#endif
 	return 0;
 }
 
@@ -292,7 +286,6 @@ SEC("socket")
 __success __retval(0)
 int big_alloc3(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	char __arena *pages;
 	u64 i;
 	int err = 0;
@@ -314,8 +307,6 @@ int big_alloc3(void *ctx)
 
 	bpf_arena_free_pages(&arena, pages, LARGE_PAGE_CNT);
 	return err;
-#endif
-	return 0;
 }
 
 /* SYSCALL programs are always sleepable. */
@@ -323,7 +314,6 @@ SEC("syscall")
 __success __retval(0)
 int big_alloc4(void *ctx)
 {
-#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 	char __arena *pages;
 	u64 i;
 	int err = 0;
@@ -345,8 +335,6 @@ int big_alloc4(void *ctx)
 
 	bpf_arena_free_pages(&arena, pages, LARGE_PAGE_CNT);
 	return err;
-#endif
-	return 0;
 }
 #else
 
-- 
2.53.0-Meta