| Message ID | cover.1562813643.git.cedric.xing@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | x86/sgx: Amend vDSO API to allow enclave/host parameter passing on untrusted stack | expand |
On Wed, Jul 10, 2019 at 09:21:30PM -0700, Cedric Xing wrote: > This patchset is based upon, and can be applied cleanly on SGX1 patch v20 > (https://lkml.org/lkml/2019/4/17/344) by Jarkko Sakkinen. > > The current proposed __vdso_sgx_enter_enclave() requires enclaves to preserve > %rsp, which prohibits enclaves from allocating space on the untrusted stack. > However, there are existing enclaves (e.g. those built with current Intel SGX > SDK libraries) relying on the untrusted stack for passing parameters to > untrusted functions (aka. o-calls), which requires allocating space on the > untrusted stack by enclaves. After all, passing data via untrusted stack is > very easy to implement (by enclaves), with essentially no overhead, therefore > is very suitable for exchanging data in small amounts, so could be desirable by > future SGX applications as well. > > This patchset introduces a new ABI for __vdso_sgx_enter_enclave() to anchor its > stack frame on %rbp (instead of %rsp), so as to allow enclaves to "push" onto > the untrusted stack by decrementing the untrusted %rsp. And in order to service > o-calls and to preserve the untrusted stack upon exceptions, the new vDSO API > takes one more optional parameter - "callback", which if supplied, will be > invoked on all enclave exits (including normal and asynchronous exits). Ample > details regarding the new ABI have been documented as comments inside the > source code located in arch/x86/entry/vsgx_enter_enclave.S > > Please note that there was a lengthy discussion on what is the "best" approach > for passing parameters for trusted/untrusted calls. Unfortunately there's no > single "best" approach that fits all use cases, hence this new ABI has been > designed intentionally to accommodate varieties. Therefore, to those not > interested in using the untrusted stack, whatever worked with the old ABI > proposed by Sean will continue to work with this new ABI. > > The SGX selftest has been augmented by two new tests. One exercises the new > callback interface, and serves as a simple example to showcase how to use it; > while the other validates the hand-crafted CFI directives in > __vdso_sgx_enter_enclave() by single-stepping through it and unwinding call > stack at every instruction. Please note that the selftest CANNOT run to > completion yet, as it depends on the vDSO fixup code to signal the process upon > #DB/#BP inside enclaves (rather than the current behavior of branching to the > handler in vDSO). > > Changelog: > · This is version 3 of this patch series with the following changes. > - Per Andy Lutomirski and Sean Christopherson, revised comments and their > format in arch/x86/entry/vsgx_enter_enclave.S > - Per Jarkko Sakkinen, revised the cover letter to articulate motivation > and objective of this patchset. > · v2 - https://patchwork.kernel.org/cover/10914161/ > · v1 - https://patchwork.kernel.org/cover/10911615/ 1. I agree with the high level idea. 2. The patches do changes out of scope. Generally, when doing kernel patches, even for a running patch set, please do not do anything extra. It is also stated in the kernel process; https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#separate-your-changes Once these are fully cleaned up we can merge them. /Jarkko
This patchset is based upon, and can be applied cleanly on SGX1 patch v20 (https://lkml.org/lkml/2019/4/17/344) by Jarkko Sakkinen. The current proposed __vdso_sgx_enter_enclave() requires enclaves to preserve %rsp, which prohibits enclaves from allocating space on the untrusted stack. However, there are existing enclaves (e.g. those built with current Intel SGX SDK libraries) relying on the untrusted stack for passing parameters to untrusted functions (aka. o-calls), which requires allocating space on the untrusted stack by enclaves. After all, passing data via untrusted stack is very easy to implement (by enclaves), with essentially no overhead, therefore is very suitable for exchanging data in small amounts, so could be desirable by future SGX applications as well. This patchset introduces a new ABI for __vdso_sgx_enter_enclave() to anchor its stack frame on %rbp (instead of %rsp), so as to allow enclaves to "push" onto the untrusted stack by decrementing the untrusted %rsp. And in order to service o-calls and to preserve the untrusted stack upon exceptions, the new vDSO API takes one more optional parameter - "callback", which if supplied, will be invoked on all enclave exits (including normal and asynchronous exits). Ample details regarding the new ABI have been documented as comments inside the source code located in arch/x86/entry/vsgx_enter_enclave.S Please note that there was a lengthy discussion on what is the "best" approach for passing parameters for trusted/untrusted calls. Unfortunately there's no single "best" approach that fits all use cases, hence this new ABI has been designed intentionally to accommodate varieties. Therefore, to those not interested in using the untrusted stack, whatever worked with the old ABI proposed by Sean will continue to work with this new ABI. The SGX selftest has been augmented by two new tests. One exercises the new callback interface, and serves as a simple example to showcase how to use it; while the other validates the hand-crafted CFI directives in __vdso_sgx_enter_enclave() by single-stepping through it and unwinding call stack at every instruction. Please note that the selftest CANNOT run to completion yet, as it depends on the vDSO fixup code to signal the process upon #DB/#BP inside enclaves (rather than the current behavior of branching to the handler in vDSO). Changelog: · This is version 3 of this patch series with the following changes. - Per Andy Lutomirski and Sean Christopherson, revised comments and their format in arch/x86/entry/vsgx_enter_enclave.S - Per Jarkko Sakkinen, revised the cover letter to articulate motivation and objective of this patchset. · v2 - https://patchwork.kernel.org/cover/10914161/ · v1 - https://patchwork.kernel.org/cover/10911615/ Cedric Xing (3): selftests/x86: Fixed Makefile for SGX selftest x86/vdso: Modify __vdso_sgx_enter_enclave() to allow parameter passing on untrusted stack selftests/x86: Augment SGX selftest to test new __vdso_sgx_enter_enclave() and its callback interface arch/x86/entry/vdso/vsgx_enter_enclave.S | 214 ++++++++++---- arch/x86/include/uapi/asm/sgx.h | 14 +- tools/testing/selftests/x86/Makefile | 12 +- tools/testing/selftests/x86/sgx/Makefile | 49 ++-- tools/testing/selftests/x86/sgx/main.c | 323 ++++++++++++++++++--- tools/testing/selftests/x86/sgx/sgx_call.S | 40 ++- 6 files changed, 500 insertions(+), 152 deletions(-)