All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Xing, Cedric" <cedric.xing@intel.com>
To: "Christopherson, Sean J" <sean.j.christopherson@intel.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"x86@kernel.org" <x86@kernel.org>,
	"linux-sgx@vger.kernel.org" <linux-sgx@vger.kernel.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"Hansen, Dave" <dave.hansen@intel.com>,
	"nhorman@redhat.com" <nhorman@redhat.com>,
	"npmccallum@redhat.com" <npmccallum@redhat.com>,
	"Ayoun, Serge" <serge.ayoun@intel.com>,
	"Katz-zamir, Shay" <shay.katz-zamir@intel.com>,
	"Huang, Haitao" <haitao.huang@intel.com>,
	"andriy.shevchenko@linux.intel.com" 
	<andriy.shevchenko@linux.intel.com>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"Svahn, Kai" <kai.svahn@intel.com>, "bp@alien8.de" <bp@alien8.de>,
	"josh@joshtriplett.org" <josh@joshtriplett.org>,
	"luto@kernel.org" <luto@kernel.org>,
	"Huang, Kai" <kai.huang@intel.com>,
	"rientjes@google.com" <rientjes@google.com>,
	Andy Lutomirski <luto@amacapital.net>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"Haitao Huang" <haitao.huang@linux.intel.com>,
	Jethro Beekman <jethro@fortanix.com>,
	"Dr . Greg Wettstein" <greg@enjellic.com>
Subject: RE: [PATCH v19,RESEND 24/27] x86/vdso: Add __vdso_sgx_enter_enclave() to wrap SGX enclave transitions
Date: Wed, 20 Mar 2019 20:38:04 +0000	[thread overview]
Message-ID: <960B34DE67B9E140824F1DCDEC400C0F4E85C5AB@ORSMSX116.amr.corp.intel.com> (raw)
In-Reply-To: <20190320191318.GF30469@linux.intel.com>

> > By requiring preservation of RSP at both AEX and EEXIT, this precludes
> > the possibility of using the untrusted stack as temporary storage by
> > enclaves. While that looks reasonable at first glance, I'm afraid it
> > isn't the case in reality. The untrusted stack is inarguably the most
> > convenient way for data exchange between an enclave and its enclosing
> > process,
> 
> I vehemently disagree with "inarguably".  IMO, passing data via
> registers is much more convenient.

Which is the most convenient approach is always dependent on data size and/or even how the data is produced/consumed. It's kind of a spectrum and we're just talking in the sense of probability. You are right that "inarguably" is arguable if the buffer is small enough to fit in registers, and the producer/consumer also has access to registers.

> 
> Even if you qualify your assertion with "data of arbitrary size unknown
> at build time", I still disagree.  Using the untrusted stack allows for
> trickery when a debugger is involved, other than that I see no
> advantages over allocating virtual memory and handing the pointer to the
> enclave at launch time.  Sure, it requires a few more lines of code to
> setup, but it's literally ~20 LoC out of thousands required to sign,
> build and launch an enclave, but it doesn't require playing games with
> the stack.

I'm NOT ruling out your approach.

And like you said, the untrusted stack enables certain trickery that helps debugging and also simplifies enclaves (even just a little). Then why are you trying to rule that out? Because of 9 LOC in vDSO?

> 
> Not to mention that the entire concept of using the untrusted stack is
> based on the assumption that the enclave is making ocalls, e.g.
> stateless enclaves or libraries that use a message queue have zero
> need/benefit for using the untrusted stack.

Don't get me wrong. I never said enclaves would require untrusted stack to make ocalls, or ocalls would require untrusted stack to make. It's just a generic approach for sharing/exchanging data. Some enclaves my need it, others may not.

My question still remains: why do you want to rule it out?

> > and is in fact being used for that purpose by almost all existing
> > enclaves to date.
> 
> That's a bit misleading, since almost all existing enclaves are built
> against Intel's SDK, which just so happens to unconditionally use the
> untrusted stack.  It's not like all enclave developers made a concious
> decision to use the untrusted stack.  If Intel rewrote the SDK to use a
> different method then one could argue that the new approach is the most
> common method of passing data.

