From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED, USER_AGENT_MUTT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 73AE9C282DD for ; Tue, 23 Apr 2019 19:31:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3B86C218C3 for ; Tue, 23 Apr 2019 19:31:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726536AbfDWTb5 (ORCPT ); Tue, 23 Apr 2019 15:31:57 -0400 Received: from mga14.intel.com ([192.55.52.115]:3570 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725945AbfDWTb5 (ORCPT ); Tue, 23 Apr 2019 15:31:57 -0400 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 23 Apr 2019 12:31:55 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,386,1549958400"; d="scan'208";a="164177286" Received: from sjchrist-coffee.jf.intel.com (HELO linux.intel.com) ([10.54.74.181]) by fmsmga002.fm.intel.com with ESMTP; 23 Apr 2019 12:26:54 -0700 Date: Tue, 23 Apr 2019 12:26:54 -0700 From: Sean Christopherson To: Cedric Xing Cc: linux-kernel@vger.kernel.org, x86@kernel.org, linux-sgx@vger.kernel.org, akpm@linux-foundation.org, Hansen@linux.intel.com, Dave , Christopherson@linux.intel.com, nhorman@redhat.com, npmccallum@redhat.com, Ayoun@linux.intel.com, Serge , Katz-zamir@linux.intel.com, Shay , Huang@linux.intel.com, Haitao , andriy.shevchenko@linux.intel.com, tglx@linutronix.de, Svahn@linux.intel.com, Kai , bp@alien8.de, josh@joshtriplett.org, luto@kernel.org, Kai , rientjes@google.com, Jarkko Sakkinen Subject: Re: [RFC PATCH v1 2/3] x86/vdso: Modify __vdso_sgx_enter_enclave() to allow parameter passing on untrusted stack Message-ID: <20190423192654.GA12691@linux.intel.com> References: <20190417103938.7762-1-jarkko.sakkinen@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Apr 22, 2019 at 05:37:24PM -0700, Cedric Xing wrote: > The previous __vdso_sgx_enter_enclave() requires enclaves to preserve %rsp, > which prohibits enclaves from allocating and passing parameters for > untrusted function calls (aka. o-calls). > > This patch addresses the problem above by introducing a new ABI that preserves > %rbp instead of %rsp. Then __vdso_sgx_enter_enclave() can anchor its frame > using %rbp so that enclaves are allowed to allocate space on the untrusted > stack by decrementing %rsp. Please note that the stack space allocated in such > way will be part of __vdso_sgx_enter_enclave()'s frame so will be freed after > __vdso_sgx_enter_enclave() returns. Therefore, __vdso_sgx_enter_enclave() has > been changed to take a callback function as an optional parameter, which if > supplied, will be invoked upon enclave exits (both AEX (Asynchronous Enclave > eXit) and normal exits), with the value of %rsp left > off by the enclave as a parameter to the callback. > > Here's the summary of API/ABI changes in this patch. More details could be > found in arch/x86/entry/vdso/vsgx_enter_enclave.S. > * 'struct sgx_enclave_exception' is renamed to 'struct sgx_enclave_exinfo' > because it is filled upon both AEX (i.e. exceptions) and normal enclave > exits. > * __vdso_sgx_enter_enclave() anchors its frame using %rbp (instead of %rsp in > the previous implementation). > * __vdso_sgx_enter_enclave() takes one more parameter - a callback function to > be invoked upon enclave exits. This callback is optional, and if not > supplied, will cause __vdso_sgx_enter_enclave() to return upon enclave exits > (same behavior as previous implementation). > * The callback function is given as a parameter the value of %rsp at enclave > exit to address data "pushed" by the enclave. A positive value returned by > the callback will be treated as an ENCLU leaf for re-entering the enclave, > while a zero or negative value will be passed through as the return > value of __vdso_sgx_enter_enclave() to its caller. It's also safe to > leave callback by longjmp() or by throwing a C++ exception. > > Signed-off-by: Cedric Xing > --- > arch/x86/entry/vdso/vsgx_enter_enclave.S | 156 ++++++++++++++--------- > arch/x86/include/uapi/asm/sgx.h | 14 +- > 2 files changed, 100 insertions(+), 70 deletions(-) > > diff --git a/arch/x86/entry/vdso/vsgx_enter_enclave.S b/arch/x86/entry/vdso/vsgx_enter_enclave.S > index fe0bf6671d6d..210f4366374a 100644 > --- a/arch/x86/entry/vdso/vsgx_enter_enclave.S > +++ b/arch/x86/entry/vdso/vsgx_enter_enclave.S > @@ -14,88 +14,118 @@ > .code64 > .section .text, "ax" > > -#ifdef SGX_KERNEL_DOC This #ifdef and the pseudo-C code below has a functional purpose. From the original commit: Note, the C-like pseudocode describing the assembly routine is wrapped in a non-existent macro instead of in a comment to trick kernel-doc into auto-parsing the documentation and function prototype. This is a double win as the pseudocode is intended to aid kernel developers, not userland enclave developers. We don't need full pseudocode, but a C-like prototype is necessary to get the kernel-doc comment parsed correctly. I'll try to look at the actual code later today. > /** > * __vdso_sgx_enter_enclave() - Enter an SGX enclave > * > * @leaf: **IN \%eax** - ENCLU leaf, must be EENTER or ERESUME > - * @tcs: **IN \%rbx** - TCS, must be non-NULL > - * @ex_info: **IN \%rcx** - Optional 'struct sgx_enclave_exception' pointer > + * @tcs: **IN 0x08(\%rsp)** - TCS, must be non-NULL > + * @ex_info: **IN 0x10(\%rsp)** - Optional 'struct sgx_enclave_exinfo' > + * pointer > + * @callback: **IN 0x18(\%rsp)** - Optional callback function to be called on > + * enclave exit or exception > * > * Return: > * **OUT \%eax** - > - * %0 on a clean entry/exit to/from the enclave, %-EINVAL if ENCLU leaf is > - * not allowed or if TCS is NULL, %-EFAULT if ENCLU or the enclave faults > + * %0 on a clean entry/exit to/from the enclave, %-EINVAL if ENCLU leaf is not > + * allowed, %-EFAULT if ENCLU or the enclave faults, or a non-positive value > + * returned from ``callback`` (if one is supplied). > * > * **Important!** __vdso_sgx_enter_enclave() is **NOT** compliant with the > - * x86-64 ABI, i.e. cannot be called from standard C code. As noted above, > - * input parameters must be passed via ``%eax``, ``%rbx`` and ``%rcx``, with > - * the return value passed via ``%eax``. All registers except ``%rsp`` must > - * be treated as volatile from the caller's perspective, including but not > - * limited to GPRs, EFLAGS.DF, MXCSR, FCW, etc... Conversely, the enclave > - * being run **must** preserve the untrusted ``%rsp`` and stack. > + * x86-64 ABI, i.e. cannot be called from standard C code. As noted above, > + * input parameters must be passed via ``%eax``, ``8(%rsp)``, ``0x10(%rsp)`` and > + * ``0x18(%rsp)``, with the return value passed via ``%eax``. All other registers > + * will be passed through to the enclave as is. All registers except ``%rbp`` > + * must be treated as volatile from the caller's perspective, including but not > + * limited to GPRs, EFLAGS.DF, MXCSR, FCW, etc... Conversely, the enclave being > + * run **must** preserve the untrusted ``%rbp``. > + * > + * ``callback`` has the following signature: > + * int callback(long rdi, long rsi, long rdx, > + * struct sgx_enclave_exinfo *ex_info, long r8, long r9, > + * void *tcs, long ursp); > + * ``callback`` **shall** follow x86_64 ABI. All GPRs **except** ``%rax``, ``%rbx`` > + * and ``rcx`` are passed through to ``callback``. ``%rdi``, ``%rsi``, ``%rdx``, > + * ``%r8``, ``%r9``, along with the value of ``%rsp`` when the enclave exited/excepted, > + * can be accessed directly as input parameters, while other GPRs can be > + * accessed in assembly if needed. > + * A positive value returned from ``callback`` will be treated as an ENCLU leaf > + * (e.g. EENTER/ERESUME) to reenter the enclave, while 0 or a negative return > + * value will be passed back to the caller of __vdso_sgx_enter_enclave(). > + * It is also **safe** to ``longjmp()`` out of ``callback``. > */ > -__vdso_sgx_enter_enclave(u32 leaf, void *tcs, > - struct sgx_enclave_exception *ex_info) > -{ > - if (leaf != SGX_EENTER && leaf != SGX_ERESUME) > - return -EINVAL; > - > - if (!tcs) > - return -EINVAL; > - > - try { > - ENCLU[leaf]; > - } catch (exception) { > - if (e) > - *e = exception; > - return -EFAULT; > - } > - > - return 0; > -} > -#endif