On Wed, Mar 20, 2019 at 12:57 PM Xing, Cedric <cedric.x...@intel.com> wrote:
>
> > Using the untrusted stack as a way to exchange data is very convenient,
> > but that doesn't mean it's a good idea.  Here are some problems it
> > causes:
> >
> >  - It prevents using a normal function to wrap enclave entry (as we're
> > seeing with this patch set).
>
> It doesn't prevent. It's all about what's agreed between the enclave and its 
> hosting process. With the optional "exit/exception callback" set to null, 
> this will behave exactly the same as in the current patch. That's what I 
> meant by "flexibility" and "superset of functionality".
>
> >
> >  - It makes quite a few unfortunate assumptions about the layout of the
> > untrusted stack.  It assumes that the untrusted stack is arbitrarily
> > expandable, which is entirely untrue in languages like Go.
>
> I'm with you that stack is not always good thing, hence I'm NOT ruling out 
> any other approaches for exchanging data. But is stack "bad" enough to be 
> ruled out completely? The point here is flexibility because the stack could 
> be "good" for its convenience. After all, only buffers of "reasonable" sizes 
> will be exchanged in most cases, and in the rare exceptions of stack overflow 
> they'd probably get caught in validation anyway. The point here again is - 
> flexibility. I'd say it's better to leave the choice to the SDK implementers 
> than to force the choice on them.
>
> > It assumes that the untrusted stack isn't further constrained by various
> > CFI mechanisms (e.g. CET), and, as of last time I checked, the
> > interaction between CET and SGX was still not specified.
>
> I was one of the architects participating in the CET ISA definition. The 
> assembly provided was crafted with CET in mind and will be fully compatible 
> with CET.
>
> > It also
> > assumes that the untrusted stack doesn't have ABI-imposed layout
> > restrictions related to unwinding, and, as far as I know, this means
> > that current enclaves with current enclave runtimes can interact quite
> > poorly with debuggers, exception handling, and various crash dumping
> > technologies.
>
> Per comments from the patch set, I guess it's been agreed that this vDSO 
> function will NOT be x86_64 ABI compatible. So I'm not sure why stacking 
> unwinding is relevant here. However, I'm with you that we should take 
> debugging/exception handling/reporting/crash dumping into consideration by 
> making this vDSO API x86_64 ABI compatible. IMO it's trivial and the 
> performance overhead in negligible (dwarfed by ENCLU anyway. I'd be more than 
> happy to provide a x86_64 ABI compatible version if that's also your 
> preference.
>
> >  - It will make it quite unpleasant to call into an enclave in a
> > coroutine depending on how the host untrusted runtime implements
> > coroutines.
>
> I'm not sure what you are referring to by "coroutine". But this vDSO API will 
> be (expected to be) the only routine that actually calls into an enclave. 
> Isn't that correct?

I mean use in languages and runtimes that allow a function and its
callees to pause and then resume later.  Something like (pseudocode,
obviously):

void invoke_the_enclave()
{
  do_eenter_through_vdso();
}

void some_ocall_handler(void *ptr)
{
  yield;
}

If the enclave has ptr pointing to the untrusted stack, then this gets
quite awkward for the runtime to handle efficiently.  IMO a much nicer
approach would be:

void invoke_the_enclave()
{
  char buffer[1024];
  while (true)
  {
    eenter (through vdso);
    if (exit was an ocall request) {
      some_ocall_handler(buffer);
    }
  }
}

And now there is nothing funny happening behind the runtime's back
when some_ocall_handler tries to yield.

Reply via email to