On Sat, 15 Jul 2023 at 03:35, Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Thu, Jul 13, 2023 at 08:02:27AM +0530, Kumar Kartikeya Dwivedi wrote:
> > The plumbing for offline unwinding when we throw an exception in
> > programs would require walking the stack, hence introduce a new
> > arch_bpf_stack_walk function. This is provided when the JIT supports
> > exceptions, i.e. bpf_jit_supports_exceptions is true. The arch-specific
> > code is really minimal, hence it should straightforward to extend this
> > support to other architectures as well, as it reuses the logic of
> > arch_stack_walk, but allowing access to unwind_state data.
> >
> > Once the stack pointer and frame pointer are known for the main subprog
> > during the unwinding, we know the stack layout and location of any
> > callee-saved registers which must be restored before we return back to
> > the kernel.
> >
> > This handling will be added in the next patch.
> >
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
> > ---
> > arch/x86/net/bpf_jit_comp.c | 21 +++++++++++++++++++++
> > include/linux/filter.h | 2 ++
> > kernel/bpf/core.c | 9 +++++++++
> > 3 files changed, 32 insertions(+)
> >
> > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> > index 438adb695daa..d326503ce242 100644
> > --- a/arch/x86/net/bpf_jit_comp.c
> > +++ b/arch/x86/net/bpf_jit_comp.c
> > @@ -16,6 +16,7 @@
> > #include <asm/set_memory.h>
> > #include <asm/nospec-branch.h>
> > #include <asm/text-patching.h>
> > +#include <asm/unwind.h>
> >
> > static u8 *emit_code(u8 *ptr, u32 bytes, unsigned int len)
> > {
> > @@ -2660,3 +2661,23 @@ void bpf_jit_free(struct bpf_prog *prog)
> >
> > bpf_prog_unlock_free(prog);
> > }
> > +
> > +bool bpf_jit_supports_exceptions(void)
> > +{
> > + return IS_ENABLED(CONFIG_UNWINDER_ORC) || IS_ENABLED(CONFIG_UNWINDER_FRAME_POINTER);
> > +}
> > +
> > +void arch_bpf_stack_walk(bool (*consume_fn)(void *cookie, u64 ip, u64 sp, u64 bp), void *cookie)
> > +{
> > +#if defined(CONFIG_UNWINDER_ORC) || defined(CONFIG_UNWINDER_FRAME_POINTER)
> > + struct unwind_state state;
> > + unsigned long addr;
> > +
> > + for (unwind_start(&state, current, NULL, NULL); !unwind_done(&state);
> > + unwind_next_frame(&state)) {
> > + addr = unwind_get_return_address(&state);
>
> I think these steps will work even with UNWINDER_GUESS.
> What is the reason for #ifdef ?
I think we require both unwind_state::sp and unwind_state::bp, but
arch/x86/include/asm/unwind.h does not include unwind_state::bp when
both UNWINDER_ORC and UNWINDER_FRAME_POINTER are unset.
Although it might be possible to calculate and save bp offset during
JIT in bpf_prog_aux (by adding roundup(stack_depth) + 8 (push rax if
tail call reachable) + callee_regs_saved) for the subprog
corresponding to a frame. Then we can make it work everywhere.
The JIT will abstract get_prog_bp(sp) using an arch specific helper.
Let me know if I misunderstood something.
>
> > + if (!addr || !consume_fn(cookie, (u64)addr, (u64)state.sp, (u64)state.bp))
> > + break;
> > + }
> > +#endif
> > +}
> > diff --git a/include/linux/filter.h b/include/linux/filter.h
> > index f69114083ec7..21ac801330bb 100644
> > --- a/include/linux/filter.h
> > +++ b/include/linux/filter.h
> > @@ -920,6 +920,8 @@ bool bpf_jit_needs_zext(void);
> > bool bpf_jit_supports_subprog_tailcalls(void);
> > bool bpf_jit_supports_kfunc_call(void);
> > bool bpf_jit_supports_far_kfunc_call(void);
> > +bool bpf_jit_supports_exceptions(void);
> > +void arch_bpf_stack_walk(bool (*consume_fn)(void *cookie, u64 ip, u64 sp, u64 bp), void *cookie);
> > bool bpf_helper_changes_pkt_data(void *func);
> >
> > static inline bool bpf_dump_raw_ok(const struct cred *cred)
> > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > index 5c484b2bc3d6..5e224cf0ec27 100644
> > --- a/kernel/bpf/core.c
> > +++ b/kernel/bpf/core.c
> > @@ -2770,6 +2770,15 @@ int __weak bpf_arch_text_invalidate(void *dst, size_t len)
> > return -ENOTSUPP;
> > }
> >
> > +bool __weak bpf_jit_supports_exceptions(void)
> > +{
> > + return false;
> > +}
> > +
> > +void __weak arch_bpf_stack_walk(bool (*consume_fn)(void *cookie, u64 ip, u64 sp, u64 bp), void *cookie)
> > +{
> > +}
> > +
> > #ifdef CONFIG_BPF_SYSCALL
> > static int __init bpf_global_ma_init(void)
> > {
> > --
> > 2.40.1
> >
