Linux-Sgx Archive on lore.kernel.org
 help / color / Atom feed
From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Jethro Beekman <jethro@fortanix.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	linux-kernel@vger.kernel.org, x86@kernel.org,
	linux-sgx@vger.kernel.org, akpm@linux-foundation.org,
	dave.hansen@intel.com, nhorman@redhat.com, npmccallum@redhat.com,
	haitao.huang@intel.com, andriy.shevchenko@linux.intel.com,
	tglx@linutronix.de, kai.svahn@intel.com, bp@alien8.de,
	josh@joshtriplett.org, luto@kernel.org, kai.huang@intel.com,
	rientjes@google.com, cedric.xing@intel.com,
	puiterwijk@redhat.com, linux-security-module@vger.kernel.org,
	Suresh Siddha <suresh.b.siddha@intel.com>,
	Haitao Huang <haitao.huang@linux.intel.com>
Subject: Re: [PATCH v26 10/22] x86/sgx: Linux Enclave Driver
Date: Thu, 13 Feb 2020 10:07:37 -0800
Message-ID: <20200213180737.GC18610@linux.intel.com> (raw)
In-Reply-To: <d17c50a7-6900-731b-43a2-d6e49b8eb44d@fortanix.com>

On Thu, Feb 13, 2020 at 02:59:52PM +0100, Jethro Beekman wrote:
> On 2020-02-09 22:25, Jarkko Sakkinen wrote:
> > +/**
> > + * struct sgx_enclave_add_pages - parameter structure for the
> > + *                                %SGX_IOC_ENCLAVE_ADD_PAGE ioctl
> > + * @src:	start address for the page data
> > + * @offset:	starting page offset
> > + * @length:	length of the data (multiple of the page size)
> > + * @secinfo:	address for the SECINFO data
> > + * @flags:	page control flags
> > + * @count:	number of bytes added (multiple of the page size)
> > + */
> > +struct sgx_enclave_add_pages {
> > +	__u64	src;
> > +	__u64	offset;
> > +	__u64	length;
> > +	__u64	secinfo;
> > +	__u64	flags;
> > +	__u64	count;
> > +};
> 
> Compared to the last time I looked at the patch set, this API removes the
> ability to measure individual pages chunks. That is not acceptable.

Why is it not acceptable?  E.g. what specific use case do you have that
_requires_ on measuring partial 4k pages of an enclave?

> On 2019-10-11 16:37, Sean Christopherson wrote:
> > Hiding the 256-byte granualarity from userspace is a good idea as it's not
> > intrinsically tied to the SGX architecture and exists only because of
> > latency requirements.
> 
> What do you mean by "it's not intrinsically tied to the SGX architecture"?
> This is a fundamental part of the SGX instruction set. This is the
> instruction definition from the SDM: "EEXTEND—Extend Uninitialized Enclave
> Measurement by 256 Bytes".

SGX fundamentally works at a 4k granularity.  EEXTEND is special cased
because extending the measurement is a slow operation, i.e. EEXTEND on more
than 256 byte chunks, *with the current implementation*, would exceeded
latency requirements, e.g. block interrupts for too long and hose the
kernel.

A future implementation of SGX could change the latency of extending the
measurement, e.g. a different algorithm that is slower/faster, and so could
introduce EEXTEND2 which would work at a different granularity than EEXTEND.

EEXTEND could have avoided the latency problems via other methods, e.g. by
being interruptible a la EINIT and/or by being restartable.  But that ship
has sailed, so to avoid future complication in the kernel's ABI we're
proposing/advocating supporting only measuring at a 4k granularity.
 
> The exact sequence of EADD/EEXTEND calls is part of the enclave hash. The OS
> mustn't arbitrarily restrict how an enclave may be loaded. If the enclave

It's not arbitrary, there are good reasons for wanting to work with 4k
granularity.  Regardless, there are many examples of the kernel arbitrarily
restricting what can be done relative to what is physically possible in
hardware.

> loader were to follows OS-specific restrictions, that would result in
> effectively different enclaves. Because of these interoperability concerns,
> 256-byte granularity *must* be exposed through the UAPI.

Interoperability with what?  Other OSes? 

> Besides only partially measuring a page, there are some other fringe cases
> that are technically possible, although I haven't seen any toolchains that do
> that. These include not interleaving EADD and EEXTEND, not using logical
> ordering for the EEXTENDs, and call EEXTEND multiple times on the same chunk.
> Maximum interoperability would require supporting any EADD/EEXTEND sequence.

Same interoperability question as above.

> Maybe we should just add an EEXTEND@offset ioctl? This would give
> fine-grained control when needed (one could set flags=0 in the add pages
> ioctl and interleave with EEXTEND as needed). If you're ok adding an EEXTEND
> ioctl I don't think this issue needs to block landing the driver in its
> current form, in which case:

We've also discussed an EEXTEND ioctl(), but ultimately couldn't come up
with a use case that _required_ partial page measurement.

