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

[Andy Lutomirski <luto@amacapital.net>]

> On Dec 7, 2018, at 12:09 PM, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
> 
>> 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@intel.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@intel.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!".

Here’s another idea: calculate some little hash or other checksum of RSP, RBP, and perhaps a couple words on the stack, and do:

call __vdso_enclave_corrupted_state

If you get a mismatch after return. That function could be:

call __vdso_enclave_corrupted_state:
  ud2

And now the debug trace makes it very clear what happened.

This may or may not be worth the effort. But ISTM the enclave is almost as likely to corrupt the host state and the. EEXIT as it is to corrupt the host state and then fault.

BTW, I read through Fortanix’s documentation of the Windows SGX calling conventions, and it seems to want RSI and RDI as out params.  Letting the vDSO be used to invoke Windows-targeted enclaves seems useful.

> 
>> 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.

Have fun with that.  The x86_32 compiler seems to really like having its PIC register preserved, and you may get some lovely compiler errors.