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-sgx@vger.kernel.org" <linux-sgx@vger.kernel.org>,
	"serge.ayoun@intel.com" <serge.ayoun@intel.com>,
	"shay.katz-zamir@intel.com" <shay.katz-zamir@intel.com>
Subject: Re: x86/sgx: v23-rc2
Date: Fri, 11 Oct 2019 11:15:50 -0700
Message-ID: <20191011181550.GB30935@linux.intel.com> (raw)
In-Reply-To: <a657081e-cdaf-7c9d-2695-23c9c07bcfbf@fortanix.com>

On Fri, Oct 11, 2019 at 04:37:25PM +0000, Jethro Beekman wrote:
> UAPI:
> 
> This got a whole lot more complex for userspace compared to the out-of-tree
> driver.
> 
> 1. Manually needing to mmap a naturally-aligned memory region by allocating
> too much memory and then unmapping parts is quite annoying. Why was the
> auto-aligning removed? I think this will need to be handled the same for
> every consumer of SGX, so I don't see why this is not handled in the kernel.
> It never seems wrong to align if NULL is passed as the requested address.
> Alternatively, is there room in the flags for a MAP_ALIGNED bit?

I'm pretty sure everyone agrees it's annoying.  The short of it is that
the SGX driver is the wrong place to do the alignment.  The driver could
key off addr=0, but we don't want to take on that implicit behavior.

A MAP_ALIGNED flag to have the allocation be naturally aligned is the
ideal solution.  It's definitely something we should pursue, but that can
and probably should be done in parallel to the SGX series.

> 2. Having to re-open the device for every enclave is also annoying. This
> means you need a filesystem available throughout the process lifetime. I
> tried dup, but that doesn't work. Can we make dup work?

The semantics of dup() won't get you what want, as dup() just creates a
new descriptor pointing at the same file.

An alternative solution that was proposed was to have an ioctl() for
creating an enclave.  But that means using an anonymous inode, which runs
afoul of SELinux permissions, e.g. every _process_ that runs enclaves
would require EXECMEM.  Linus was quite clear that SGX wouldn't be merged
if using it required users to degrade existing security.

I'm open to other ideas.  I wasn't aware this was a pain point and file
stuff isn't exactly my area of expertise, so I haven't put much/any
thought into alternatives.

> 3. Needing to mprotect every page with the precise permissions needed after
> EINIT is really bad. This means I have to remember this data for every page
> between EADD and EINIT. I don't care about SELinux, I trust the ECPM will do
> its job for me. Can we make it so that I can protect the whole range at once,
> or protect the individual pages at EADD time?

You can mprotect() or mmap(..., MAP_FIXED) an enclave range once all
pages covered by the specified range have been added to the enclave, i.e.
at EADD.  I double checked this with the selftest.  Holler if you're
seeing different behavior.


> VDSO:
> 
> It is *difficult* to link to weakly link to a symbol in the VDSO. Anyway, I
> figured it out.
> 
> 1. What if I don't want to automatically ERESUME after kernel interrupt?

Do EENTER/ERESUME directly instead of going through the vDSO.

> 2. I normally do a sanity check after ENCLU[EENTER] that EAX = EEXIT. The
> current implementation just clears EAX instead without looking at it.

Hmm, the only reason I can think of for checking EAX would be to support
userspace mucking with EAX in a #DB/#BP signal handler.  At that point, I
would expect the signal handler to modify RIP as well.  Reaching the XOR
via any other non-EEXIT path would require a kernel bug.

Was there a specific scenario or use case you had in mind?  I'm not
against adding a check, I just don't see what value it would provide.

  reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-10 11:37 Jarkko Sakkinen
2019-10-10 13:37 ` Jarkko Sakkinen
2019-10-10 17:09   ` Sean Christopherson
2019-10-10 17:39     ` Sean Christopherson
2019-10-11 16:37 ` Jethro Beekman
2019-10-11 18:15   ` Sean Christopherson [this message]
2019-10-14  8:43     ` Jethro Beekman
2019-10-17 17:57       ` Sean Christopherson

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=20191011181550.GB30935@linux.intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jethro@fortanix.com \
    --cc=linux-sgx@vger.kernel.org \
    --cc=serge.ayoun@intel.com \
    --cc=shay.katz-zamir@intel.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 linux-sgx@archiver.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