On Wed, Mar 18, 2020 at 05:38:29PM -0700, Xing, Cedric wrote: > On 3/18/2020 4:40 PM, Sean Christopherson wrote: > %rbx can always be restored as long as it is saved at a fixed offset from > %rbp. For example, given the standard prolog below: > > push %rbp > mov %rsp, %rbp > push %rbx > > It can be paired with the following standard epilog: > > mov -8(%rbp), %rbx > leave > ret > > Alternatively, given "red zone" of 128 bytes, the following epilog will also > work: > > leave > mov -0x10(%rsp), %rbx > ret > > In no cases do we have to worry about enclave mucking the stack as long as > %rbp is preserved. > > >>>>While this is more work, it is standard calling convention work that > >>>>doesn't require internal knowledge of __vdso..(). Alternatively, if we > >>>>don't like the extra work, we can document the %rbx hack explicitly > >>>>into the handler documentation and make it part of the interface. But > >>>>we need some explicit way for the handler to pop enclave output stack > >>>>params that doesn't depend on internal knowledge of the __vdso...() > >>>>invariants. > >>> > >>>IIUC, this is what you're suggesting? Having to align the stack makes this > >>>a bit annoying, but it's not bad by any means. > >>> > >>>diff --git a/arch/x86/entry/vdso/vsgx_enter_enclave.S b/arch/x86/entry/vdso/vsgx_enter_enclave.S > >>>index 94a8e5f99961..05d54f79b557 100644 > >>>--- a/arch/x86/entry/vdso/vsgx_enter_enclave.S > >>>+++ b/arch/x86/entry/vdso/vsgx_enter_enclave.S > >>>@@ -139,8 +139,9 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave) > >>> /* Pass the untrusted RSP (at exit) to the callback via %rcx. */ > >>> mov %rsp, %rcx > >>> > >>>- /* Save the untrusted RSP in %rbx (non-volatile register). */ > >>>+ /* Save the untrusted RSP offset in %rbx (non-volatile register). */ > >>> mov %rsp, %rbx > >>>+ and $0xf, %rbx > >>> > >>> /* > >>> * Align stack per x86_64 ABI. Note, %rsp needs to be 16-byte aligned > >>>@@ -161,8 +162,8 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave) > >>> mov 0x20(%rbp), %rax > >>> call .Lretpoline > >>> > >>>- /* Restore %rsp to its post-exit value. */ > >>>- mov %rbx, %rsp > >>>+ /* Undo the post-exit %rsp adjustment. */ > >>>+ lea 0x20(%rsp,%rbx), %rsp > >>> > > Per discussion above, this is useful only if the enclave has problem > cleaning up its own mess left on the untrusted stack, and the exit handler > wants to EENTER the enclave again by returning to __vdso...(). It sounds > very uncommon to me, and more like a bug than an expected behavior. Are > there any existing code doing this or any particular application that needs > this. If no, I'd say not to do it. Ya, I'm on the fence as well. The only counter-argument is that doing: push %rbp mov %rsp, %rbp push %rbx ... pop %rbx leave ret with the relative adjustment would allow the exit handler (or enclave) to change %rbx. I'm not saying that is remote sane, but if we're going for maximum flexibility... Anyways, patches incoming, let's discuss there.
