LKML Archive on lore.kernel.org
 help / color / Atom feed
From: "Ayoun, Serge" <serge.ayoun@intel.com>
To: 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>
Cc: "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"Hansen, Dave" <dave.hansen@intel.com>,
	"Christopherson, Sean J" <sean.j.christopherson@intel.com>,
	"nhorman@redhat.com" <nhorman@redhat.com>,
	"npmccallum@redhat.com" <npmccallum@redhat.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>,
	"Xing, Cedric" <cedric.xing@intel.com>
Subject: RE: [PATCH v21 16/28] x86/sgx: Add the Linux SGX Enclave Driver
Date: Mon, 29 Jul 2019 11:17:57 +0000
Message-ID: <88B7642769729B409B4A93D7C5E0C5E7C65ABB8D@hasmsx108.ger.corp.intel.com> (raw)
In-Reply-To: <20190713170804.2340-17-jarkko.sakkinen@linux.intel.com>

> From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Sent: Saturday, July 13, 2019 20:08
> Subject: [PATCH v21 16/28] x86/sgx: Add the Linux SGX Enclave Driver

> +static long sgx_ioc_enclave_add_page(struct file *filep, void __user
> +*arg) {
> +	struct sgx_encl *encl = filep->private_data;
> +	struct sgx_enclave_add_page addp;
> +	struct sgx_secinfo secinfo;
> +	struct page *data_page;
> +	unsigned long prot;
> +	void *data;
> +	int ret;
> +
> +	if (copy_from_user(&addp, arg, sizeof(addp)))
> +		return -EFAULT;
> +
> +	if (copy_from_user(&secinfo, (void __user *)addp.secinfo,
> +			   sizeof(secinfo)))
> +		return -EFAULT;
> +
> +	data_page = alloc_page(GFP_HIGHUSER);
> +	if (!data_page)
> +		return -ENOMEM;
> +
> +	data = kmap(data_page);
> +
> +	prot = _calc_vm_trans(secinfo.flags, SGX_SECINFO_R, PROT_READ)
> |
> +	       _calc_vm_trans(secinfo.flags, SGX_SECINFO_W, PROT_WRITE) |
> +	       _calc_vm_trans(secinfo.flags, SGX_SECINFO_X, PROT_EXEC);
> +
> +	/* TCS pages need to be RW in the PTEs, but can be 0 in the EPCM. */
> +	if ((secinfo.flags & SGX_SECINFO_PAGE_TYPE_MASK) ==
> SGX_SECINFO_TCS)
> +		prot |= PROT_READ | PROT_WRITE;

For TCS pages you add both RD and WR maximum protection bits.
For the enclave to be able to run, user mode will have to change the "vma->vm_flags" from PROT_NONE to PROT_READ | PROT_WRITE (otherwise eenter fails). 
This is exactly what your selftest  does.
But when mmap (or mprotect) is called with PROT_READ bit, it automatically adds the PROT_EXEC bit unless the host application has been compiled with '-z noexecstack' option; pasting below the mmap() code which does it:

	if ((prot & PROT_READ) && (current->personality & READ_IMPLIES_EXEC))
		if (!(file && path_noexec(&file->f_path)))
			prot |= PROT_EXEC;

The problem is that if PROT_EXEC bit is added then sgx_mmap callback will fail since PROT_EXEC will get blocked by your code and not allowed for TCS pages.
This restriction is not necessary at all, i.e. I wouldn't block PROT_EXEC on tcs area because anyway, the hardware will never let those areas to execute: the SGX protection flags are fixed by the cpu and can not be changed by any mean.
So in order to facilitate user's interface I would let prot |= PROT_READ | PROT_WRITE | PROT_EXEC; we do not give up to any security criteria and make user interaction easier.

> +
> +	ret = sgx_encl_page_import_user(data, addp.src, prot);
> +	if (ret)
> +		goto out;
> +
> +	ret = sgx_encl_add_page(encl, addp.addr, data, &secinfo,
> addp.mrmask,
> +				prot);
> +	if (ret)
> +		goto out;
> +
> +out:
> +	kunmap(data_page);
> +	__free_page(data_page);
> +	return ret;
> +}
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


  reply index

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-13 17:07 [PATCH v21 00/28] Intel SGX foundations Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 01/28] x86/cpufeatures: Add Intel-defined SGX feature bit Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 02/28] x86/cpufeatures: Add SGX sub-features (as Linux-defined bits) Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 03/28] x86/msr: Add IA32_FEATURE_CONTROL.SGX_ENABLE definition Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 04/28] x86/cpufeatures: Add Intel-defined SGX_LC feature bit Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 05/28] x86/msr: Add SGX Launch Control MSR definitions Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 06/28] x86/mm: x86/sgx: Add new 'PF_SGX' page fault error code bit Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 07/28] x86/mm: x86/sgx: Signal SIGSEGV for userspace #PFs w/ PF_SGX Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 08/28] x86/cpu/intel: Detect SGX support and update caps appropriately Jarkko Sakkinen
2019-07-24 19:35   ` Sean Christopherson
2019-08-02 20:48     ` Jarkko Sakkinen
2019-08-07 15:17     ` Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 09/28] x86/sgx: Add ENCLS architectural error codes Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 10/28] x86/sgx: Add SGX1 and SGX2 architectural data structures Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 11/28] x86/sgx: Add wrappers for ENCLS leaf functions Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 12/28] x86/sgx: Enumerate and track EPC sections Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 13/28] x86/sgx: Add functions to allocate and free EPC pages Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 14/28] x86/sgx: Add sgx_einit() for initializing enclaves Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 15/28] mm: Introduce vm_ops->may_mprotect() Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 16/28] x86/sgx: Add the Linux SGX Enclave Driver Jarkko Sakkinen
2019-07-29 11:17   ` Ayoun, Serge [this message]
2019-08-07 15:15     ` Jarkko Sakkinen
2019-08-07 15:17       ` Jarkko Sakkinen
2019-08-07 16:45         ` Jethro Beekman
2019-08-08 15:40       ` Sean Christopherson
2019-08-09 15:02         ` Jarkko Sakkinen
2019-08-09 15:24           ` Sean Christopherson
2019-08-05 16:16   ` Sean Christopherson
2019-08-05 21:39     ` Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 17/28] x86/sgx: Add provisioning Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 18/28] x86/sgx: Add swapping code to the core and SGX driver Jarkko Sakkinen
2019-08-07  6:33   ` Jethro Beekman
2019-08-07 19:12     ` Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 19/28] x86/sgx: ptrace() support for the " Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 20/28] x86/vdso: Add support for exception fixup in vDSO functions Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 21/28] x86/fault: Add helper function to sanitize error code Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 22/28] x86/traps: Attempt to fixup exceptions in vDSO before signaling Jarkko Sakkinen
2019-07-13 17:07 ` [PATCH v21 23/28] x86/vdso: Add __vdso_sgx_enter_enclave() to wrap SGX enclave transitions Jarkko Sakkinen
2019-07-17 22:07   ` Xing, Cedric
2019-07-13 17:08 ` [PATCH v21 24/28] selftests/x86: Add a selftest for SGX Jarkko Sakkinen
2019-07-17 22:37   ` Xing, Cedric
2019-08-02 20:46     ` Jarkko Sakkinen
2019-08-16 15:43     ` Jarkko Sakkinen
2019-08-16 15:51       ` Jarkko Sakkinen
2019-08-16 16:56     ` Jarkko Sakkinen
2019-07-13 17:08 ` [PATCH v21 25/28] x86/sgx: Update MAINTAINERS Jarkko Sakkinen
2019-07-13 17:08 ` [PATCH v21 26/28] docs: x86/sgx: Add Architecture documentation Jarkko Sakkinen
2019-07-13 17:08 ` [PATCH v21 27/28] docs: x86/sgx: Document kernel internals Jarkko Sakkinen
2019-07-13 17:08 ` [PATCH v21 28/28] docs: x86/sgx: Document the enclave API Jarkko Sakkinen
2019-07-14 14:36 ` [PATCH v21 00/28] Intel SGX foundations Jarkko Sakkinen
2019-08-07  6:40   ` Jethro Beekman

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=88B7642769729B409B4A93D7C5E0C5E7C65ABB8D@hasmsx108.ger.corp.intel.com \
    --to=serge.ayoun@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=jarkko.sakkinen@linux.intel.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@kernel.org \
    --cc=nhorman@redhat.com \
    --cc=npmccallum@redhat.com \
    --cc=rientjes@google.com \
    --cc=sean.j.christopherson@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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

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

Example config snippet for mirrors

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


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