Everything exists for a reason. It's unimportant what has been done. What matters is why that was done in that particular way. I was trying to inspire thinking.

> 
> > Given the expectation that this API will be used by all future SGX
> > application, it looks unwise to ban the most convenient and commonly
> > used approach for data exchange.
> >
> > Given an enclave can touch everything (registers and memory) of the
> > enclosing process, it's reasonable to restrict the enclave by means of
> > "calling convention" to allow the enclosing process to retain its
> > context. And for that purpose, SGX ISA does offer 2 registers (i.e.
> > RSP and RBP) for applications to choose. Instead of preserving RSP,
> > I'd prefer RBP, which will end up with more flexibility in all SGX
> > applications in future.
> 
> I disagree that the SGX ISA intends for applications to choose between
> preserving RSP and RBP, e.g. the SDM description of SSA.UR{B,S}P states:
> 
>   Non-Enclave (outside) {RBP,stack} pointer. Saved by EENTER, restored
> on AEX.
> 
> To me, the "Saved/restored" wording implies that URBP and URSP should
> *never* be touched by the enclave.  Sure, the proposed vDSO interface
> doesn't require RBP to be preserved, but only because the goal was to
> deviate from hardware as little as possible, not because anyone wants to
> encourage enclaves to muck with RBP.

I'm so sorry to tell you that you have misunderstood the SDM. If this is a common misunderstanding, I guess I could talk to the architect responsible for this SDM chapter to see if we could amend the language.

The purpose of restoring RSP is because software needs a stack to handle exception. Well, that's not 100% accurate because it's a user mode stack. Anyway, it tells the used part from the unused space in the stack. RBP on the other hand is NEVER required from interrupt/exception handling perspective, but we decided to add it because we'd like to offer a choice, just like I said earlier. The calling thread could then anchor its frame on either RSP or RBP.  

  reply	other threads:[~2019-03-20 20:38 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-20 16:20 [PATCH v19,RESEND 00/27] Intel SGX1 support Jarkko Sakkinen
2019-03-20 16:20 ` [PATCH v19,RESEND 01/27] x86/cpufeatures: Add Intel-defined SGX feature bit Jarkko Sakkinen
2019-03-20 19:41   ` Neil Horman
2019-03-21 14:16     ` Jarkko Sakkinen
2019-03-20 16:20 ` [PATCH v19,RESEND 02/27] x86/cpufeatures: Add SGX sub-features (as Linux-defined bits) Jarkko Sakkinen
2019-03-20 16:20 ` [PATCH v19,RESEND 03/27] x86/msr: Add IA32_FEATURE_CONTROL.SGX_ENABLE definition Jarkko Sakkinen
2019-03-20 16:20 ` [PATCH v19,RESEND 04/27] x86/cpufeatures: Add Intel-defined SGX_LC feature bit Jarkko Sakkinen
2019-03-20 16:20 ` [PATCH v19,RESEND 05/27] x86/msr: Add SGX Launch Control MSR definitions Jarkko Sakkinen
2019-03-20 16:20 ` [PATCH v19,RESEND 06/27] x86/mm: x86/sgx: Add new 'PF_SGX' page fault error code bit Jarkko Sakkinen
2019-03-20 16:20 ` [PATCH v19,RESEND 07/27] x86/mm: x86/sgx: Signal SIGSEGV for userspace #PFs w/ PF_SGX Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 08/27] x86/cpu/intel: Detect SGX support and update caps appropriately Jarkko Sakkinen
2019-03-26 12:17   ` Huang, Kai
2019-03-26 14:27     ` Sean Christopherson
2019-03-26 21:25       ` Huang, Kai
2019-03-26 21:57         ` Sean Christopherson
2019-03-26 23:19           ` Huang, Kai
2019-03-20 16:21 ` [PATCH v19,RESEND 09/27] x86/sgx: Add ENCLS architectural error codes Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 10/27] x86/sgx: Add SGX1 and SGX2 architectural data structures Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 11/27] x86/sgx: Add definitions for SGX's CPUID leaf and variable sub-leafs Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 12/27] x86/sgx: Enumerate and track EPC sections Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 13/27] x86/sgx: Add wrappers for ENCLS leaf functions Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 14/27] x86/sgx: Add functions to allocate and free EPC pages Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 15/27] x86/sgx: Add sgx_einit() for initializing enclaves Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 16/27] x86/sgx: Add the Linux SGX Enclave Driver Jarkko Sakkinen
2019-03-26 12:01   ` Huang, Kai
2019-03-26 12:40     ` Thomas Gleixner
2019-03-26 14:54       ` Sean Christopherson
2019-03-26 21:11         ` Huang, Kai
2019-03-27  5:02     ` Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 17/27] x86/sgx: Add provisioning Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 18/27] x86/sgx: Add swapping code to the core and SGX driver Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 19/27] x86/sgx: ptrace() support for the " Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 20/27] x86/vdso: Add support for exception fixup in vDSO functions Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 21/27] x86/fault: Add helper function to sanitize error code Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 22/27] x86/fault: Attempt to fixup unhandled #PF in vDSO before signaling Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 23/27] x86/traps: Attempt to fixup exceptions " Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 24/27] x86/vdso: Add __vdso_sgx_enter_enclave() to wrap SGX enclave transitions Jarkko Sakkinen
2019-03-20 18:30   ` Xing, Cedric
2019-03-20 18:52     ` Andy Lutomirski
2019-03-20 19:57       ` Xing, Cedric
2019-03-20 21:03         ` Sean Christopherson
2019-03-21  0:17           ` Xing, Cedric
2019-03-22 21:20             ` Sean Christopherson
2019-03-21 17:17         ` Andy Lutomirski
2019-03-22 20:31           ` Xing, Cedric
2019-03-20 19:02     ` Jethro Beekman
2019-03-20 20:10       ` Xing, Cedric
2019-03-20 19:13     ` Sean Christopherson
2019-03-20 20:38       ` Xing, Cedric [this message]
2019-03-22 21:59         ` Sean Christopherson
2019-03-23 17:36           ` Xing, Cedric
2019-03-23 21:38             ` Andy Lutomirski
2019-03-24  8:59               ` Xing, Cedric
2019-03-25 18:03                 ` Sean Christopherson
2019-03-25 23:59                   ` Andy Lutomirski
2019-03-26  4:53                     ` Xing, Cedric
2019-03-26 17:08                       ` Andy Lutomirski
2019-03-28  4:23                         ` Xing, Cedric
2019-03-28 19:18                           ` Andy Lutomirski
2019-03-28 23:19                             ` Xing, Cedric
2019-03-29  9:48                               ` Jarkko Sakkinen
2019-03-31  8:43                                 ` Dr. Greg
2019-04-03 23:03                             ` Sean Christopherson
2019-03-25 23:54                 ` Andy Lutomirski
2019-03-26  4:16                   ` Xing, Cedric
2019-03-20 16:21 ` [PATCH v19,RESEND 25/27] x86/sgx: SGX documentation Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 26/27] selftests/x86: Add a selftest for SGX Jarkko Sakkinen
2019-03-20 16:21 ` [PATCH v19,RESEND 27/27] x86/sgx: Update MAINTAINERS Jarkko Sakkinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=960B34DE67B9E140824F1DCDEC400C0F4E85C5AB@ORSMSX116.amr.corp.intel.com \
    --to=cedric.xing@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=greg@enjellic.com \
    --cc=haitao.huang@intel.com \
    --cc=haitao.huang@linux.intel.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jethro@fortanix.com \
    --cc=josh@joshtriplett.org \
    --cc=kai.huang@intel.com \
    --cc=kai.svahn@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sgx@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=luto@kernel.org \
    --cc=nhorman@redhat.com \
    --cc=npmccallum@redhat.com \
    --cc=rientjes@google.com \
    --cc=sean.j.christopherson@intel.com \
    --cc=serge.ayoun@intel.com \
    --cc=shay.katz-zamir@intel.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.