Re: [PATCH 23/33] x86/asm/bpf: Create stack frames in bpf_jit.S

From	Alexei Starovoitov <alexei.starovoitov@gmail.com>
Newsgroups	linux.kernel
Subject	Re: [PATCH 23/33] x86/asm/bpf: Create stack frames in bpf_jit.S
Date	2016-01-22 18:20 +0100
Message-ID	<qTNxp-77w-13@gated-at.bofh.it> (permalink)
References	(1 earlier) <qTwmS-3nT-21@gated-at.bofh.it> <qTzXs-5Rs-5@gated-at.bofh.it> <qTB3d-6EO-63@gated-at.bofh.it> <qTBmz-74w-35@gated-at.bofh.it> <qTMhY-62g-13@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Fri, Jan 22, 2016 at 09:58:04AM -0600, Josh Poimboeuf wrote:
> On Thu, Jan 21, 2016 at 08:18:46PM -0800, Alexei Starovoitov wrote:
> > On Thu, Jan 21, 2016 at 09:55:31PM -0600, Josh Poimboeuf wrote:
> > > On Thu, Jan 21, 2016 at 06:44:28PM -0800, Alexei Starovoitov wrote:
> > > > On Thu, Jan 21, 2016 at 04:49:27PM -0600, Josh Poimboeuf wrote:
> > > > > bpf_jit.S has several callable non-leaf functions which don't honor
> > > > > CONFIG_FRAME_POINTER, which can result in bad stack traces.
> > > > > 
> > > > > Create a stack frame before the call instructions when
> > > > > CONFIG_FRAME_POINTER is enabled.
> > > > > 
> > > > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> > > > > Cc: Alexei Starovoitov <ast@kernel.org>
> > > > > Cc: netdev@vger.kernel.org
> > > > > ---
> > > > >  arch/x86/net/bpf_jit.S | 9 +++++++--
...
> > > > >  /* rsi contains offset and can be scratched */
> > > > >  #define bpf_slow_path_common(LEN)		\
> > > > > +	lea	-MAX_BPF_STACK + 32(%rbp), %rdx;\
> > > > > +	FRAME_BEGIN;				\
> > > > >  	mov	%rbx, %rdi; /* arg1 == skb */	\
> > > > >  	push	%r9;				\
> > > > >  	push	SKBDATA;			\
> > > > >  /* rsi already has offset */			\
> > > > >  	mov	$LEN,%ecx;	/* len */	\
> > > > > -	lea	- MAX_BPF_STACK + 32(%rbp),%rdx;			\
> > > > >  	call	skb_copy_bits;			\
> > > > >  	test    %eax,%eax;			\
> > > > >  	pop	SKBDATA;			\
> > > > > -	pop	%r9;
> > > > > +	pop	%r9;				\
> > > > > +	FRAME_END
...
> > > Well, but the point of these patches isn't to make the tool happy.  It's
> > > really to make sure that runtime stack traces can be made reliable.
> > > Maybe I'm missing something but I don't see why JIT code can't honor
> > > CONFIG_FRAME_POINTER just like any other code.
> > 
> > It can if there is no performance cost added.
> 
> CONFIG_FRAME_POINTER always adds a small performance cost but as you
> mentioned it only affects the slow path here.  And hopefully we'll soon
> have an in-kernel DWARF unwinder on x86 so we can get rid of the need
> for frame pointers.

ok. fair enough.
Acked-by: Alexei Starovoitov <ast@kernel.org>

Thread

[PATCH 23/33] x86/asm/bpf: Create stack frames in bpf_jit.S Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:00 +0100
  Re: [PATCH 23/33] x86/asm/bpf: Create stack frames in bpf_jit.S Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-01-22 03:50 +0100
    Re: [PATCH 23/33] x86/asm/bpf: Create stack frames in bpf_jit.S Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 05:00 +0100
      Re: [PATCH 23/33] x86/asm/bpf: Create stack frames in bpf_jit.S Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-01-22 05:20 +0100
        Re: [PATCH 23/33] x86/asm/bpf: Create stack frames in bpf_jit.S Ingo Molnar <mingo@kernel.org> - 2016-01-22 08:40 +0100
        Re: [PATCH 23/33] x86/asm/bpf: Create stack frames in bpf_jit.S Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 17:00 +0100
          Re: [PATCH 23/33] x86/asm/bpf: Create stack frames in bpf_jit.S Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-01-22 18:20 +0100
            Re: [PATCH 23/33] x86/asm/bpf: Create stack frames in bpf_jit.S Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 18:40 +0100
              Re: [PATCH 23/33] x86/asm/bpf: Create stack frames in bpf_jit.S Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-01-22 18:50 +0100

csiph-web