Re: [PATCH bpf-next v5 10/16] bpf: Reject stack arguments if tail call reachable
Yonghong Song <[email protected]>
| Newsgroups | dev.linux.lists.sashiko,org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On 4/16/26 9:08 PM, [email protected] wrote: > Sashiko AI review found 2 potential issue(s): > - [Critical] False negative allowing tail calls and stack arguments together, leading to JIT memory corruption > - [High] State leak in verifier DFS causing false positive rejections of valid BPF programs > -- > > commit df24720ae892549300f1910df635621a3875dbdf > Author: Yonghong Song <[email protected]> > > bpf: Reject stack arguments if tail call reachable > > This commit updates the verifier to reject BPF programs if they contain both > tail calls and stack arguments. This is enforced during the maximum stack > depth check traversal. > >> Tailcalls are been deprecated. So reject stack arguments >> if tail call is in the way. > This isn't a bug, but there is a grammatical error here. "are been" should > likely be "have been". > >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >> index 7a65b532e84a8..739c3127520c3 100644 >> --- a/kernel/bpf/verifier.c >> +++ b/kernel/bpf/verifier.c >> @@ -5662,6 +5662,11 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx, >> verbose(env, "cannot tail call within exception cb\n"); >> return -EINVAL; >> } >> + if (subprog[tmp].incoming_stack_arg_depth || >> + subprog[tmp].outgoing_stack_arg_depth) { >> + verbose(env, "tail_calls are not allowed in programs with stack args\n"); >> + return -EINVAL; >> + } > Looking at the earlier parts of check_max_stack_depth_subprog(), the local > variable tail_call_reachable only appears to be set to true when jumping > into a callee that has a tail call: > > if (subprog[idx].has_tail_call) > tail_call_reachable = true; > > If the main program (subprog[0]) natively contains a tail call, > tail_call_reachable would remain false, causing this entire check block to > be skipped. > > Does this logic successfully catch the case where the main program itself > contains a tail call and also uses stack arguments? > > If this escapes the verifier, the JIT might emit prologue tail call > instructions (pushing 24 bytes on x86) while calculating stack argument > offsets assuming no tail call pushes, overwriting the BPF program stack. > > Could this regression lead to memory corruption in the JIT? > > When the traversal pops a frame to return to the caller and jumps back to > continue_func, the tail_call_reachable variable is not reset to false: > > /* pop caller idx from callee */ > idx = dinfo[idx].caller; > ... > goto continue_func; > > If the verifier traverses a subprogram with a tail call, returns, and then > traverses a completely unrelated sibling subprogram that uses stack > arguments, it seems it would incorrectly evaluate tail_call_reachable as > true and reject the valid sibling. > > Is there a state leak in this DFS traversal that could cause false positive > rejections of valid BPF programs? You are right. This has been their for sometime. So I guess we can live with it even for stack arguments. > >> subprog[tmp].tail_call_reachable = true; >> } >> if (subprog[0].tail_call_reachable)