| Message ID | 20200319011130.8556-3-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/sgx: Make vDSO callable from C | expand |
On Wed, Mar 18, 2020 at 06:11:24PM -0700, Sean Christopherson wrote: > Modify the %rsp fixup after returning from the exit handler to be > relative instead of absolute to avoid clobbering any %rsp adjustments > made by the exit handler, e.g. if the exit handler modifies the stack > prior to re-entering the enclave. > > Reported-by: Nathaniel McCallum <npmccallum@redhat.com> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > > I'm on the fence as to whether or not this is a good idea. It's not super > painful, but it's not exactly standard/obvious code. Part of me thinks > its a bug to not let the exit handler manipulate %rsp, the other part of > me thinks it's straight up crazy :-) After some hours of processing this, I think this makes sense. It makes the interface more robust. This is not printf(). /Jarkko
On Fri, Mar 20, 2020 at 04:39:51AM +0200, Jarkko Sakkinen wrote: > On Wed, Mar 18, 2020 at 06:11:24PM -0700, Sean Christopherson wrote: > > Modify the %rsp fixup after returning from the exit handler to be > > relative instead of absolute to avoid clobbering any %rsp adjustments > > made by the exit handler, e.g. if the exit handler modifies the stack > > prior to re-entering the enclave. > > > > Reported-by: Nathaniel McCallum <npmccallum@redhat.com> > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > > --- > > > > I'm on the fence as to whether or not this is a good idea. It's not super > > painful, but it's not exactly standard/obvious code. Part of me thinks > > its a bug to not let the exit handler manipulate %rsp, the other part of > > me thinks it's straight up crazy :-) > > After some hours of processing this, I think this makes sense. > > It makes the interface more robust. This is not printf(). Has been merged. /Jarkko
diff --git a/arch/x86/entry/vdso/vsgx_enter_enclave.S b/arch/x86/entry/vdso/vsgx_enter_enclave.S index 22a22e0774d8..14f07d5e47ae 100644 --- a/arch/x86/entry/vdso/vsgx_enter_enclave.S +++ b/arch/x86/entry/vdso/vsgx_enter_enclave.S @@ -137,8 +137,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 @@ -159,8 +160,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 /* * If the return from callback is zero or negative, return immediately,
Modify the %rsp fixup after returning from the exit handler to be relative instead of absolute to avoid clobbering any %rsp adjustments made by the exit handler, e.g. if the exit handler modifies the stack prior to re-entering the enclave. Reported-by: Nathaniel McCallum <npmccallum@redhat.com> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- I'm on the fence as to whether or not this is a good idea. It's not super painful, but it's not exactly standard/obvious code. Part of me thinks its a bug to not let the exit handler manipulate %rsp, the other part of me thinks it's straight up crazy :-) arch/x86/entry/vdso/vsgx_enter_enclave.S | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)