Commit ba7b3e7d authored by Kumar Kartikeya Dwivedi's avatar Kumar Kartikeya Dwivedi Committed by Alexei Starovoitov

bpf: Fix subprog idx logic in check_max_stack_depth

The assignment to idx in check_max_stack_depth happens once we see a
bpf_pseudo_call or bpf_pseudo_func. This is not an issue as the rest of
the code performs a few checks and then pushes the frame to the frame
stack, except the case of async callbacks. If the async callback case
causes the loop iteration to be skipped, the idx assignment will be
incorrect on the next iteration of the loop. The value stored in the
frame stack (as the subprogno of the current subprog) will be incorrect.

This leads to incorrect checks and incorrect tail_call_reachable
marking. Save the target subprog in a new variable and only assign to
idx once we are done with the is_async_cb check which may skip pushing
of frame to the frame stack and subsequent stack depth checks and tail
call markings.

Fixes: 7ddc80a4 ("bpf: Teach stack depth check about async callbacks.")
Signed-off-by: default avatarKumar Kartikeya Dwivedi <memxor@gmail.com>
Link: https://lore.kernel.org/r/20230717161530.1238-2-memxor@gmail.comSigned-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parent 8fcd7c7b
...@@ -5621,7 +5621,7 @@ static int check_max_stack_depth(struct bpf_verifier_env *env) ...@@ -5621,7 +5621,7 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
continue_func: continue_func:
subprog_end = subprog[idx + 1].start; subprog_end = subprog[idx + 1].start;
for (; i < subprog_end; i++) { for (; i < subprog_end; i++) {
int next_insn; int next_insn, sidx;
if (!bpf_pseudo_call(insn + i) && !bpf_pseudo_func(insn + i)) if (!bpf_pseudo_call(insn + i) && !bpf_pseudo_func(insn + i))
continue; continue;
...@@ -5631,14 +5631,14 @@ static int check_max_stack_depth(struct bpf_verifier_env *env) ...@@ -5631,14 +5631,14 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
/* find the callee */ /* find the callee */
next_insn = i + insn[i].imm + 1; next_insn = i + insn[i].imm + 1;
idx = find_subprog(env, next_insn); sidx = find_subprog(env, next_insn);
if (idx < 0) { if (sidx < 0) {
WARN_ONCE(1, "verifier bug. No program starts at insn %d\n", WARN_ONCE(1, "verifier bug. No program starts at insn %d\n",
next_insn); next_insn);
return -EFAULT; return -EFAULT;
} }
if (subprog[idx].is_async_cb) { if (subprog[sidx].is_async_cb) {
if (subprog[idx].has_tail_call) { if (subprog[sidx].has_tail_call) {
verbose(env, "verifier bug. subprog has tail_call and async cb\n"); verbose(env, "verifier bug. subprog has tail_call and async cb\n");
return -EFAULT; return -EFAULT;
} }
...@@ -5647,6 +5647,7 @@ static int check_max_stack_depth(struct bpf_verifier_env *env) ...@@ -5647,6 +5647,7 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
continue; continue;
} }
i = next_insn; i = next_insn;
idx = sidx;
if (subprog[idx].has_tail_call) if (subprog[idx].has_tail_call)
tail_call_reachable = true; tail_call_reachable = true;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment