Linux-Sgx Archive on lore.kernel.org
 help / color / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: Nathaniel McCallum <npmccallum@redhat.com>
Cc: Andy Lutomirski <luto@kernel.org>,
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	X86 ML <x86@kernel.org>,
	linux-sgx@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
	Sean Christopherson <sean.j.christopherson@intel.com>,
	Jethro Beekman <jethro@fortanix.com>,
	Cedric Xing <cedric.xing@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	asapek@google.com, Borislav Petkov <bp@alien8.de>,
	chenalexchen@google.com, Conrad Parker <conradparker@google.com>,
	cyhanish@google.com, Dave Hansen <dave.hansen@intel.com>,
	"Huang, Haitao" <haitao.huang@intel.com>,
	Josh Triplett <josh@joshtriplett.org>,
	"Huang, Kai" <kai.huang@intel.com>,
	"Svahn, Kai" <kai.svahn@intel.com>, Keith Moyer <kmoy@google.com>,
	Christian Ludloff <ludloff@google.com>,
	Neil Horman <nhorman@redhat.com>,
	Patrick Uiterwijk <puiterwijk@redhat.com>,
	David Rientjes <rientjes@google.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	yaozhangx@google.com
Subject: Re: [PATCH v36 21/24] x86/vdso: Implement a vDSO for Intel SGX enclave call
Date: Mon, 17 Aug 2020 08:01:04 -0700
Message-ID: <2125C4DB-FB1C-4049-8020-99ED0AB2DEE3@amacapital.net> (raw)
In-Reply-To: <CAOASepOFh-vOrNZEVDFrDSuHs+9GEzzpXUTG-fZMuyjWAkpRWw@mail.gmail.com>


> On Aug 17, 2020, at 6:12 AM, Nathaniel McCallum <npmccallum@redhat.com> wrote:
> 
> On Mon, Aug 10, 2020 at 7:09 PM Andy Lutomirski <luto@kernel.org> wrote:
>> 
>>> On Thu, Aug 6, 2020 at 7:55 AM Nathaniel McCallum <npmccallum@redhat.com> wrote:
>>> 
>>> In a past revision of this patch, I had requested a void *misc
>>> parameter that could be passed through vdso_sgx_enter_enclave_t into
>>> sgx_enclave_exit_handler_t. This request encountered some push back
>>> and I dropped the issue. However, I'd like to revisit it or something
>>> similar.
>> 
>> Why do you need an exit handler at all?  IIRC way back when I
>> suggested that we simply not support it at all.  If you want to
>> call__vdso_sgx_enter_enclave() in a loop, call it in a loop.  If you
>> want to wrap it intelligently in Rust, you don't want a callback
>> anyway -- that forces you have an FFI (or non-Rust, anyway) frame on
>> the stack, which interacts poorly with panic handling and prevents you
>> from using await in your Rust callback handler.  If, on the other
>> hand, you just call __vdso_sg_enter_enclave() in a loop, all these
>> problems go away and, if you really want, you can pass in a callback
>> in Rust and call the callback from Rust.
>> 
>> What am I missing?  I still don't really understand why we are
>> supporting this mechanism at all.  Just the asm code to invoke the
>> callback seems to be about half of the entire function.
> 
> There are three ways to pass state between the enclave and the outside world:
> 1. A pre-allocated memory block at enclave creation time.
> 2. A contract for pushing values onto the stack during entry/exit.
> 3. All registers and flags besides rax, rbx, and rcx.
> 
> Under the current vDSO function:
> 
> #1 is completely possible without a handler. The challenge is how to
> communicate the address of this memory to the enclave. This can be
> accomplished by a parameter in a measured block or by convention.
> Otherwise, it needs to use #2 or #3 to communicate the address of the
> block.
> 
> #2 requires a handler written in assembly. The semantics are well known.

No one seems particularly interested in my suggestion that the RSP exposed to the enclave be different from the actual untrusted stack. Oh well.

That being said, if I were writing a Rust wrapper for SGX (or Python or any language with a reasonable form of async/await), I would want the vDSO code to support swapping RSP before ENCLU because I would want to have first-class support for invoking an enclave from an async function and suspending inside the handler. Allocating a real stack (and shadow stack once CET shows up) for this is disgusting.  If the vDSO supported this natively, it could (in principle) interact with signal delivery such that signals would not see the alternative stack.

