From: Alexei Starovoitov <alexei.starovoitov@gmail•com>
To: Josh Poimboeuf <jpoimboe@redhat•com>
Cc: Thomas Gleixner <tglx@linutronix•de>,
Ingo Molnar <mingo@redhat•com>, "H. Peter Anvin" <hpa@zytor•com>,
x86@kernel•org, linux-kernel@vger•kernel.org,
live-patching@vger•kernel.org, Michal Marek <mmarek@suse•cz>,
Peter Zijlstra <peterz@infradead•org>,
Andy Lutomirski <luto@kernel•org>, Borislav Petkov <bp@alien8•de>,
Linus Torvalds <torvalds@linux-foundation•org>,
Andi Kleen <andi@firstfloor•org>, Pedro Alves <palves@redhat•com>,
Namhyung Kim <namhyung@gmail•com>,
Bernd Petrovitsch <bernd@petrovitsch•priv.at>,
Chris J Arges <chris.j.arges@canonical•com>,
Andrew Morton <akpm@linux-foundation•org>,
Jiri Slaby <jslaby@suse•cz>,
Arnaldo Carvalho de Melo <acme@kernel•org>,
Alexei Starovoitov <ast@kernel•org>,
netdev@vger•kernel.org, daniel@iogearbox•net
Subject: Re: [PATCH 23/33] x86/asm/bpf: Create stack frames in bpf_jit.S
Date: Thu, 21 Jan 2016 18:44:28 -0800 [thread overview]
Message-ID: <20160122024427.GA6000@ast-mbp.thefacebook.com> (raw)
In-Reply-To: <fa4c41976b438b51954cb8021f06bceb1d1d66cc.1453405861.git.jpoimboe@redhat.com>
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 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/net/bpf_jit.S b/arch/x86/net/bpf_jit.S
> index eb4a3bd..f2a7faf 100644
> --- a/arch/x86/net/bpf_jit.S
> +++ b/arch/x86/net/bpf_jit.S
> @@ -8,6 +8,7 @@
> * of the License.
> */
> #include <linux/linkage.h>
> +#include <asm/frame.h>
>
> /*
> * Calling convention :
> @@ -65,16 +66,18 @@ FUNC(sk_load_byte_positive_offset)
>
> /* 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
I'm not sure what above is doing.
There is already 'push rbp; mov rbp,rsp' at the beginning of generated
code and with above the stack trace will show two function at the same ip?
since there were no calls between them?
I think the stack walker will get even more confused?
Also the JIT of bpf_call insn will emit variable number of push/pop
around the call and I definitely don't want to add extra push rbp
there, since it's the critical path and callee will do its own
push rbp.
Also there are push/pops emitted around div/mod
and there is indirect goto emitted as well for bpf_tail_call
that jumps into different function body without touching
current stack.
Also none of the JITed function are dwarf annotated.
I could be missing something. I think either this patch
is not need or you need to teach the tool to ignore
all JITed stuff. I don't think it's practical to annotate
everything. Different JITs do their own magic.
s390 JIT is even more fancy.
next prev parent reply other threads:[~2016-01-22 2:44 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-21 22:49 [PATCH 00/33] Compile-time stack metadata validation Josh Poimboeuf
2016-01-21 22:49 ` [PATCH 22/33] x86/asm/bpf: Annotate callable functions Josh Poimboeuf
2016-01-21 22:49 ` [PATCH 23/33] x86/asm/bpf: Create stack frames in bpf_jit.S Josh Poimboeuf
2016-01-22 2:44 ` Alexei Starovoitov [this message]
2016-01-22 3:55 ` Josh Poimboeuf
2016-01-22 4:18 ` Alexei Starovoitov
2016-01-22 7:36 ` Ingo Molnar
2016-01-22 15:58 ` Josh Poimboeuf
2016-01-22 17:18 ` Alexei Starovoitov
2016-01-22 17:36 ` Josh Poimboeuf
2016-01-22 17:40 ` Alexei Starovoitov
2016-01-21 22:49 ` [PATCH 31/33] bpf: Add __bpf_prog_run() to stacktool whitelist Josh Poimboeuf
2016-01-21 22:57 ` Daniel Borkmann
2016-01-22 2:55 ` Alexei Starovoitov
2016-01-22 4:13 ` Josh Poimboeuf
2016-01-22 17:19 ` Alexei Starovoitov
2016-01-22 17:43 ` [PATCH 00/33] Compile-time stack metadata validation Chris J Arges
[not found] ` <20160122174348.GB29221-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
2016-01-22 19:14 ` Josh Poimboeuf
[not found] ` <20160122191447.GH20502-8wJ5/zUtDR0XGNroddHbYwC/G2K4zDHf@public.gmane.org>
2016-01-22 20:40 ` Chris J Arges
[not found] ` <20160122204034.GA5826-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
2016-01-22 20:47 ` Josh Poimboeuf
2016-02-12 10:36 ` Jiri Slaby
2016-02-12 10:41 ` Jiri Slaby
2016-02-12 14:45 ` Josh Poimboeuf
2016-02-12 17:10 ` Peter Zijlstra
2016-02-12 18:32 ` Josh Poimboeuf
[not found] ` <20160212183206.GB29004-8wJ5/zUtDR0XGNroddHbYwC/G2K4zDHf@public.gmane.org>
2016-02-12 18:34 ` Josh Poimboeuf
2016-02-12 20:10 ` Peter Zijlstra
2016-02-15 16:31 ` Josh Poimboeuf
[not found] ` <20160215163134.GA20585-8wJ5/zUtDR0XGNroddHbYwC/G2K4zDHf@public.gmane.org>
2016-02-15 16:49 ` Peter Zijlstra
[not found] ` <CA+55aFzoPCd_LcSx1FUuEhSBYk2KrfzXGj-Vcn39W5bz=KuZhA@mail.gmail.com>
2016-02-15 20:01 ` Josh Poimboeuf
[not found] ` <CA+55aFzoPCd_LcSx1FUuEhSBYk2KrfzXGj-Vcn39W5bz=KuZhA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-15 20:02 ` Andi Kleen
2016-02-23 8:14 ` Ingo Molnar
[not found] ` <20160223081406.GA606-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-23 14:27 ` Arnaldo Carvalho de Melo
2016-02-23 15:07 ` Josh Poimboeuf
2016-02-23 15:28 ` Arnaldo Carvalho de Melo
2016-02-23 15:01 ` Josh Poimboeuf
2016-02-24 7:40 ` Ingo Molnar
[not found] ` <20160224074054.GA13199-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-24 16:32 ` Josh Poimboeuf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160122024427.GA6000@ast-mbp.thefacebook.com \
--to=alexei.starovoitov@gmail$(echo .)com \
--cc=acme@kernel$(echo .)org \
--cc=akpm@linux-foundation$(echo .)org \
--cc=andi@firstfloor$(echo .)org \
--cc=ast@kernel$(echo .)org \
--cc=bernd@petrovitsch$(echo .)priv.at \
--cc=bp@alien8$(echo .)de \
--cc=chris.j.arges@canonical$(echo .)com \
--cc=daniel@iogearbox$(echo .)net \
--cc=hpa@zytor$(echo .)com \
--cc=jpoimboe@redhat$(echo .)com \
--cc=jslaby@suse$(echo .)cz \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=live-patching@vger$(echo .)kernel.org \
--cc=luto@kernel$(echo .)org \
--cc=mingo@redhat$(echo .)com \
--cc=mmarek@suse$(echo .)cz \
--cc=namhyung@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=palves@redhat$(echo .)com \
--cc=peterz@infradead$(echo .)org \
--cc=tglx@linutronix$(echo .)de \
--cc=torvalds@linux-foundation$(echo .)org \
--cc=x86@kernel$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox