selftests/bpf: tests with delayed read/precision makrs in loop body
authorEduard Zingerman <eddyz87@gmail.com>
Tue, 24 Oct 2023 00:09:14 +0000 (03:09 +0300)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 1 Feb 2024 00:18:58 +0000 (16:18 -0800)
commit 389ede06c2974b2f878a7ebff6b0f4f707f9db74 upstream.

These test cases try to hide read and precision marks from loop
convergence logic: marks would only be assigned on subsequent loop
iterations or after exploring states pushed to env->head stack first.
Without verifier fix to use exact states comparison logic for
iterators convergence these tests (except 'triple_continue') would be
errorneously marked as safe.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20231024000917.12153-5-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tools/testing/selftests/bpf/progs/iters.c

index 6b9b3c5..764a684 100644 (file)
@@ -14,6 +14,13 @@ int my_pid;
 int arr[256];
 int small_arr[16] SEC(".data.small_arr");
 
+struct {
+       __uint(type, BPF_MAP_TYPE_HASH);
+       __uint(max_entries, 10);
+       __type(key, int);
+       __type(value, int);
+} amap SEC(".maps");
+
 #ifdef REAL_TEST
 #define MY_PID_GUARD() if (my_pid != (bpf_get_current_pid_tgid() >> 32)) return 0
 #else
@@ -716,4 +723,515 @@ int iter_pass_iter_ptr_to_subprog(const void *ctx)
        return 0;
 }
 
