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