selftests/bpf: test correct loop_entry update in copy_verifier_state

A somewhat cumbersome test case sensitive to correct copying of
bpf_verifier_state->loop_entry fields in
verifier.c:copy_verifier_state().
W/o the fix from a previous commit the program is accepted as safe.

     1:  /* poison block */
     2:  if (random() != 24) {       // assume false branch is placed first
     3:    i = iter_new();
     4:    while (iter_next(i));
     5:    iter_destroy(i);
     6:    return;
     7:  }
     8:
     9:  /* dfs_depth block */
    10:  for (i = 10; i > 0; i--);
    11:
    12:  /* main block */
    13:  i = iter_new();             // fp[-16]
    14:  b = -24;                    // r8
    15:  for (;;) {
    16:    if (iter_next(i))
    17:      break;
    18:    if (random() == 77) {     // assume false branch is placed first
    19:      *(u64 *)(r10 + b) = 7;  // this is not safe when b == -25
    20:      iter_destroy(i);
    21:      return;
    22:    }
    23:    if (random() == 42) {     // assume false branch is placed first
    24:      b = -25;
    25:    }
    26:  }
    27:  iter_destroy(i);

The goal of this example is to:
(a) poison env->cur_state->loop_entry with a state S,
    such that S->branches == 0;
(b) set state S as a loop_entry for all checkpoints in
    /* main block */, thus forcing NOT_EXACT states comparisons;
(c) exploit incorrect loop_entry set for checkpoint at line 18
    by first creating a checkpoint with b == -24 and then
    pruning the state with b == -25 using that checkpoint.

The /* poison block */ is responsible for goal (a).
It forces verifier to first validate some unrelated iterator based
loop, which leads to an update_loop_entry() call in is_state_visited(),
which places checkpoint created at line 4 as env->cur_state->loop_entry.
Starting from line 8, the branch count for that checkpoint is 0.

The /* dfs_depth block */ is responsible for goal (b).
It abuses the fact that update_loop_entry(cur, hdr) only updates
cur->loop_entry when hdr->dfs_depth <= cur->dfs_depth.
After line 12 every state has dfs_depth bigger then dfs_depth of
poisoned env->cur_state->loop_entry. Thus the above condition is never
true for lines 12-27.

The /* main block */ is responsible for goal (c).
Verification proceeds as follows:
- checkpoint {b=-24,i=active} created at line 16;
- jump 18->23 is verified first, jump to 19 pushed to stack;
- jump 23->26 is verified first, jump to 24 pushed to stack;
- checkpoint {b=-24,i=active} created at line 15;
- current state is pruned by checkpoint created at line 16,
  this sets branches count for checkpoint at line 15 to 0;
- jump to 24 is popped from stack;
- line 16 is reached in state {b=-25,i=active};
- this is pruned by a previous checkpoint {b=-24,i=active}:
  - checkpoint's loop_entry is poisoned and has branch count of 0,
    hence states are compared using NOT_EXACT rules;
  - b is not marked precise yet.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20250215110411.3236773-3-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

diff --git a/tools/testing/selftests/bpf/progs/iters.c b/tools/testing/selftests/bpf/progs/iters.c
index 190822b2f08bca0454951cfdaa926499686ba1c7..007831dc8c46d2fca66eb4e4ecc801c03ba1bab7 100644 (file)
--- a/tools/testing/selftests/bpf/progs/iters.c
+++ b/tools/testing/selftests/bpf/progs/iters.c
@@ -1174,6 +1174,122 @@ __naked int loop_state_deps2(void)
        );
 }
 
+SEC("?raw_tp")
+__failure
+__msg("math between fp pointer and register with unbounded")
+__flag(BPF_F_TEST_STATE_FREQ)
+__naked int loop_state_deps3(void)
+{
+       /* This is equivalent to a C program below.
+        *
+        *   if (random() != 24) {       // assume false branch is placed first
+        *     i = iter_new();           // fp[-8]
+        *     while (iter_next(i));
+        *     iter_destroy(i);
+        *     return;
+        *   }
+        *
+        *   for (i = 10; i > 0; i--);   // increase dfs_depth for child states
+        *
+        *   i = iter_new();             // fp[-8]
+        *   b = -24;                    // r8
+        *   for (;;) {                  // checkpoint (L)
+        *     if (iter_next(i))         // checkpoint (N)
+        *       break;
+        *     if (random() == 77) {     // assume false branch is placed first
+        *       *(u64 *)(r10 + b) = 7;  // this is not safe when b == -25
+        *       iter_destroy(i);
+        *       return;
+        *     }
+        *     if (random() == 42) {     // assume false branch is placed first
+        *       b = -25;
+        *     }
+        *   }
+        *   iter_destroy(i);
+        *
+        * In case of a buggy verifier first loop might poison
+        * env->cur_state->loop_entry with a state having 0 branches
+        * and small dfs_depth. This would trigger NOT_EXACT states
+        * comparison for some states within second loop.
+        * Specifically, checkpoint (L) might be problematic if:
+        * - branch with '*(u64 *)(r10 + b) = 7' is not explored yet;
+        * - checkpoint (L) is first reached in state {b=-24};
+        * - traversal is pruned at checkpoint (N) setting checkpoint's (L)
+        *   branch count to 0, thus making it eligible for use in pruning;
+        * - checkpoint (L) is next reached in state {b=-25},
+        *   this would cause NOT_EXACT comparison with a state {b=-24}
+        *   while 'b' is not marked precise yet.
+        */
+       asm volatile (
+               "call %[bpf_get_prandom_u32];"
+               "if r0 == 24 goto 2f;"
+               "r1 = r10;"
+               "r1 += -8;"
+               "r2 = 0;"
+               "r3 = 5;"
+               "call %[bpf_iter_num_new];"
+       "1:"
+               "r1 = r10;"
+               "r1 += -8;"
+               "call %[bpf_iter_num_next];"
+               "if r0 != 0 goto 1b;"
+               "r1 = r10;"
+               "r1 += -8;"
+               "call %[bpf_iter_num_destroy];"
+               "r0 = 0;"
+               "exit;"
+       "2:"
+               /* loop to increase dfs_depth */
+               "r0 = 10;"
+       "3:"
+               "r0 -= 1;"
+               "if r0 != 0 goto 3b;"
+               /* end of loop */
+               "r1 = r10;"
+               "r1 += -8;"
+               "r2 = 0;"
+               "r3 = 10;"
+               "call %[bpf_iter_num_new];"
+               "r8 = -24;"
+       "main_loop_%=:"
+               "r1 = r10;"
+               "r1 += -8;"
+               "call %[bpf_iter_num_next];"
+               "if r0 == 0 goto main_loop_end_%=;"
+               /* first if */
+               "call %[bpf_get_prandom_u32];"
+               "if r0 == 77 goto unsafe_write_%=;"
+               /* second if */
+               "call %[bpf_get_prandom_u32];"
+               "if r0 == 42 goto poison_r8_%=;"
+               /* iterate */
+               "goto main_loop_%=;"
+       "main_loop_end_%=:"
+               "r1 = r10;"
+               "r1 += -8;"
+               "call %[bpf_iter_num_destroy];"
+               "r0 = 0;"
+               "exit;"
+
+       "unsafe_write_%=:"
+               "r0 = r10;"
+               "r0 += r8;"
+               "r1 = 7;"
+               "*(u64 *)(r0 + 0) = r1;"
+               "goto main_loop_end_%=;"
+
+       "poison_r8_%=:"
+               "r8 = -25;"
+               "goto main_loop_%=;"
+               :
+               : __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)