Re: [PATCH v28 21/22] x86/vdso: Implement a vDSO for Intel SGX enclave call

From: Nathaniel McCallum
Date: Fri Mar 13 2020 - 12:08:12 EST


On Thu, Mar 12, 2020 at 8:52 PM Sean Christopherson
<sean.j.christopherson@xxxxxxxxx> wrote:
>
> On Wed, Mar 11, 2020 at 03:30:44PM -0400, Nathaniel McCallum wrote:
> > On Tue, Mar 3, 2020 at 6:40 PM Jarkko Sakkinen
> > <jarkko.sakkinen@xxxxxxxxxxxxxxx> wrote:
> > > + * The exit handler's return value is interpreted as follows:
> > > + * >0: continue, restart __vdso_sgx_enter_enclave() with @ret as @leaf
> > > + * 0: success, return @ret to the caller
> > > + * <0: error, return @ret to the caller
> > > + *
> > > + * The userspace exit handler is responsible for unwinding the stack, e.g. to
> > > + * pop @e, u_rsp and @tcs, prior to returning to __vdso_sgx_enter_enclave().
> >
> > Unless I misunderstand, this documentation...
>
> Hrm, that does appear wrong. I'm guessing that was leftover from a previous
> incarnation of the code. Or I botched the description, which is just as
> likely.

I figured out what happened on my end. This documentation error led me
to misread the code. More below.

> > > + * The exit handler may also transfer control, e.g. via longjmp() or a C++
> > > + * exception, without returning to __vdso_sgx_enter_enclave().
> > > + *
> > > + * Return:
> > > + * 0 on success,
> > > + * -EINVAL if ENCLU leaf is not allowed,
> > > + * -EFAULT if an exception occurs on ENCLU or within the enclave
> > > + * -errno for all other negative values returned by the userspace exit handler
> > > + */
>
> ...
>
> > > + /* Load the callback pointer to %rax and invoke it via retpoline. */
> > > + mov 0x20(%rbp), %rax
> > > + call .Lretpoline
> > > +
> > > + /* Restore %rsp to its post-exit value. */
> > > + mov %rbx, %rsp
> >
> > ... doesn't seem to match this code.
> >
> > If the handler pops from the stack and then we restore the stack here,
> > the handler had no effect.
> >
> > Also, one difference between this interface and a raw ENCLU[EENTER] is
> > that we can't pass arguments on the untrusted stack during EEXIT. If
> > we want to support that workflow, then we need to allow the ability
> > for the handler to pop "additional" values without restoring the stack
> > pointer to the exact value here.
>
> > Also, one difference between this interface and a raw ENCLU[EENTER] is
> > that we can't pass arguments on the untrusted stack during EEXIT. If
> > we want to support that workflow, then we need to allow the ability
> > for the handler to pop "additional" values without restoring the stack
> > pointer to the exact value here.
>
> The callback shenanigans exist precisely to allow passing arguments on the
> untrusted stack. The vDSO is very careful to preserve the stack memory
> above RSP, and to snapshot RSP at the time of exit, e.g. the arguments in
> memory and their addresses relative to u_rsp live across EEXIT. It's the
> same basic concept as regular function calls, e.g. the callee doesn't pop
> params off the stack, it just knows what addresses relative to RSP hold
> the data it wants. The enclave, being the caller, is responsible for
> cleaning up u_rsp.
>
> FWIW, if the handler reaaaly wanted to pop off the stack, it could do so,
> fixup the stack, and then re-call __vdso_sgx_enter_enclave() instead of
> returning (to the original __vdso_sgx_enter_enclave()).

My understanding from the documentation issue above was that *if* you
wanted to push parameters back on the stack during enclave exit, you
would *have* to supply a handler so it could pop the parameters and
reset the stack. Which is why restoring %rsp from %rbx didn't make
sense to me.

Related to my other message in this thread, if
__vdso_sgx_enter_enclave() preserved %rbx and took @leaf as a stack
parameter, you could call __vdso_sgx_enter_enclave() from C so long as
the enclave didn't push return arguments on the stack. A workaround
for that case would be to fix up the stack in the handler. It would be
enough for the handler to simply set %rbx to the desired stack
location and return (though all of this unclean of course).

> > > + /*
> > > + * If the return from callback is zero or negative, return immediately,
> > > + * else re-execute ENCLU with the postive return value interpreted as
> > > + * the requested ENCLU leaf.
> > > + */
> > > + cmp $0, %eax
> > > + jle .Lout
> > > + jmp .Lenter_enclave
> > > +
> > > +.Lretpoline:
> > > + call 2f
> > > +1: pause
> > > + lfence
> > > + jmp 1b
> > > +2: mov %rax, (%rsp)
> > > + ret
> > > + .cfi_endproc
> > > +
> > > +_ASM_VDSO_EXTABLE_HANDLE(.Lenclu_eenter_eresume, .Lhandle_exception)
>