History

bpf, x64: rework pro/epilogue and tailcall handling in JIT

This commit serves two things: 1) it optimizes BPF prologue/epilogue generation 2) it makes possible to have tailcalls within BPF subprogram Both points are related to each other since without 1), 2) could not be achieved. In [1], Alexei says: "The prologue will look like: nop5 xor eax,eax  // two new bytes if bpf_tail_call() is used in this // function push rbp mov rbp, rsp sub rsp, rounded_stack_depth push rax // zero init tail_call counter variable number of push rbx,r13,r14,r15 Then bpf_tail_call will pop variable number rbx,.. and final 'pop rax' Then 'add rsp, size_of_current_stack_frame' jmp to next function and skip over 'nop5; xor eax,eax; push rpb; mov rbp, rsp' This way new function will set its own stack size and will init tail call counter with whatever value the parent had. If next function doesn't use bpf_tail_call it won't have 'xor eax,eax'. Instead it would need to have 'nop2' in there." Implement that suggestion. Since the layout of stack is changed, tail call counter handling can not rely anymore on popping it to rbx just like it have been handled for constant prologue case and later overwrite of rbx with actual value of rbx pushed to stack. Therefore, let's use one of the register (%rcx) that is considered to be volatile/caller-saved and pop the value of tail call counter in there in the epilogue. Drop the BUILD_BUG_ON in emit_prologue and in emit_bpf_tail_call_indirect where instruction layout is not constant anymore. Introduce new poke target, 'tailcall_bypass' to poke descriptor that is dedicated for skipping the register pops and stack unwind that are generated right before the actual jump to target program. For case when the target program is not present, BPF program will skip the pop instructions and nop5 dedicated for jmpq $target. An example of such state when only R6 of callee saved registers is used by program: ffffffffc0513aa1: e9 0e 00 00 00 jmpq 0xffffffffc0513ab4 ffffffffc0513aa6: 5b pop %rbx ffffffffc0513aa7: 58 pop %rax ffffffffc0513aa8: 48 81 c4 00 00 00 00 add $0x0,%rsp ffffffffc0513aaf: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) ffffffffc0513ab4: 48 89 df mov %rbx,%rdi When target program is inserted, the jump that was there to skip pops/nop5 will become the nop5, so CPU will go over pops and do the actual tailcall. One might ask why there simply can not be pushes after the nop5? In the following example snippet: ffffffffc037030c: 48 89 fb mov %rdi,%rbx (...) ffffffffc0370332: 5b pop %rbx ffffffffc0370333: 58 pop %rax ffffffffc0370334: 48 81 c4 00 00 00 00 add $0x0,%rsp ffffffffc037033b: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) ffffffffc0370340: 48 81 ec 00 00 00 00 sub $0x0,%rsp ffffffffc0370347: 50 push %rax ffffffffc0370348: 53 push %rbx ffffffffc0370349: 48 89 df mov %rbx,%rdi ffffffffc037034c: e8 f7 21 00 00 callq 0xffffffffc0372548 There is the bpf2bpf call (at ffffffffc037034c) right after the tailcall and jump target is not present. ctx is in %rbx register and BPF subprogram that we will call into on ffffffffc037034c is relying on it, e.g. it will pick ctx from there. Such code layout is therefore broken as we would overwrite the content of %rbx with the value that was pushed on the prologue. That is the reason for the 'bypass' approach. Special care needs to be taken during the install/update/remove of tailcall target. In case when target program is not present, the CPU must not execute the pop instructions that precede the tailcall. To address that, the following states can be defined: A nop, unwind, nop B nop, unwind, tail C skip, unwind, nop D skip, unwind, tail A is forbidden (lead to incorrectness). The state transitions between tailcall install/update/remove will work as follows: First install tail call f: C->D->B(f) * poke the tailcall, after that get rid of the skip Update tail call f to f': B(f)->B(f') * poke the tailcall (poke->tailcall_target) and do NOT touch the poke->tailcall_bypass Remove tail call: B(f')->C(f') * poke->tailcall_bypass is poked back to jump, then we wait the RCU grace period so that other programs will finish its execution and after that we are safe to remove the poke->tailcall_target Install new tail call (f''): C(f')->D(f'')->B(f''). * same as first step This way CPU can never be exposed to "unwind, tail" state. Last but not least, when tailcalls get mixed with bpf2bpf calls, it would be possible to encounter the endless loop due to clearing the tailcall counter if for example we would use the tailcall3-like from BPF selftests program that would be subprogram-based, meaning the tailcall would be present within the BPF subprogram. This test, broken down to particular steps, would do: entry -> set tailcall counter to 0, bump it by 1, tailcall to func0 func0 -> call subprog_tail (we are NOT skipping the first 11 bytes of prologue and this subprogram has a tailcall, therefore we clear the counter...) subprog -> do the same thing as entry and then loop forever. To address this, the idea is to go through the call chain of bpf2bpf progs and look for a tailcall presence throughout whole chain. If we saw a single tail call then each node in this call chain needs to be marked as a subprog that can reach the tailcall. We would later feed the JIT with this info and: - set eax to 0 only when tailcall is reachable and this is the entry prog - if tailcall is reachable but there's no tailcall in insns of currently JITed prog then push rax anyway, so that it will be possible to propagate further down the call chain - finally if tailcall is reachable, then we need to precede the 'call' insn with mov rax, [rbp - (stack_depth + 8)] Tail call related cases from test_verifier kselftest are also working fine. Sample BPF programs that utilize tail calls (sockex3, tracex5) work properly as well. [1]: https://lore.kernel.org/bpf/20200517043227.2gpq22ifoq37ogst@ast-mbp.dhcp.thefacebook.com/

Maciej Fijalkowski committed 6 years ago

