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>
This commit is contained in:
Eduard Zingerman 2025-02-15 03:03:53 -08:00 committed by Alexei Starovoitov
parent bbbc02b744
commit 6da35da1a3

View File

@ -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)