> Tested-by: Jethro Beekman <jethro@fortanix.com>
> 
> Sorry for being super late with this, I know you asked me for feedback about
> this specific point in October. However, I did previously mention several
> times that being able to measure individual 256-byte chunks is necessary.
> 
> -- Jethro Beekman | Fortanix

  reply index

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-09 21:25 [PATCH v26 00/22] Intel SGX foundations Jarkko Sakkinen
2020-02-09 21:25 ` [PATCH v26 01/22] x86/cpufeatures: x86/msr: Add Intel SGX hardware bits Jarkko Sakkinen
2020-02-09 21:25 ` [PATCH v26 02/22] x86/cpufeatures: x86/msr: Intel SGX Launch Control " Jarkko Sakkinen
2020-02-09 21:25 ` [PATCH v26 03/22] x86/mm: x86/sgx: Signal SIGSEGV with PF_SGX Jarkko Sakkinen
2020-02-09 21:25 ` [PATCH v26 04/22] x86/sgx: Add SGX microarchitectural data structures Jarkko Sakkinen
2020-02-09 21:25 ` [PATCH v26 05/22] x86/sgx: Add wrappers for ENCLS leaf functions Jarkko Sakkinen
2020-02-09 21:25 ` [PATCH v26 06/22] x86/cpu/intel: Detect SGX supprt Jarkko Sakkinen
2020-02-12 16:57   ` Sean Christopherson
2020-02-13 18:04     ` Jarkko Sakkinen
2020-02-09 21:25 ` [PATCH v26 07/22] x86/sgx: Enumerate and track EPC sections Jarkko Sakkinen
2020-02-09 21:25 ` [PATCH v26 08/22] x86/sgx: Add functions to allocate and free EPC pages Jarkko Sakkinen
2020-02-09 21:25 ` [PATCH v26 09/22] mm: Introduce vm_ops->may_mprotect() Jarkko Sakkinen
2020-02-09 21:25 ` [PATCH v26 10/22] x86/sgx: Linux Enclave Driver Jarkko Sakkinen
2020-02-13 13:59   ` Jethro Beekman
2020-02-13 18:07     ` Sean Christopherson [this message]
2020-02-14  9:24       ` Jethro Beekman
2020-02-14 17:11         ` Sean Christopherson
2020-02-14 17:40           ` Andy Lutomirski
2020-02-14 17:52             ` Sean Christopherson
2020-02-15 16:56               ` Andy Lutomirski
2020-02-18 22:12                 ` Sean Christopherson
2020-02-15 18:05           ` Dr. Greg
2020-02-15  7:32     ` Jarkko Sakkinen
2020-02-15  7:35       ` Jarkko Sakkinen
2020-02-19  3:26   ` Jordan Hand
2020-02-20 18:13     ` Sean Christopherson
2020-02-20 18:33       ` Jordan Hand
2020-02-20 18:48         ` Sean Christopherson
2020-02-20 22:16           ` Jarkko Sakkinen
2020-02-21  0:11             ` Jordan Hand
2020-02-21 12:53               ` Jarkko Sakkinen
2020-02-21  0:32             ` Andy Lutomirski
2020-02-21 13:01               ` Jarkko Sakkinen
2020-02-20 18:51       ` Andy Lutomirski
2020-02-20 19:15         ` Sean Christopherson
2020-02-20 22:10     ` Jarkko Sakkinen
2020-02-09 21:25 ` [PATCH v26 11/22] selftests/x86: Recurse into subdirectories Jarkko Sakkinen
2020-02-09 21:25 ` [PATCH v26 12/22] selftests/x86: Add a selftest for SGX Jarkko Sakkinen
2020-02-09 21:26 ` [PATCH v26 13/22] x86/sgx: Add provisioning Jarkko Sakkinen
2020-02-13 10:49   ` Jethro Beekman
2020-02-15  7:25     ` Jarkko Sakkinen
2020-02-09 21:26 ` [PATCH v26 14/22] x86/sgx: Add a page reclaimer Jarkko Sakkinen
2020-02-09 21:26 ` [PATCH v26 15/22] x86/sgx: ptrace() support for the SGX driver Jarkko Sakkinen
2020-02-09 21:26 ` [PATCH v26 16/22] x86/vdso: Add support for exception fixup in vDSO functions Jarkko Sakkinen
2020-02-09 21:26 ` [PATCH v26 17/22] x86/fault: Add helper function to sanitize error code Jarkko Sakkinen
2020-02-09 21:26 ` [PATCH v26 18/22] x86/traps: Attempt to fixup exceptions in vDSO before signaling Jarkko Sakkinen
2020-02-09 21:26 ` [PATCH v26 19/22] x86/vdso: Add __vdso_sgx_enter_enclave() to wrap SGX enclave transitions Jarkko Sakkinen
2020-02-13 13:29   ` Jethro Beekman
2020-02-15  7:26     ` Jarkko Sakkinen
2020-02-09 21:26 ` [PATCH v26 20/22] selftests/x86: Add vDSO selftest for SGX Jarkko Sakkinen
2020-02-09 21:26 ` [PATCH v26 21/22] x86/sgx: Update MAINTAINERS Jarkko Sakkinen
2020-02-09 21:26 ` [PATCH v26 22/22] docs: x86/sgx: Document SGX micro architecture and kernel internals Jarkko Sakkinen
2020-02-22  4:13   ` Randy Dunlap
2020-02-23 17:04     ` Jarkko Sakkinen
2020-02-23 17:05       ` Jarkko Sakkinen

Reply instructions:

You may reply publically 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=20200213180737.GC18610@linux.intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=cedric.xing@intel.com \
    --cc=dave.hansen@intel.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-security-module@vger.kernel.org \
    --cc=linux-sgx@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=nhorman@redhat.com \
    --cc=npmccallum@redhat.com \
    --cc=puiterwijk@redhat.com \
    --cc=rientjes@google.com \
    --cc=suresh.b.siddha@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

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