Linux-Sgx Archive on lore.kernel.org
 help / color / Atom feed
From: Jethro Beekman <jethro@fortanix.com>
To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	"linux-sgx@vger.kernel.org" <linux-sgx@vger.kernel.org>
Cc: "sean.j.christopherson@intel.com"
	<sean.j.christopherson@intel.com>,
	"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 16:37:25 +0000
Message-ID: <a657081e-cdaf-7c9d-2695-23c9c07bcfbf@fortanix.com> (raw)
In-Reply-To: <20191010113745.GA12842@linux.intel.com>

[-- Attachment #1: Type: text/plain, Size: 3239 bytes --]

Hi all,

I wrote a preliminary patch integrating the latest kernel patches with the Rust EDP, see https://github.com/fortanix/rust-sgx/pull/181 . I tested against (I think) v23-rc2 with the 3 patch sets from Sean applied. In particular, I started from https://github.com/jsakkine-intel/linux-sgx 8915aef. It would be nice if you'd use git tags so everyone can be on the same page.

I haven't done a whole lot of testing, so I can't speak to the stability of the driver. But I do have some comments regarding the API.



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?

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?

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?



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?

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

--
Jethro Beekman | Fortanix

On 2019-10-10 13:37, Jarkko Sakkinen wrote:
> tag v23-rc2
> Tagger: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Date:   Thu Oct 10 14:33:07 2019 +0300
> 
> x86/sgx: v23-rc1 patch set
> 
> * Return -EIO instead of -ECANCELED when ptrace() fails to read a TCS page.
> * In the reclaimer, pin page before ENCLS[EBLOCK] because pinning can fail
>   (because of OOM) even in legit behaviour and after EBLOCK the reclaiming
>   flow can be only reverted by killing the whole enclave.
> * Fixed SGX_ATTR_RESERVED_MASK. Bit 7 was marked as reserved while in fact
>   it should have been bit 6 (Table 37-3 in the SDM).
> * Return -EPERM from SGX_IOC_ENCLAVE_INIT when ENCLS[EINIT] returns an SGX
>   error code.
> * In v22 __uaccess_begin() was used to pin the source page in
>   __sgx_encl_add_page(). Switch to get_user_pages() in order to avoid
>   deadlock (mmap_sem might get locked twice in the same thread).
> -----BEGIN PGP SIGNATURE-----
> 
> iJYEABYIAD4WIQRE6pSOnaBC00OEHEIaerohdGur0gUCXZ8XTyAcamFya2tvLnNh
> a2tpbmVuQGxpbnV4LmludGVsLmNvbQAKCRAaerohdGur0phXAP9QPYcpyUTSO9hk
> sG/pV7vsIjS4lO6pxBCgWCtg3/ZkvAEApLCu7mFvyZs3rDcbOlQA+nQiVv+rUwzu
> tsYmW2YsgQ4=
> =VeL3
> -----END PGP SIGNATURE-----
> 
> /Jarkko
> 


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4054 bytes --]

  parent 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 [this message]
2019-10-11 18:15   ` Sean Christopherson
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=a657081e-cdaf-7c9d-2695-23c9c07bcfbf@fortanix.com \
    --to=jethro@fortanix.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=linux-sgx@vger.kernel.org \
    --cc=sean.j.christopherson@intel.com \
    --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