linux-kernel - Re: [RFC PATCH v2 4/4] x86/vdso: Add __vdso_sgx_enter_enclave() to wrap SGX enclave transitions

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, 7 Dec 2018 12:09:35 -0800
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Andy Lutomirski <luto@...capital.net>
Cc:     Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        X86 ML <x86@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        Josh Triplett <josh@...htriplett.org>,
        linux-sgx@...r.kernel.org, haitao.huang@...ux.intel.com,
        Jethro Beekman <jethro@...tanix.com>,
        "Dr. Greg Wettstein" <greg@...ellic.com>
Subject: Re: [RFC PATCH v2 4/4] x86/vdso: Add __vdso_sgx_enter_enclave() to
 wrap SGX enclave transitions

On Fri, Dec 07, 2018 at 11:23:10AM -0800, Andy Lutomirski wrote:
> 
> > On Dec 7, 2018, at 11:02 AM, Sean Christopherson <sean.j.christopherson@...el.com> wrote:
> > 
> >> On Fri, Dec 07, 2018 at 09:56:09AM -0800, Andy Lutomirski wrote:
> >> On Fri, Dec 7, 2018 at 8:51 AM Sean Christopherson
> >> <sean.j.christopherson@...el.com> wrote:
> >>> I like that the exit handler allows userspace to trap/panic with the full
> >>> call stack in place, and in a dedicated path, i.e. outside of the basic
> >>> enter/exit code.  An exit handler probably doesn't fundamentally change
> >>> what userspace can do with respect to debugging/reporting, but I think
> >>> it would actually simplify some userspace implementations, e.g. I'd use
> >>> it in my tests like so:
> >>> 
> >>> long fault_handler(struct sgx_enclave_exit_info *exit_info, void *tcs, void *priv)
> >>> {
> >>>        if (exit_info->leaf == SGX_EEXIT)
> >>>                return 0;
> >>> 
> >>>        <report exception and die/hang>
> >>> }
> >>> 
> >> 
> >> Hmm.  That't not totally silly, although you could accomplish almost
> >> the same thing by wrapping the vDSO helper in another function.
> > 
> > True, but I think there's value in having the option to intercept an
> > exception at the exact(ish) point of failure, without having to return
> > up the stack.
> > 
> > The enclave has full access to the process' memory space, including the
> > untrsuted stack.  It's not too far fetched to envision a scenario where
> > the enclave corrupts the stack even if the enclave isn't intentionally
> > using the stack, e.g. the host passes in variable that a points at the
> > stack instead of whatever memory is supposed to be shared between the
> > enclave and host.  It'd be nice to have the ability to triage something
> > like that without having to e.g. configure breakpoints on the stack.
> 
> Ah, I see. You’re saying that, if the non-enclave stare is corrupted such
> that RIP  is okay and RSP still points somewhere reasonable but the return
> address is garbage, then we can at least get to the fault handler and print
> something?

Yep.  Even for something more subtle like GPR corruption it could dump the
entire call stack before attempting to return back up.

> This only works if the fault handler pointer itself is okay, though, which
> somewhat limits the usefulness, given that its pointer is quite likely to
> be on the stack very close to the return address.

Yeah, it's not a silver bullet by any means, but it does seem useful for at
least some scenarios.  Even exploding when invoking the handler instead of
at a random point might prove useful, e.g. "calling my exit handler exploded,
maybe my enclave corrupted the stack!".
 
> I really wish the ENCLU instruction had seen fit to preserve some registers.

Speaking of preserving registers, the asm blob needs to mark RBX as
clobbered since it's modified for EEXIT.