On Sun, 2023-09-03 at 23:14 +0800, Leon Hwang wrote:
> This patch series fixes a tailcall infinite loop on x64.
>
> From commit ebf7d1f508a73871 ("bpf, x64: rework pro/epilogue and
> tailcall
> handling in JIT"), the tailcall on x64 works better than before.
>
> From commit e411901c0b775a3a ("bpf: allow for tailcalls in BPF
> subprograms
> for x64 JIT"), tailcall is able to run in BPF subprograms on x64.
>
> From commit 5b92a28aae4dd0f8 ("bpf: Support attaching tracing BPF
> program
> to other BPF programs"), BPF program is able to trace other BPF
> programs.
>
> How about combining them all together?
>
> 1. FENTRY/FEXIT on a BPF subprogram.
> 2. A tailcall runs in the BPF subprogram.
> 3. The tailcall calls the subprogram's caller.
>
> As a result, a tailcall infinite loop comes up. And the loop would
> halt
> the machine.
>
> As we know, in tail call context, the tail_call_cnt propagates by
> stack
> and rax register between BPF subprograms. So do in trampolines.
>
> How did I discover the bug?
>
> From commit 7f6e4312e15a5c37 ("bpf: Limit caller's stack depth 256
> for
> subprogs with tailcalls"), the total stack size limits to around
> 8KiB.
> Then, I write some bpf progs to validate the stack consuming, that
> are
> tailcalls running in bpf2bpf and FENTRY/FEXIT tracing on bpf2bpf[1].
>
> At that time, accidently, I made a tailcall loop. And then the loop
> halted
> my VM. Without the loop, the bpf progs would consume over 8KiB stack
> size.
> But the _stack-overflow_ did not halt my VM.
>
> With bpf_printk(), I confirmed that the tailcall count limit did not
> work
> expectedly. Next, read the code and fix it.
>
> Finally, unfortunately, I only fix it on x64 but other arches. As a
> result, CI tests failed because this bug hasn't been fixed on s390x.
>
> Some helps on s390x are requested.
I will take a look, thanks for letting me know.
I noticed there was something peculiar in this area when implementing
the trampoline:
* Note 1: The callee can increment the tail call counter, but
* we do not load it back, since the x86 JIT does not do this
* either.
but I thought that this was intentional.
[...]
