netdev - Re: [PATCH 7/9] x86/unwind/orc: Fall back to using frame pointers for generated code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 14 Jun 2019 08:29:06 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Josh Poimboeuf <jpoimboe@...hat.com>, X86 ML <x86@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Network Development <netdev@...r.kernel.org>,
        bpf <bpf@...r.kernel.org>, Song Liu <songliubraving@...com>,
        Kairui Song <kasong@...hat.com>
Subject: Re: [PATCH 7/9] x86/unwind/orc: Fall back to using frame pointers for
 generated code

On Fri, Jun 14, 2019 at 12:41 AM Peter Zijlstra <peterz@...radead.org> wrote:
>
> On Thu, Jun 13, 2019 at 11:00:09PM -0700, Alexei Starovoitov wrote:
>
> > There is something wrong with
> > commit d15d356887e7 ("perf/x86: Make perf callchains work without CONFIG_FRAME_POINTER")
>
> It assumes we can always unwind stack, which is, imo, not a weird thing.
>
> > If I simply revert it and have CONFIG_UNWINDER_FRAME_POINTER=y
> > JITed stacks work just fine, because
> > bpf_get_stackid()->get_perf_callchain()
> > need to start unwinding before any bpf stuff.
>
> How does stack unwinding work if we try and unwind from an interrupt
> that hits inside a BPF program? That too needs to work properly.
>
> > After that commit it needs to go through which is a bug on its own.
> > imo patch 1 doesn't really fix that issue.
>
> This we agree on, patch 1 doesn't solve that at all. But we also should
> not loose the initial regs->ip value.
>
> > As far as mangled rbp can we partially undo old
> > commit 177366bf7ceb ("bpf: change x86 JITed program stack layout")
> > that introduced that rbp adjustment.
>
> > Going through bpf code is only interesting in case of panics somewhere
> > in bpf helpers. Back then we didn't even have ksym of jited code.
>
> I disagree here, interrupts/NMIs hitting inside BPF should be able to
> reliably unwind the entire stack. Back then is irrelevant, these days we
> expect a reliable unwind.
>
> > Anyhow I agree that we need to make the jited frame proper,
> > but unwinding need to start before any bpf stuff.
> > That's a bigger issue.
>
> I strongly disagree, we should be able to unwind through bpf.

I'm not saying that we should not.
I'm saying that unwinding through jited code was not working
for long time and it wasn't an issue. The issue is start of unwind.
This is user expected behavior that we cannot just break.
The users expect stack trace to be the same JITed vs interpreted.
Hence unwinder should start before interpreter/jit.
That's the first problem to address.
In parallel we can discuss how to make unwinding work through jited code.