Skip to content

Commit 2b2efe1

Browse files
Alexei Starovoitovborkmann
authored andcommitted
bpf: Fix may_goto with negative offset.
Zac's syzbot crafted a bpf prog that exposed two bugs in may_goto. The 1st bug is the way may_goto is patched. When offset is negative it should be patched differently. The 2nd bug is in the verifier: when current state may_goto_depth is equal to visited state may_goto_depth it means there is an actual infinite loop. It's not correct to prune exploration of the program at this point. Note, that this check doesn't limit the program to only one may_goto insn, since 2nd and any further may_goto will increment may_goto_depth only in the queued state pushed for future exploration. The current state will have may_goto_depth == 0 regardless of number of may_goto insns and the verifier has to explore the program until bpf_exit. Fixes: 011832b ("bpf: Introduce may_goto instruction") Reported-by: Zac Ecob <[email protected]> Signed-off-by: Alexei Starovoitov <[email protected]> Signed-off-by: Andrii Nakryiko <[email protected]> Signed-off-by: Daniel Borkmann <[email protected]> Acked-by: Eduard Zingerman <[email protected]> Closes: https://lore.kernel.org/bpf/CAADnVQL-15aNp04-cyHRn47Yv61NXfYyhopyZtUyxNojUZUXpA@mail.gmail.com/ Link: https://lore.kernel.org/bpf/[email protected]
1 parent 316930d commit 2b2efe1

File tree

1 file changed

+6
-3
lines changed

1 file changed

+6
-3
lines changed

kernel/bpf/verifier.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17460,11 +17460,11 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
1746017460
goto skip_inf_loop_check;
1746117461
}
1746217462
if (is_may_goto_insn_at(env, insn_idx)) {
17463-
if (states_equal(env, &sl->state, cur, RANGE_WITHIN)) {
17463+
if (sl->state.may_goto_depth != cur->may_goto_depth &&
17464+
states_equal(env, &sl->state, cur, RANGE_WITHIN)) {
1746417465
update_loop_entry(cur, &sl->state);
1746517466
goto hit;
1746617467
}
17467-
goto skip_inf_loop_check;
1746817468
}
1746917469
if (calls_callback(env, insn_idx)) {
1747017470
if (states_equal(env, &sl->state, cur, RANGE_WITHIN))
@@ -20049,7 +20049,10 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
2004920049

2005020050
stack_depth_extra = 8;
2005120051
insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_AX, BPF_REG_10, stack_off);
20052-
insn_buf[1] = BPF_JMP_IMM(BPF_JEQ, BPF_REG_AX, 0, insn->off + 2);
20052+
if (insn->off >= 0)
20053+
insn_buf[1] = BPF_JMP_IMM(BPF_JEQ, BPF_REG_AX, 0, insn->off + 2);
20054+
else
20055+
insn_buf[1] = BPF_JMP_IMM(BPF_JEQ, BPF_REG_AX, 0, insn->off - 1);
2005320056
insn_buf[2] = BPF_ALU64_IMM(BPF_SUB, BPF_REG_AX, 1);
2005420057
insn_buf[3] = BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_AX, stack_off);
2005520058
cnt = 4;

0 commit comments

Comments
 (0)