The iterative Tarjan DFS in bpf_compute_scc() emulates recursion with an explicit 'dfs' stack: when a successor has not been visited yet, the successor is pushed and the walk restarts at the top of the loop. On the way back to a vertex the successor scan starts over at index zero, so a vertex with k successors rescans up to k successors on each of its up to k descents, i.e. O(k^2) work. For ordinary instructions k <= 2 and this is irrelevant. For a gotox the successors are the jump table of the containing subprogram, whose size is bounded only by the max_entries of the insn_array map, so k can reach the 1M instruction complexity limit. Loading such a program keeps a CPU busy in the loop for a very long time before verification even begins. Record in 'dfs_pos' the successor index each frame stopped at and resume the scan there. Each edge is therefore examined a bounded number of times and the walk becomes linear in the number of edges. Fixes: 493d9e0d6083 ("bpf, x86: add support for indirect jumps") Reported-by: STAR Labs SG Signed-off-by: Daniel Borkmann Acked-by: Eduard Zingerman --- kernel/bpf/cfg.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/kernel/bpf/cfg.c b/kernel/bpf/cfg.c index 0de2f634ef67..54ff20234236 100644 --- a/kernel/bpf/cfg.c +++ b/kernel/bpf/cfg.c @@ -825,7 +825,7 @@ int bpf_compute_scc(struct bpf_verifier_env *env) struct bpf_insn_aux_data *aux = env->insn_aux_data; const u32 insn_cnt = env->prog->len; int stack_sz, dfs_sz, err = 0; - u32 *stack, *pre, *low, *dfs; + u32 *stack, *pre, *low, *dfs, *dfs_pos; u32 i, j, t, w; u32 next_preorder_num; u32 next_scc_id; @@ -838,13 +838,16 @@ int bpf_compute_scc(struct bpf_verifier_env *env) * - 'stack' accumulates vertices in DFS order, see invariant comment below; * - 'pre[t] == p' => preorder number of vertex 't' is 'p'; * - 'low[t] == n' => smallest preorder number of the vertex reachable from 't' is 'n'; - * - 'dfs' DFS traversal stack, used to emulate explicit recursion. + * - 'dfs' DFS traversal stack, used to emulate explicit recursion; + * - 'dfs_pos[k] == j' => the frame 'dfs[k]' resumes visiting its + * successors at index 'j'. */ stack = kvcalloc(insn_cnt, sizeof(int), GFP_KERNEL_ACCOUNT); pre = kvcalloc(insn_cnt, sizeof(int), GFP_KERNEL_ACCOUNT); low = kvcalloc(insn_cnt, sizeof(int), GFP_KERNEL_ACCOUNT); dfs = kvcalloc(insn_cnt, sizeof(*dfs), GFP_KERNEL_ACCOUNT); - if (!stack || !pre || !low || !dfs) { + dfs_pos = kvcalloc(insn_cnt, sizeof(*dfs_pos), GFP_KERNEL_ACCOUNT); + if (!stack || !pre || !low || !dfs || !dfs_pos) { err = -ENOMEM; goto exit; } @@ -927,6 +930,7 @@ int bpf_compute_scc(struct bpf_verifier_env *env) stack_sz = 0; dfs_sz = 1; dfs[0] = i; + dfs_pos[0] = 0; dfs_continue: while (dfs_sz) { w = dfs[dfs_sz - 1]; @@ -936,13 +940,22 @@ int bpf_compute_scc(struct bpf_verifier_env *env) next_preorder_num++; stack[stack_sz++] = w; } - /* Visit 'w' successors */ + /* Visit remaining 'w' successors */ succ = bpf_insn_successors(env, w); - for (j = 0; j < succ->cnt; ++j) { + for (j = dfs_pos[dfs_sz - 1]; j < succ->cnt; ++j) { if (pre[succ->items[j]]) { low[w] = min(low[w], low[succ->items[j]]); } else { - dfs[dfs_sz++] = succ->items[j]; + /* + * Once DFS for succ->items[j] is complete, + * pre[succ->items[j]] is non-zero, hence + * resuming at 'j' allows to follow the + * low[w] = min(...) update branch above. + */ + dfs_pos[dfs_sz - 1] = j; + dfs_pos[dfs_sz] = 0; + dfs[dfs_sz] = succ->items[j]; + dfs_sz++; goto dfs_continue; } } @@ -992,5 +1005,6 @@ int bpf_compute_scc(struct bpf_verifier_env *env) kvfree(pre); kvfree(low); kvfree(dfs); + kvfree(dfs_pos); return err; } -- 2.43.0