Commit ebf7d1f1 parent 7f6e4316 files · +244 −55Browse files

Showing the changes to kernel (3 of 6 files). Show all

Unified · Split

3 files
  1. kernel/bpf/arraymap.c +34 −6
  2. kernel/bpf/core.c +1 −1
  3. kernel/bpf/verifier.c +16 −0
kernel/bpf/arraymap.c+34−6
@@ -898,6 +898,7 @@ static void prog_array_map_poke_run(struct bpf_map *map, u32 key, · 897 unmodified lines
898898 struct bpf_prog *old,
899899 struct bpf_prog *new)
900900 {
901+ u8 *old_addr, *new_addr, *old_bypass_addr;
901902 struct prog_poke_elem *elem;
902903 struct bpf_array_aux *aux;
903904
@@ -949,12 +950,39 @@ static void prog_array_map_poke_run(struct bpf_map *map, u32 key, · 45 unmodified lines
949950 poke->tail_call.key != key)
950951 continue;
951952
952− ret = bpf_arch_text_poke(poke->tailcall_target, BPF_MOD_JUMP,
953− old ? (u8 *)old->bpf_func +
954− poke->adj_off : NULL,
955− new ? (u8 *)new->bpf_func +
956− poke->adj_off : NULL);
957− BUG_ON(ret < 0 && ret != -EINVAL);
953+ old_bypass_addr = old ? NULL : poke->bypass_addr;
954+ old_addr = old ? (u8 *)old->bpf_func + poke->adj_off : NULL;
955+ new_addr = new ? (u8 *)new->bpf_func + poke->adj_off : NULL;
956+
957+ if (new) {
958+ ret = bpf_arch_text_poke(poke->tailcall_target,
959+ BPF_MOD_JUMP,
960+ old_addr, new_addr);
961+ BUG_ON(ret < 0 && ret != -EINVAL);
962+ if (!old) {
963+ ret = bpf_arch_text_poke(poke->tailcall_bypass,
964+ BPF_MOD_JUMP,
965+ poke->bypass_addr,
966+ NULL);
967+ BUG_ON(ret < 0 && ret != -EINVAL);
968+ }
969+ } else {
970+ ret = bpf_arch_text_poke(poke->tailcall_bypass,
971+ BPF_MOD_JUMP,
972+ old_bypass_addr,
973+ poke->bypass_addr);
974+ BUG_ON(ret < 0 && ret != -EINVAL);
975+ /* let other CPUs finish the execution of program
976+ * so that it will not possible to expose them
977+ * to invalid nop, stack unwind, nop state
978+ */
979+ if (!ret)
980+ synchronize_rcu();
981+ ret = bpf_arch_text_poke(poke->tailcall_target,
982+ BPF_MOD_JUMP,
983+ old_addr, NULL);
984+ BUG_ON(ret < 0 && ret != -EINVAL);
985+ }
958986 }
959987 }
960988 }
kernel/bpf/core.c+1−1
@@ -776,7 +776,7 @@ int bpf_jit_add_poke_descriptor(struct bpf_prog *prog, · 775 unmodified lines
776776 if (size > poke_tab_max)
777777 return -ENOSPC;
778778 if (poke->tailcall_target || poke->tailcall_target_stable ||
779− poke->adj_off)
779+ poke->tailcall_bypass || poke->adj_off || poke->bypass_addr)
780780 return -EINVAL;
781781
782782 switch (poke->reason) {
kernel/bpf/verifier.c+16−0
@@ -2983,8 +2983,10 @@ static int check_max_stack_depth(struct bpf_verifier_env *env) · 2982 unmodified lines
29832983 int depth = 0, frame = 0, idx = 0, i = 0, subprog_end;
29842984 struct bpf_subprog_info *subprog = env->subprog_info;
29852985 struct bpf_insn *insn = env->prog->insnsi;
2986+ bool tail_call_reachable = false;
29862987 int ret_insn[MAX_CALL_FRAMES];
29872988 int ret_prog[MAX_CALL_FRAMES];
2989+ int j;
29882990
29892991 process_func:
29902992 /* protect against potential stack overflow that might happen when
@@ -3040,6 +3042,10 @@ continue_func: · 49 unmodified lines
30403042 i);
30413043 return -EFAULT;
30423044 }
3045+
3046+ if (subprog[idx].has_tail_call)
3047+ tail_call_reachable = true;
3048+
30433049 frame++;
30443050 if (frame >= MAX_CALL_FRAMES) {
30453051 verbose(env, "the call stack of %d frames is too deep !\n",
@@ -3048,6 +3054,15 @@ continue_func: · 2 unmodified lines
30483054 }
30493055 goto process_func;
30503056 }
3057+ /* if tail call got detected across bpf2bpf calls then mark each of the
3058+ * currently present subprog frames as tail call reachable subprogs;
3059+ * this info will be utilized by JIT so that we will be preserving the
3060+ * tail call counter throughout bpf2bpf calls combined with tailcalls
3061+ */
3062+ if (tail_call_reachable)
3063+ for (j = 0; j < frame; j++)
3064+ subprog[ret_prog[j]].tail_call_reachable = true;
3065+
30513066 /* end of for() loop means the last insn of the 'subprog'
30523067 * was reached. Doesn't matter whether it was JA or EXIT
30533068 */
@@ -10322,6 +10337,7 @@ static int jit_subprogs(struct bpf_verifier_env *env) · 7268 unmodified lines
1032210337 num_exentries++;
1032310338 }
1032410339 func[i]->aux->num_exentries = num_exentries;
10340+ func[i]->aux->tail_call_reachable = env->subprog_info[i].tail_call_reachable;
1032510341 func[i] = bpf_int_jit_compile(func[i]);
1032610342 if (!func[i]->jited) {
1032710343 err = -ENOTSUPP;

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t