Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: "Xing, Cedric" <cedric.xing@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>,
	"Christopherson, Sean J" <sean.j.christopherson@intel.com>,
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	Stephen Smalley <sds@tycho.nsa.gov>,
	James Morris <jmorris@namei.org>,
	"Serge E . Hallyn" <serge@hallyn.com>,
	LSM List <linux-security-module@vger.kernel.org>,
	Paul Moore <paul@paul-moore.com>,
	Eric Paris <eparis@parisplace.org>,
	"selinux@vger.kernel.org" <selinux@vger.kernel.org>,
	Jethro Beekman <jethro@fortanix.com>,
	"Hansen, Dave" <dave.hansen@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>, X86 ML <x86@kernel.org>,
	"linux-sgx@vger.kernel.org" <linux-sgx@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	"nhorman@redhat.com" <nhorman@redhat.com>,
	"npmccallum@redhat.com" <npmccallum@redhat.com>,
	"Ayoun, Serge" <serge.ayoun@intel.com>,
	"Katz-zamir, Shay" <shay.katz-zamir@intel.com>,
	"Huang, Haitao" <haitao.huang@intel.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	"Svahn, Kai" <kai.svahn@intel.com>,
	Borislav Petkov <bp@alien8.de>,
	Josh Triplett <josh@joshtriplett.org>,
	"Huang, Kai" <kai.huang@intel.com>,
	David Rientjes <rientjes@google.com>,
	"Roberts, William C" <william.c.roberts@intel.com>,
	"Tricca, Philip B" <philip.b.tricca@intel.com>
Subject: Re: [RFC PATCH v2 2/5] x86/sgx: Require userspace to define enclave pages' protection bits
Date: Tue, 11 Jun 2019 17:09:28 -0700
Message-ID: <331B31BF-9892-4FB3-9265-3E37412F80F4@amacapital.net> (raw)
In-Reply-To: <960B34DE67B9E140824F1DCDEC400C0F655010EF@ORSMSX116.amr.corp.intel.com>



On Jun 10, 2019, at 3:28 PM, Xing, Cedric <cedric.xing@intel.com> wrote:

>> From: Andy Lutomirski [mailto:luto@kernel.org]
>> Sent: Monday, June 10, 2019 12:15 PM
>> 
>> On Mon, Jun 10, 2019 at 11:29 AM Xing, Cedric <cedric.xing@intel.com>
>> wrote:
>>> 
>>>> From: Christopherson, Sean J
>>>> Sent: Wednesday, June 05, 2019 7:12 PM
>>>> 
>>>> +/**
>>>> + * sgx_map_allowed - check vma protections against the associated
>>>> enclave page
>>>> + * @encl:    an enclave
>>>> + * @start:   start address of the mapping (inclusive)
>>>> + * @end:     end address of the mapping (exclusive)
>>>> + * @prot:    protection bits of the mapping
>>>> + *
>>>> + * Verify a userspace mapping to an enclave page would not violate
>>>> +the security
>>>> + * requirements of the *kernel*.  Note, this is in no way related
>>>> +to the
>>>> + * page protections enforced by hardware via the EPCM.  The EPCM
>>>> +protections
>>>> + * can be directly extended by the enclave, i.e. cannot be relied
>>>> +upon by the
>>>> + * kernel for security guarantees of any kind.
>>>> + *
>>>> + * Return:
>>>> + *   0 on success,
>>>> + *   -EACCES if the mapping is disallowed
>>>> + */
>>>> +int sgx_map_allowed(struct sgx_encl *encl, unsigned long start,
>>>> +                 unsigned long end, unsigned long prot) {
>>>> +     struct sgx_encl_page *page;
>>>> +     unsigned long addr;
>>>> +
>>>> +     prot &= (VM_READ | VM_WRITE | VM_EXEC);
>>>> +     if (!prot || !encl)
>>>> +             return 0;
>>>> +
>>>> +     mutex_lock(&encl->lock);
>>>> +
>>>> +     for (addr = start; addr < end; addr += PAGE_SIZE) {
>>>> +             page = radix_tree_lookup(&encl->page_tree, addr >>
>>>> PAGE_SHIFT);
>>>> +
>>>> +             /*
>>>> +              * Do not allow R|W|X to a non-existent page, or
>> protections
>>>> +              * beyond those of the existing enclave page.
>>>> +              */
>>>> +             if (!page || (prot & ~page->prot))
>>>> +                     return -EACCES;
>>> 
>>> In SGX2, pages will be "mapped" before being populated.
>>> 
>>> Here's a brief summary for those who don't have enough background on
>> how new EPC pages could be added to a running enclave in SGX2:
>>>  - There are 2 new instructions - EACCEPT and EAUG.
>>>  - EAUG is used by SGX module to add (augment) a new page to an
>> existing enclave. The newly added page is *inaccessible* until the
>> enclave *accepts* it.
>>>  - EACCEPT is the instruction for an enclave to accept a new page.
>>> 
>>> And the s/w flow for an enclave to request new EPC pages is expected
>> to be something like the following:
>>>  - The enclave issues EACCEPT at the linear address that it would
>> like a new page.
>>>  - EACCEPT results in #PF, as there's no page at the linear address
>> above.
>>>  - SGX module is notified about the #PF, in form of its vma->vm_ops-
>>> fault() being called by kernel.
>>>  - SGX module EAUGs a new EPC page at the fault address, and resumes
>> the enclave.
>>>  - EACCEPT is reattempted, and succeeds at this time.
>> 
>> This seems like an odd workflow.  Shouldn't the #PF return back to
>> untrusted userspace so that the untrusted user code can make its own
>> decision as to whether it wants to EAUG a page there as opposed to, say,
>> killing the enclave or waiting to keep resource usage under control?
> 
> This may seem odd to some at the first glance. But if you can think of how static heap (pre-allocated by EADD before EINIT) works, the load parses the "metadata" coming with the enclave to decide the address/size of the heap, EADDs it, and calls it done. In the case of "dynamic" heap (allocated dynamically by EAUG after EINIT), the same thing applies - the loader determines the range of the heap, tells the SGX module about it, and calls it done. Everything else is the between the enclave and the SGX module.
> 
> In practice, untrusted code usually doesn't know much about enclaves, just like it doesn't know much about the shared objects loaded into its address space either. Without the necessary knowledge, untrusted code usually just does what it is told (via o-calls, or return value from e-calls), without judging that's right or wrong. 
> 
> When it comes to #PF like what I described, of course a signal could be sent to the untrusted code but what would it do then? Usually it'd just come back asking for a page at the fault address. So we figured it'd be more efficient to just have the kernel EAUG at #PF. 
> 
> Please don't get me wrong though, as I'm not dictating what the s/w flow shall be. It's just going to be a choice offered to user mode. And that choice was planned to be offered via mprotect() - i.e. a writable vma causes kernel to EAUG while a non-writable vma will result in a signal (then the user mode could decide whether to EAUG). The key point is flexibility - as we want to allow all reasonable s/w flows instead of dictating one over others. We had similar discussions on vDSO API before. And I think you accepted my approach because of its flexibility. Am I right?

As long as user code can turn this off, I have no real objection. But it might make sense to have it be more explicit — have an ioctl set up a range as “EAUG-on-demand”.

But this is all currently irrelevant. We can argue about it when the patches show up. :)

  reply index

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-06  2:11 [RFC PATCH v2 0/5] security: x86/sgx: SGX vs. LSM Sean Christopherson
2019-06-06  2:11 ` [RFC PATCH v2 1/5] mm: Introduce vm_ops->may_mprotect() Sean Christopherson
2019-06-10 15:06   ` Jarkko Sakkinen
2019-06-10 15:55     ` Sean Christopherson
2019-06-10 17:47       ` Xing, Cedric
2019-06-10 19:49         ` Sean Christopherson
2019-06-10 22:06           ` Xing, Cedric
2019-06-06  2:11 ` [RFC PATCH v2 2/5] x86/sgx: Require userspace to define enclave pages' protection bits Sean Christopherson
2019-06-10 15:27   ` Jarkko Sakkinen
2019-06-10 16:15     ` Sean Christopherson
2019-06-10 17:45       ` Jarkko Sakkinen
2019-06-10 18:17         ` Sean Christopherson
2019-06-12 19:26           ` Jarkko Sakkinen
2019-06-10 18:29   ` Xing, Cedric
2019-06-10 19:15     ` Andy Lutomirski
2019-06-10 22:28       ` Xing, Cedric
2019-06-12  0:09         ` Andy Lutomirski [this message]
2019-06-12 14:34           ` Sean Christopherson
2019-06-12 18:20             ` Xing, Cedric
2019-06-06  2:11 ` [RFC PATCH v2 3/5] x86/sgx: Enforce noexec filesystem restriction for enclaves Sean Christopherson
2019-06-10 16:00   ` Jarkko Sakkinen
2019-06-10 16:44     ` Andy Lutomirski
2019-06-11 17:21       ` Stephen Smalley
2019-06-06  2:11 ` [RFC PATCH v2 4/5] LSM: x86/sgx: Introduce ->enclave_load() hook for Intel SGX Sean Christopherson
2019-06-07 19:58   ` Stephen Smalley
2019-06-10 16:21     ` Sean Christopherson
2019-06-10 16:05   ` Jarkko Sakkinen
2019-06-06  2:11 ` [RFC PATCH v2 5/5] security/selinux: Add enclave_load() implementation Sean Christopherson
2019-06-07 21:16   ` Stephen Smalley
2019-06-10 16:46     ` Sean Christopherson
2019-06-17 16:38   ` Jarkko Sakkinen
2019-06-10  7:03 ` [RFC PATCH v1 0/3] security/x86/sgx: SGX specific LSM hooks Cedric Xing
2019-06-10  7:03   ` [RFC PATCH v1 1/3] LSM/x86/sgx: Add " Cedric Xing
2019-06-10  7:03   ` [RFC PATCH v1 2/3] LSM/x86/sgx: Implement SGX specific hooks in SELinux Cedric Xing
2019-06-11 13:40     ` Stephen Smalley
2019-06-11 22:02       ` Sean Christopherson
2019-06-12  9:32         ` Dr. Greg
2019-06-12 14:25           ` Sean Christopherson
2019-06-13  7:25             ` Dr. Greg
2019-06-12 19:30         ` Andy Lutomirski
2019-06-12 22:02           ` Sean Christopherson
2019-06-13  0:10             ` Xing, Cedric
2019-06-13  1:02             ` Xing, Cedric
2019-06-13 17:02         ` Stephen Smalley
2019-06-13 23:03           ` Xing, Cedric
2019-06-13 23:17             ` Sean Christopherson
2019-06-14  0:31               ` Xing, Cedric
2019-06-14  0:46           ` Sean Christopherson
2019-06-14 15:38             ` Sean Christopherson
2019-06-16 22:14               ` Andy Lutomirski
2019-06-17 16:49                 ` Sean Christopherson
2019-06-17 17:08                   ` Andy Lutomirski
2019-06-18 15:40                   ` Dr. Greg
2019-06-14 17:16             ` Xing, Cedric
2019-06-14 17:45               ` Sean Christopherson
2019-06-14 17:53                 ` Sean Christopherson
2019-06-14 20:01                   ` Sean Christopherson
2019-06-16 22:16               ` Andy Lutomirski
2019-06-14 23:19             ` Dr. Greg
2019-06-11 22:55       ` Xing, Cedric
2019-06-13 18:00         ` Stephen Smalley
2019-06-13 19:48           ` Sean Christopherson
2019-06-13 21:09             ` Xing, Cedric
2019-06-13 21:02           ` Xing, Cedric
2019-06-14  0:37           ` Sean Christopherson
2019-06-10  7:03   ` [RFC PATCH v1 3/3] LSM/x86/sgx: Call new LSM hooks from SGX subsystem Cedric Xing
2019-06-10 17:36   ` [RFC PATCH v1 0/3] security/x86/sgx: SGX specific LSM hooks 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=331B31BF-9892-4FB3-9265-3E37412F80F4@amacapital.net \
    --to=luto@amacapital.net \
    --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=eparis@parisplace.org \
    --cc=haitao.huang@intel.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jethro@fortanix.com \
    --cc=jmorris@namei.org \
    --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=paul@paul-moore.com \
    --cc=philip.b.tricca@intel.com \
    --cc=rientjes@google.com \
    --cc=sds@tycho.nsa.gov \
    --cc=sean.j.christopherson@intel.com \
    --cc=selinux@vger.kernel.org \
    --cc=serge.ayoun@intel.com \
    --cc=serge@hallyn.com \
    --cc=shay.katz-zamir@intel.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=william.c.roberts@intel.com \
    --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-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/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-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org linux-security-module@archiver.kernel.org
	public-inbox-index linux-security-module


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


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