On Thu, Jul 13, 2023 at 08:02:29AM +0530, Kumar Kartikeya Dwivedi wrote: > Now that we allow exception throwing using bpf_throw kfunc, it can > appear as the final instruction in a prog. When this happens, and we > begin to unwind the stack using arch_bpf_stack_walk, the instruction > pointer (IP) may appear to lie outside the JITed instructions. This > happens because the return address is the instruction following the > call, but the bpf_throw never returns to the program, so the JIT > considers instruction ending at the bpf_throw call as the final JITed > instruction and end of the jited_length for the program. > > This becomes a problem when we search the IP using is_bpf_text_address > and bpf_prog_ksym_find, both of which use bpf_ksym_find under the hood, > and it rightfully considers addr == ksym.end to be outside the program's > boundaries. > > Insert a dummy 'int3' instruction which will never be hit to bump the > jited_length and allow us to handle programs with their final > isntruction being a call to bpf_throw. > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> > --- > arch/x86/net/bpf_jit_comp.c | 11 +++++++++++ > include/linux/bpf.h | 2 ++ > 2 files changed, 13 insertions(+) > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > index 8d97c6a60f9a..052230cc7f50 100644 > --- a/arch/x86/net/bpf_jit_comp.c > +++ b/arch/x86/net/bpf_jit_comp.c > @@ -1579,6 +1579,17 @@ st: if (is_imm8(insn->off)) > } > if (emit_call(&prog, func, image + addrs[i - 1] + offs)) > return -EINVAL; > + /* Similar to BPF_EXIT_INSN, call for bpf_throw may be > + * the final instruction in the program. Insert an int3 > + * following the call instruction so that we can still > + * detect pc to be part of the bpf_prog in > + * bpf_ksym_find, otherwise when this is the last > + * instruction (as allowed by verifier, similar to exit > + * and jump instructions), pc will be == ksym.end, > + * leading to bpf_throw failing to unwind the stack. > + */ > + if (func == (u8 *)&bpf_throw) > + EMIT1(0xCC); /* int3 */ Probably worth explaining that this happens because bpf_throw is marked __attribute__((noreturn)) and compiler can emit it last without BPF_EXIT insn. Meaing the program might not have BPF_EXIT at all. I wonder though whether this self-inflicted pain is worth it. May be it shouldn't be marked as noreturn. What do we gain by marking?