+SEC("?raw_tp")
+__failure
+__msg("R1 type=scalar expected=fp")
+__naked int delayed_read_mark(void)
+{
+       /* This is equivalent to C program below.
+        * The call to bpf_iter_num_next() is reachable with r7 values &fp[-16] and 0xdead.
+        * State with r7=&fp[-16] is visited first and follows r6 != 42 ... continue branch.
+        * At this point iterator next() call is reached with r7 that has no read mark.
+        * Loop body with r7=0xdead would only be visited if verifier would decide to continue
+        * with second loop iteration. Absence of read mark on r7 might affect state
+        * equivalent logic used for iterator convergence tracking.
+        *
+        * r7 = &fp[-16]
+        * fp[-16] = 0
+        * r6 = bpf_get_prandom_u32()
+        * bpf_iter_num_new(&fp[-8], 0, 10)
+        * while (bpf_iter_num_next(&fp[-8])) {
+        *   r6++
+        *   if (r6 != 42) {
+        *     r7 = 0xdead
+        *     continue;
+        *   }
+        *   bpf_probe_read_user(r7, 8, 0xdeadbeef); // this is not safe
+        * }
+        * bpf_iter_num_destroy(&fp[-8])
+        * return 0
+        */
+       asm volatile (
+               "r7 = r10;"
+               "r7 += -16;"
+               "r0 = 0;"
+               "*(u64 *)(r7 + 0) = r0;"
+               "call %[bpf_get_prandom_u32];"
+               "r6 = r0;"
+               "r1 = r10;"
+               "r1 += -8;"
+               "r2 = 0;"
+               "r3 = 10;"
+               "call %[bpf_iter_num_new];"
+       "1:"
+               "r1 = r10;"
+               "r1 += -8;"
+               "call %[bpf_iter_num_next];"
+               "if r0 == 0 goto 2f;"
+               "r6 += 1;"
+               "if r6 != 42 goto 3f;"
+               "r7 = 0xdead;"
+               "goto 1b;"
+       "3:"
+               "r1 = r7;"
+               "r2 = 8;"
+               "r3 = 0xdeadbeef;"
+               "call %[bpf_probe_read_user];"
+               "goto 1b;"
+       "2:"
+               "r1 = r10;"
+               "r1 += -8;"
+               "call %[bpf_iter_num_destroy];"
+               "r0 = 0;"
+               "exit;"
+               :
+               : __imm(bpf_get_prandom_u32),
+                 __imm(bpf_iter_num_new),
+                 __imm(bpf_iter_num_next),
+                 __imm(bpf_iter_num_destroy),
+                 __imm(bpf_probe_read_user)
+               : __clobber_all
+       );
+}
+
+SEC("?raw_tp")
+__failure
+__msg("math between fp pointer and register with unbounded")
+__naked int delayed_precision_mark(void)
+{
+       /* This is equivalent to C program below.
+        * The test is similar to delayed_iter_mark but verifies that incomplete
+        * precision don't fool verifier.
+        * The call to bpf_iter_num_next() is reachable with r7 values -16 and -32.
+        * State with r7=-16 is visited first and follows r6 != 42 ... continue branch.
+        * At this point iterator next() call is reached with r7 that has no read
+        * and precision marks.
+        * Loop body with r7=-32 would only be visited if verifier would decide to continue
+        * with second loop iteration. Absence of precision mark on r7 might affect state
+        * equivalent logic used for iterator convergence tracking.
+        *
+        * r8 = 0
+        * fp[-16] = 0
+        * r7 = -16
+        * r6 = bpf_get_prandom_u32()
+        * bpf_iter_num_new(&fp[-8], 0, 10)
+        * while (bpf_iter_num_next(&fp[-8])) {
+        *   if (r6 != 42) {
+        *     r7 = -32
+        *     r6 = bpf_get_prandom_u32()
+        *     continue;
+        *   }
+        *   r0 = r10
+        *   r0 += r7
+        *   r8 = *(u64 *)(r0 + 0)           // this is not safe
+        *   r6 = bpf_get_prandom_u32()
+        * }
+        * bpf_iter_num_destroy(&fp[-8])
+        * return r8
+        */
+       asm volatile (
+               "r8 = 0;"
+               "*(u64 *)(r10 - 16) = r8;"
+               "r7 = -16;"
+               "call %[bpf_get_prandom_u32];"
+               "r6 = r0;"
+               "r1 = r10;"
+               "r1 += -8;"
+               "r2 = 0;"
+               "r3 = 10;"
+               "call %[bpf_iter_num_new];"
+       "1:"
+               "r1 = r10;"
+               "r1 += -8;\n"
+               "call %[bpf_iter_num_next];"
+               "if r0 == 0 goto 2f;"
+               "if r6 != 42 goto 3f;"
+               "r7 = -32;"
+               "call %[bpf_get_prandom_u32];"
+               "r6 = r0;"
+               "goto 1b;\n"
+       "3:"
+               "r0 = r10;"
+               "r0 += r7;"
+               "r8 = *(u64 *)(r0 + 0);"
+               "call %[bpf_get_prandom_u32];"
+               "r6 = r0;"
+               "goto 1b;\n"
+       "2:"
+               "r1 = r10;"
+               "r1 += -8;"
+               "call %[bpf_iter_num_destroy];"
+               "r0 = r8;"
+               "exit;"
+               :
+               : __imm(bpf_get_prandom_u32),
+                 __imm(bpf_iter_num_new),
+                 __imm(bpf_iter_num_next),
+                 __imm(bpf_iter_num_destroy),
+                 __imm(bpf_probe_read_user)
+               : __clobber_all
+       );
+}
+
+SEC("?raw_tp")
+__failure
+__msg("math between fp pointer and register with unbounded")
+__flag(BPF_F_TEST_STATE_FREQ)
+__naked int loop_state_deps1(void)
+{
+       /* This is equivalent to C program below.
+        *
+        * The case turns out to be tricky in a sense that:
+        * - states with c=-25 are explored only on a second iteration
+        *   of the outer loop;
+        * - states with read+precise mark on c are explored only on
+        *   second iteration of the inner loop and in a state which
+        *   is pushed to states stack first.
+        *
+        * Depending on the details of iterator convergence logic
+        * verifier might stop states traversal too early and miss
+        * unsafe c=-25 memory access.
+        *
+        *   j = iter_new();             // fp[-16]
+        *   a = 0;                      // r6
+        *   b = 0;                      // r7
+        *   c = -24;                    // r8
+        *   while (iter_next(j)) {
+        *     i = iter_new();           // fp[-8]
+        *     a = 0;                    // r6
+        *     b = 0;                    // r7
+        *     while (iter_next(i)) {
+        *       if (a == 1) {
+        *         a = 0;
+        *         b = 1;
+        *       } else if (a == 0) {
+        *         a = 1;
+        *         if (random() == 42)
+        *           continue;
+        *         if (b == 1) {
+        *           *(r10 + c) = 7;  // this is not safe
+        *           iter_destroy(i);
+        *           iter_destroy(j);
+        *           return;
+        *         }
+        *       }
+        *     }
+        *     iter_destroy(i);
+        *     a = 0;
+        *     b = 0;
+        *     c = -25;
+        *   }
+        *   iter_destroy(j);
+        *   return;
+        */
+       asm volatile (
+               "r1 = r10;"
+               "r1 += -16;"
+               "r2 = 0;"
+               "r3 = 10;"
+               "call %[bpf_iter_num_new];"
+               "r6 = 0;"
+               "r7 = 0;"
+               "r8 = -24;"
+       "j_loop_%=:"
+               "r1 = r10;"
+               "r1 += -16;"
+               "call %[bpf_iter_num_next];"
+               "if r0 == 0 goto j_loop_end_%=;"
+               "r1 = r10;"
+               "r1 += -8;"
+               "r2 = 0;"
+               "r3 = 10;"
+               "call %[bpf_iter_num_new];"
+               "r6 = 0;"
+               "r7 = 0;"
+       "i_loop_%=:"
+               "r1 = r10;"
+               "r1 += -8;"
+               "call %[bpf_iter_num_next];"
+               "if r0 == 0 goto i_loop_end_%=;"
+       "check_one_r6_%=:"
+               "if r6 != 1 goto check_zero_r6_%=;"
+               "r6 = 0;"
+               "r7 = 1;"
+               "goto i_loop_%=;"
+       "check_zero_r6_%=:"
+               "if r6 != 0 goto i_loop_%=;"
+               "r6 = 1;"
+               "call %[bpf_get_prandom_u32];"
+               "if r0 != 42 goto check_one_r7_%=;"
+               "goto i_loop_%=;"
+       "check_one_r7_%=:"
+               "if r7 != 1 goto i_loop_%=;"
+               "r0 = r10;"
+               "r0 += r8;"
+               "r1 = 7;"
+               "*(u64 *)(r0 + 0) = r1;"
+               "r1 = r10;"
+               "r1 += -8;"
+               "call %[bpf_iter_num_destroy];"
+               "r1 = r10;"
+               "r1 += -16;"
+               "call %[bpf_iter_num_destroy];"
+               "r0 = 0;"
+               "exit;"
+       "i_loop_end_%=:"
+               "r1 = r10;"
+               "r1 += -8;"
+               "call %[bpf_iter_num_destroy];"
+               "r6 = 0;"
+               "r7 = 0;"
+               "r8 = -25;"
+               "goto j_loop_%=;"
+       "j_loop_end_%=:"
+               "r1 = r10;"
+               "r1 += -16;"
+               "call %[bpf_iter_num_destroy];"
+               "r0 = 0;"
+               "exit;"
+               :
+               : __imm(bpf_get_prandom_u32),
+                 __imm(bpf_iter_num_new),
+                 __imm(bpf_iter_num_next),
+                 __imm(bpf_iter_num_destroy)
+               : __clobber_all
+       );
+}
+
+SEC("?raw_tp")
+__success
+__naked int triple_continue(void)
+{
+       /* This is equivalent to C program below.
+        * High branching factor of the loop body turned out to be
+        * problematic for one of the iterator convergence tracking
+        * algorithms explored.
+        *
+        * r6 = bpf_get_prandom_u32()
+        * bpf_iter_num_new(&fp[-8], 0, 10)
+        * while (bpf_iter_num_next(&fp[-8])) {
+        *   if (bpf_get_prandom_u32() != 42)
+        *     continue;
+        *   if (bpf_get_prandom_u32() != 42)
+        *     continue;
+        *   if (bpf_get_prandom_u32() != 42)
+        *     continue;
+        *   r0 += 0;
+        * }
+        * bpf_iter_num_destroy(&fp[-8])
+        * return 0
+        */
+       asm volatile (
+               "r1 = r10;"
+               "r1 += -8;"
+               "r2 = 0;"
+               "r3 = 10;"
+               "call %[bpf_iter_num_new];"
+       "loop_%=:"
+               "r1 = r10;"
+               "r1 += -8;"
+               "call %[bpf_iter_num_next];"
+               "if r0 == 0 goto loop_end_%=;"
+               "call %[bpf_get_prandom_u32];"
+               "if r0 != 42 goto loop_%=;"
+               "call %[bpf_get_prandom_u32];"
+               "if r0 != 42 goto loop_%=;"
+               "call %[bpf_get_prandom_u32];"
+               "if r0 != 42 goto loop_%=;"
+               "r0 += 0;"
+               "goto loop_%=;"
+       "loop_end_%=:"
+               "r1 = r10;"
+               "r1 += -8;"
+               "call %[bpf_iter_num_destroy];"
+               "r0 = 0;"
+               "exit;"
+               :
+               : __imm(bpf_get_prandom_u32),
+                 __imm(bpf_iter_num_new),
+                 __imm(bpf_iter_num_next),
+                 __imm(bpf_iter_num_destroy)
+               : __clobber_all
+       );
+}
+
+SEC("?raw_tp")
+__success
+__naked int widen_spill(void)
+{
+       /* This is equivalent to C program below.
+        * The counter is stored in fp[-16], if this counter is not widened
+        * verifier states representing loop iterations would never converge.
+        *
+        * fp[-16] = 0
+        * bpf_iter_num_new(&fp[-8], 0, 10)
+        * while (bpf_iter_num_next(&fp[-8])) {
+        *   r0 = fp[-16];
+        *   r0 += 1;
+        *   fp[-16] = r0;
+        * }
+        * bpf_iter_num_destroy(&fp[-8])
+        * return 0
+        */
+       asm volatile (
+               "r0 = 0;"
+               "*(u64 *)(r10 - 16) = r0;"
+               "r1 = r10;"
+               "r1 += -8;"
+               "r2 = 0;"
+               "r3 = 10;"
+               "call %[bpf_iter_num_new];"
+       "loop_%=:"
+               "r1 = r10;"
+               "r1 += -8;"
+               "call %[bpf_iter_num_next];"
+               "if r0 == 0 goto loop_end_%=;"
+               "r0 = *(u64 *)(r10 - 16);"
+               "r0 += 1;"
+               "*(u64 *)(r10 - 16) = r0;"
+               "goto loop_%=;"
+       "loop_end_%=:"
+               "r1 = r10;"
+               "r1 += -8;"
+               "call %[bpf_iter_num_destroy];"
+               "r0 = 0;"
+               "exit;"
+               :
+               : __imm(bpf_iter_num_new),
+                 __imm(bpf_iter_num_next),
+                 __imm(bpf_iter_num_destroy)
+               : __clobber_all
+       );
+}
+
+SEC("raw_tp")
+__success
+__naked int checkpoint_states_deletion(void)
+{
+       /* This is equivalent to C program below.
+        *
+        *   int *a, *b, *c, *d, *e, *f;
+        *   int i, sum = 0;
+        *   bpf_for(i, 0, 10) {
+        *     a = bpf_map_lookup_elem(&amap, &i);
+        *     b = bpf_map_lookup_elem(&amap, &i);
+        *     c = bpf_map_lookup_elem(&amap, &i);
+        *     d = bpf_map_lookup_elem(&amap, &i);
+        *     e = bpf_map_lookup_elem(&amap, &i);
+        *     f = bpf_map_lookup_elem(&amap, &i);
+        *     if (a) sum += 1;
+        *     if (b) sum += 1;
+        *     if (c) sum += 1;
+        *     if (d) sum += 1;
+        *     if (e) sum += 1;
+        *     if (f) sum += 1;
+        *   }
+        *   return 0;
+        *
+        * The body of the loop spawns multiple simulation paths
+        * with different combination of NULL/non-NULL information for a/b/c/d/e/f.
+        * Each combination is unique from states_equal() point of view.
+        * Explored states checkpoint is created after each iterator next call.
+        * Iterator convergence logic expects that eventually current state
+        * would get equal to one of the explored states and thus loop
+        * exploration would be finished (at-least for a specific path).
+        * Verifier evicts explored states with high miss to hit ratio
+        * to to avoid comparing current state with too many explored
+        * states per instruction.
+        * This test is designed to "stress test" eviction policy defined using formula:
+        *
+        *    sl->miss_cnt > sl->hit_cnt * N + N // if true sl->state is evicted
+        *
+        * Currently N is set to 64, which allows for 6 variables in this test.
+        */
+       asm volatile (
+               "r6 = 0;"                  /* a */
+               "r7 = 0;"                  /* b */
+               "r8 = 0;"                  /* c */
+               "*(u64 *)(r10 - 24) = r6;" /* d */
+               "*(u64 *)(r10 - 32) = r6;" /* e */
+               "*(u64 *)(r10 - 40) = r6;" /* f */
+               "r9 = 0;"                  /* sum */
+               "r1 = r10;"
+               "r1 += -8;"
+               "r2 = 0;"
+               "r3 = 10;"
+               "call %[bpf_iter_num_new];"
+       "loop_%=:"
+               "r1 = r10;"
+               "r1 += -8;"
+               "call %[bpf_iter_num_next];"
+               "if r0 == 0 goto loop_end_%=;"
+
+               "*(u64 *)(r10 - 16) = r0;"
+
+               "r1 = %[amap] ll;"
+               "r2 = r10;"
+               "r2 += -16;"
+               "call %[bpf_map_lookup_elem];"
+               "r6 = r0;"
+
+               "r1 = %[amap] ll;"
+               "r2 = r10;"
+               "r2 += -16;"
+               "call %[bpf_map_lookup_elem];"
+               "r7 = r0;"
+
+               "r1 = %[amap] ll;"
+               "r2 = r10;"
+               "r2 += -16;"
+               "call %[bpf_map_lookup_elem];"
+               "r8 = r0;"
+
+               "r1 = %[amap] ll;"
+               "r2 = r10;"
+               "r2 += -16;"
+               "call %[bpf_map_lookup_elem];"
+               "*(u64 *)(r10 - 24) = r0;"
+
+               "r1 = %[amap] ll;"
+               "r2 = r10;"
+               "r2 += -16;"
+               "call %[bpf_map_lookup_elem];"
+               "*(u64 *)(r10 - 32) = r0;"
+
+               "r1 = %[amap] ll;"
+               "r2 = r10;"
+               "r2 += -16;"
+               "call %[bpf_map_lookup_elem];"
+               "*(u64 *)(r10 - 40) = r0;"
+
+               "if r6 == 0 goto +1;"
+               "r9 += 1;"
+               "if r7 == 0 goto +1;"
+               "r9 += 1;"
+               "if r8 == 0 goto +1;"
+               "r9 += 1;"
+               "r0 = *(u64 *)(r10 - 24);"
+               "if r0 == 0 goto +1;"
+               "r9 += 1;"
+               "r0 = *(u64 *)(r10 - 32);"
+               "if r0 == 0 goto +1;"
+               "r9 += 1;"
+               "r0 = *(u64 *)(r10 - 40);"
+               "if r0 == 0 goto +1;"
+               "r9 += 1;"
+
+               "goto loop_%=;"
+       "loop_end_%=:"
+               "r1 = r10;"
+               "r1 += -8;"
+               "call %[bpf_iter_num_destroy];"
+               "r0 = 0;"
+               "exit;"
+               :
+               : __imm(bpf_map_lookup_elem),
+                 __imm(bpf_iter_num_new),
+                 __imm(bpf_iter_num_next),
+                 __imm(bpf_iter_num_destroy),
+                 __imm_addr(amap)
+               : __clobber_all
+       );
+}
+
 char _license[] SEC("license") = "GPL";