I do admit that the implementation would not be pretty. Honestly, I think Intel messed up by exposing USER_RSP to enclaves in the first place.

> 
> #3 is possible without a handler, but only for the subset of the
> registers allowed by the calling convention. However, with a handler
> written in assembly you can pass both in and out the full range of
> registers. To do this, the assembly handler needs a pointer to a
> buffer to save the registers into. How does it get said pointer?
> Currently: offsetof, which Rust doesn't support.

I find this justification a bit silly. Binutils asm (gas) doesn’t support offsetof for C structs either, and Linux works just fine. We’re talking about hardcoding one number along with an assertion somewhere that the number is correct, right?  Couldn’t sizeof be used, too?

To be clear, I think that passing around a misc pointer seems entirely reasonable, but I see it as a nice feature, not as a requirement for correct usage of the function.

  reply index

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-16 13:52 [PATCH v36 00/24] Intel SGX foundations Jarkko Sakkinen
2020-07-16 13:52 ` [PATCH v36 01/24] x86/cpufeatures: x86/msr: Add Intel SGX hardware bits Jarkko Sakkinen
2020-08-06 13:13   ` Darren Kenny
2020-07-16 13:52 ` [PATCH v36 02/24] x86/cpufeatures: x86/msr: Add Intel SGX Launch Control " Jarkko Sakkinen
2020-08-06 13:14   ` Darren Kenny
2020-07-16 13:52 ` [PATCH v36 03/24] x86/mm: x86/sgx: Signal SIGSEGV with PF_SGX Jarkko Sakkinen
2020-08-06 13:16   ` Darren Kenny
2020-08-20 15:31   ` Borislav Petkov
2020-08-21 17:35     ` Jarkko Sakkinen
2020-07-16 13:52 ` [PATCH v36 04/24] x86/sgx: Add SGX microarchitectural data structures Jarkko Sakkinen
2020-08-06 13:14   ` Darren Kenny
2020-07-16 13:52 ` [PATCH v36 05/24] x86/sgx: Add wrappers for ENCLS leaf functions Jarkko Sakkinen
2020-08-07  9:37   ` Darren Kenny
2020-07-16 13:52 ` [PATCH v36 06/24] x86/cpu/intel: Detect SGX support Jarkko Sakkinen
2020-08-06 13:17   ` Darren Kenny
2020-07-16 13:52 ` [PATCH v36 07/24] x86/cpu/intel: Add nosgx kernel parameter Jarkko Sakkinen
2020-08-06 13:18   ` Darren Kenny
2020-07-16 13:52 ` [PATCH v36 08/24] x86/sgx: Initialize metadata for Enclave Page Cache (EPC) sections Jarkko Sakkinen
2020-08-06 13:27   ` Darren Kenny
2020-07-16 13:52 ` [PATCH v36 09/24] x86/sgx: Add __sgx_alloc_epc_page() and sgx_free_epc_page() Jarkko Sakkinen
2020-08-06 13:29   ` Darren Kenny
2020-07-16 13:52 ` [PATCH v36 10/24] mm: Add vm_ops->mprotect() Jarkko Sakkinen
2020-08-06 13:35   ` Darren Kenny
2020-07-16 13:52 ` [PATCH v36 11/24] x86/sgx: Add SGX enclave driver Jarkko Sakkinen
2020-08-06 13:59   ` Darren Kenny
2020-08-25 16:44   ` Borislav Petkov
2020-08-26 13:46     ` Jarkko Sakkinen
2020-07-16 13:52 ` [PATCH v36 12/24] x86/sgx: Add SGX_IOC_ENCLAVE_CREATE Jarkko Sakkinen
2020-08-06 15:40   ` Darren Kenny
2020-08-26 14:52   ` Borislav Petkov
2020-08-27 13:24     ` Jarkko Sakkinen
2020-08-27 16:15       ` Borislav Petkov
2020-08-28 23:39         ` Jarkko Sakkinen
2020-08-29  0:21       ` Jarkko Sakkinen
2020-09-01 16:41   ` Haitao Huang
2020-09-04 11:55     ` Jarkko Sakkinen
2020-07-16 13:52 ` [PATCH v36 13/24] x86/sgx: Add SGX_IOC_ENCLAVE_ADD_PAGES Jarkko Sakkinen
2020-08-06 16:29   ` Darren Kenny
2020-07-16 13:52 ` [PATCH v36 14/24] x86/sgx: Add SGX_IOC_ENCLAVE_INIT Jarkko Sakkinen
2020-08-06 16:40   ` Darren Kenny
2020-07-16 13:52 ` [PATCH v36 15/24] x86/sgx: Allow a limited use of ATTRIBUTE.PROVISIONKEY for attestation Jarkko Sakkinen
2020-08-06 17:00   ` Darren Kenny
2020-08-18 13:30     ` Jarkko Sakkinen
2020-07-16 13:52 ` [PATCH v36 16/24] x86/sgx: Add a page reclaimer Jarkko Sakkinen
2020-07-16 13:52 ` [PATCH v36 17/24] x86/sgx: ptrace() support for the SGX driver Jarkko Sakkinen
2020-07-16 13:52 ` [PATCH v36 18/24] x86/vdso: Add support for exception fixup in vDSO functions Jarkko Sakkinen
2020-07-16 13:52 ` [PATCH v36 19/24] x86/fault: Add helper function to sanitize error code Jarkko Sakkinen
2020-07-16 13:52 ` [PATCH v36 20/24] x86/traps: Attempt to fixup exceptions in vDSO before signaling Jarkko Sakkinen
2020-07-16 13:53 ` [PATCH v36 21/24] x86/vdso: Implement a vDSO for Intel SGX enclave call Jarkko Sakkinen
2020-08-06 14:55   ` Nathaniel McCallum
2020-08-10 22:23     ` Sean Christopherson
2020-08-11  7:16       ` Jethro Beekman
2020-08-11 14:54         ` Sean Christopherson
2020-08-18 14:52       ` Jarkko Sakkinen
2020-08-18 15:06         ` Jarkko Sakkinen
2020-08-18 15:15           ` Nathaniel McCallum
2020-08-18 16:43             ` Jarkko Sakkinen
2020-08-19 13:33               ` Nathaniel McCallum
2020-08-19 14:00                 ` Jethro Beekman
2020-08-19 21:23                 ` Jarkko Sakkinen
2020-08-10 23:08     ` Andy Lutomirski
2020-08-10 23:48       ` Sean Christopherson
2020-08-11  0:52         ` Andy Lutomirski
2020-08-11 15:16           ` Andy Lutomirski
2020-08-13 19:38             ` Sean Christopherson
2020-08-17 13:12       ` Nathaniel McCallum
2020-08-17 15:01         ` Andy Lutomirski [this message]
2020-08-18 15:15       ` Jarkko Sakkinen
2020-08-20  0:19         ` Andy Lutomirski
2020-08-18 14:26     ` Jarkko Sakkinen
2020-07-16 13:53 ` [PATCH v36 22/24] selftests/x86: Add a selftest for SGX Jarkko Sakkinen
2020-08-27  4:47   ` Nathaniel McCallum
2020-08-27 15:20     ` Sean Christopherson
2020-08-28 23:27       ` Jarkko Sakkinen
2020-07-16 13:53 ` [PATCH v36 23/24] docs: x86/sgx: Document SGX micro architecture and kernel internals Jarkko Sakkinen
2020-07-28 21:35   ` Pavel Machek
2020-08-06 10:21     ` Dr. Greg
2020-08-08 22:18       ` Pavel Machek
2020-08-19 20:55     ` Jarkko Sakkinen
2020-07-16 13:53 ` [PATCH v36 24/24] 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=2125C4DB-FB1C-4049-8020-99ED0AB2DEE3@amacapital.net \
    --to=luto@amacapital.net \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=asapek@google.com \
    --cc=bp@alien8.de \
    --cc=cedric.xing@intel.com \
    --cc=chenalexchen@google.com \
    --cc=conradparker@google.com \
    --cc=cyhanish@google.com \
    --cc=dave.hansen@intel.com \
    --cc=haitao.huang@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=kmoy@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sgx@vger.kernel.org \
    --cc=ludloff@google.com \
    --cc=luto@kernel.org \
    --cc=nhorman@redhat.com \
    --cc=npmccallum@redhat.com \
    --cc=puiterwijk@redhat.com \
    --cc=rientjes@google.com \
    --cc=sean.j.christopherson@intel.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=yaozhangx@google.com \
    /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

Linux-Sgx Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-sgx/0 linux-sgx/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-sgx linux-sgx/ https://lore.kernel.org/linux-sgx \
		linux-sgx@vger.kernel.org
	public-inbox-index linux-sgx

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-sgx


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git