Skip to content

Commit 7805264

Browse files
committed
bpf: fix precision backtracking instruction iteration
jira LE-1907 Rebuild_History Non-Buildable kernel-5.14.0-427.18.1.el9_4 commit-author Andrii Nakryiko <andrii@kernel.org> commit 4bb7ea9 Fix an edge case in __mark_chain_precision() which prematurely stops backtracking instructions in a state if it happens that state's first and last instruction indexes are the same. This situations doesn't necessarily mean that there were no instructions simulated in a state, but rather that we starting from the instruction, jumped around a bit, and then ended up at the same instruction before checkpointing or marking precision. To distinguish between these two possible situations, we need to consult jump history. If it's empty or contain a single record "bridging" parent state and first instruction of processed state, then we indeed backtracked all instructions in this state. But if history is not empty, we are definitely not done yet. Move this logic inside get_prev_insn_idx() to contain it more nicely. Use -ENOENT return code to denote "we are out of instructions" situation. This bug was exposed by verifier_loop1.c's bounded_recursion subtest, once the next fix in this patch set is applied. Acked-by: Eduard Zingerman <eddyz87@gmail.com> Fixes: b5dc016 ("bpf: precise scalar_value tracking") Signed-off-by: Andrii Nakryiko <andrii@kernel.org> Link: https://lore.kernel.org/r/20231110002638.4168352-3-andrii@kernel.org Signed-off-by: Alexei Starovoitov <ast@kernel.org> (cherry picked from commit 4bb7ea9) Signed-off-by: Jonathan Maple <jmaple@ciq.com>
1 parent 600b4a2 commit 7805264

File tree

1 file changed

+19
-2
lines changed

1 file changed

+19
-2
lines changed

kernel/bpf/verifier.c

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3199,12 +3199,29 @@ static int push_jmp_history(struct bpf_verifier_env *env,
31993199

32003200
/* Backtrack one insn at a time. If idx is not at the top of recorded
32013201
* history then previous instruction came from straight line execution.
3202+
* Return -ENOENT if we exhausted all instructions within given state.
3203+
*
3204+
* It's legal to have a bit of a looping with the same starting and ending
3205+
* insn index within the same state, e.g.: 3->4->5->3, so just because current
3206+
* instruction index is the same as state's first_idx doesn't mean we are
3207+
* done. If there is still some jump history left, we should keep going. We
3208+
* need to take into account that we might have a jump history between given
3209+
* state's parent and itself, due to checkpointing. In this case, we'll have
3210+
* history entry recording a jump from last instruction of parent state and
3211+
* first instruction of given state.
32023212
*/
32033213
static int get_prev_insn_idx(struct bpf_verifier_state *st, int i,
32043214
u32 *history)
32053215
{
32063216
u32 cnt = *history;
32073217

3218+
if (i == st->first_insn_idx) {
3219+
if (cnt == 0)
3220+
return -ENOENT;
3221+
if (cnt == 1 && st->jmp_history[0].idx == i)
3222+
return -ENOENT;
3223+
}
3224+
32083225
if (cnt && st->jmp_history[cnt - 1].idx == i) {
32093226
i = st->jmp_history[cnt - 1].prev_idx;
32103227
(*history)--;
@@ -4084,10 +4101,10 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int regno)
40844101
* Nothing to be tracked further in the parent state.
40854102
*/
40864103
return 0;
4087-
if (i == first_idx)
4088-
break;
40894104
subseq_idx = i;
40904105
i = get_prev_insn_idx(st, i, &history);
4106+
if (i == -ENOENT)
4107+
break;
40914108
if (i >= env->prog->len) {
40924109
/* This can happen if backtracking reached insn 0
40934110
* and there are still reg_mask or stack_mask

0 commit comments

Comments
 (0)