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]

Message-ID: <20190614013051.6gnwduy4dsygbamj@treble>
Date:   Thu, 13 Jun 2019 20:30:51 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
        bpf@...r.kernel.org, Peter Zijlstra <peterz@...radead.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 Thu, Jun 13, 2019 at 03:00:55PM -0700, Alexei Starovoitov wrote:
> > @@ -392,8 +402,16 @@ bool unwind_next_frame(struct unwind_state *state)
> >  	 * calls and calls to noreturn functions.
> >  	 */
> >  	orc = orc_find(state->signal ? state->ip : state->ip - 1);
> > -	if (!orc)
> > -		goto err;
> > +	if (!orc) {
> > +		/*
> > +		 * As a fallback, try to assume this code uses a frame pointer.
> > +		 * This is useful for generated code, like BPF, which ORC
> > +		 * doesn't know about.  This is just a guess, so the rest of
> > +		 * the unwind is no longer considered reliable.
> > +		 */
> > +		orc = &orc_fp_entry;
> > +		state->error = true;
> 
> That seems fragile.

I don't think so.  The unwinder has sanity checks to make sure it
doesn't go off the rails.  And it works just fine.  The beauty is that
it should work for all generated code (not just BPF).

> Can't we populate orc_unwind tables after JIT ?

As I mentioned it would introduce a lot more complexity.  For each JIT
function, BPF would have to tell ORC the following:

- where the BPF function lives
- how big the stack frame is
- where RBP and other callee-saved regs are on the stack

There's a lot more fragility lurking there, compared to the above.

Not to mention the unwinder would need BPF-specific knowledge, unless we
created some generic abstraction for generated code to register their
functions (which we have actually considered in the past).  But the
above approach is much simpler: just have all generated code use frame
pointers.

-- 
Josh