From: Stephen Smalley <sds@tycho.nsa.gov> To: Sean Christopherson <sean.j.christopherson@intel.com>, Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Cc: Andy Lutomirski <luto@kernel.org>, Cedric Xing <cedric.xing@intel.com>, 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, Jethro Beekman <jethro@fortanix.com>, Dave Hansen <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, Andrew Morton <akpm@linux-foundation.org>, nhorman@redhat.com, npmccallum@redhat.com, Serge Ayoun <serge.ayoun@intel.com>, Shay Katz-zamir <shay.katz-zamir@intel.com>, Haitao Huang <haitao.huang@intel.com>, Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Kai Svahn <kai.svahn@intel.com>, Borislav Petkov <bp@alien8.de>, Josh Triplett <josh@joshtriplett.org>, Kai Huang <kai.huang@intel.com>, David Rientjes <rientjes@google.com>, William Roberts <william.c.roberts@intel.com>, Philip Tricca <philip.b.tricca@intel.com> Subject: Re: [RFC PATCH v2 4/5] LSM: x86/sgx: Introduce ->enclave_load() hook for Intel SGX Date: Fri, 7 Jun 2019 15:58:34 -0400 Message-ID: <5706a7ec-5497-c560-92fa-91c9751b9096@tycho.nsa.gov> (raw) In-Reply-To: <20190606021145.12604-5-sean.j.christopherson@intel.com> On 6/5/19 10:11 PM, Sean Christopherson wrote: > enclave_load() is roughly analogous to the existing file_mprotect(). > > Due to the nature of SGX and its Enclave Page Cache (EPC), all enclave > VMAs are backed by a single file, i.e. /dev/sgx/enclave, that must be > MAP_SHARED. Furthermore, all enclaves need read, write and execute > VMAs. As a result, the existing/standard call to file_mprotect() does > not provide any meaningful security for enclaves since an LSM can only > deny/grant access to the EPC as a whole. > > security_enclave_load() is called when SGX is first loading an enclave > page, i.e. copying a page from normal memory into the EPC. Although > the prototype for enclave_load() is similar to file_mprotect(), e.g. > SGX could theoretically use file_mprotect() and set reqprot=prot, a > separate hook is desirable as the semantics of an enclave's protection > bits are different than those of vmas, e.g. an enclave page tracks the > maximal set of protections, whereas file_mprotect() operates on the > actual protections being provided. In other words, LSMs will likely > want to implement different policies for enclave page protections. > > Note, extensive discussion yielded no sane alternative to some form of > SGX specific LSM hook[1]. > > [1] https://lkml.kernel.org/r/CALCETrXf8mSK45h7sTK5Wf+pXLVn=Bjsc_RLpgO-h-qdzBRo5Q@mail.gmail.com > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > arch/x86/kernel/cpu/sgx/driver/ioctl.c | 12 ++++++------ > include/linux/lsm_hooks.h | 13 +++++++++++++ > include/linux/security.h | 12 ++++++++++++ > security/security.c | 7 +++++++ > 4 files changed, 38 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c b/arch/x86/kernel/cpu/sgx/driver/ioctl.c > index 44b2d73de7c3..29c0df672250 100644 > --- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c > +++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c > @@ -8,6 +8,7 @@ > #include <linux/highmem.h> > #include <linux/ratelimit.h> > #include <linux/sched/signal.h> > +#include <linux/security.h> > #include <linux/shmem_fs.h> > #include <linux/slab.h> > #include <linux/suspend.h> > @@ -582,9 +583,6 @@ static int sgx_encl_page_copy(void *dst, unsigned long src, unsigned long prot) > struct vm_area_struct *vma; > int ret; > > - if (!(prot & VM_EXEC)) > - return 0; > - Is there a real use case where LSM will want to be called if !(prot & VM_EXEC)? Also, you seem to be mixing prot and PROT_EXEC with vm_flags and VM_EXEC; other code does not appear to assume they are identical and explicitly converts, e.g. calc_vm_prot_bits(). > /* Hold mmap_sem across copy_from_user() to avoid a TOCTOU race. */ > down_read(¤t->mm->mmap_sem); > > @@ -599,15 +597,17 @@ static int sgx_encl_page_copy(void *dst, unsigned long src, unsigned long prot) > * but with some future proofing against other cases that may deny > * execute permissions. > */ > - if (!(vma->vm_flags & VM_MAYEXEC)) { > + if ((prot & VM_EXEC) && !(vma->vm_flags & VM_MAYEXEC)) { > ret = -EACCES; > goto out; > } > > + ret = security_enclave_load(vma, prot); > + if (ret) > + goto out; > + > if (copy_from_user(dst, (void __user *)src, PAGE_SIZE)) > ret = -EFAULT; > - else > - ret = 0; > > out: > up_read(¤t->mm->mmap_sem); > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index 47f58cfb6a19..c6f47a7eef70 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -1446,6 +1446,12 @@ > * @bpf_prog_free_security: > * Clean up the security information stored inside bpf prog. > * > + * Security hooks for Intel SGX enclaves. > + * > + * @enclave_load: > + * @vma: the source memory region of the enclave page being loaded. > + * @prot: the (maximal) protections of the enclave page. > + * Return 0 if permission is granted. > */ > union security_list_options { > int (*binder_set_context_mgr)(struct task_struct *mgr); > @@ -1807,6 +1813,10 @@ union security_list_options { > int (*bpf_prog_alloc_security)(struct bpf_prog_aux *aux); > void (*bpf_prog_free_security)(struct bpf_prog_aux *aux); > #endif /* CONFIG_BPF_SYSCALL */ > + > +#ifdef CONFIG_INTEL_SGX > + int (*enclave_load)(struct vm_area_struct *vma, unsigned long prot); > +#endif /* CONFIG_INTEL_SGX */ > }; > > struct security_hook_heads { > @@ -2046,6 +2056,9 @@ struct security_hook_heads { > struct hlist_head bpf_prog_alloc_security; > struct hlist_head bpf_prog_free_security; > #endif /* CONFIG_BPF_SYSCALL */ > +#ifdef CONFIG_INTEL_SGX > + struct hlist_head enclave_load; > +#endif /* CONFIG_INTEL_SGX */ > } __randomize_layout; > > /* > diff --git a/include/linux/security.h b/include/linux/security.h > index 659071c2e57c..0b6d1eb7368b 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -1829,5 +1829,17 @@ static inline void security_bpf_prog_free(struct bpf_prog_aux *aux) > #endif /* CONFIG_SECURITY */ > #endif /* CONFIG_BPF_SYSCALL */ > > +#ifdef CONFIG_INTEL_SGX > +#ifdef CONFIG_SECURITY > +int security_enclave_load(struct vm_area_struct *vma, unsigned long prot); > +#else > +static inline int security_enclave_load(struct vm_area_struct *vma, > + unsigned long prot) > +{ > + return 0; > +} > +#endif /* CONFIG_SECURITY */ > +#endif /* CONFIG_INTEL_SGX */ > + > #endif /* ! __LINUX_SECURITY_H */ > > diff --git a/security/security.c b/security/security.c > index 613a5c00e602..c6f7f26969b2 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -2359,3 +2359,10 @@ void security_bpf_prog_free(struct bpf_prog_aux *aux) > call_void_hook(bpf_prog_free_security, aux); > } > #endif /* CONFIG_BPF_SYSCALL */ > + > +#ifdef CONFIG_INTEL_SGX > +int security_enclave_load(struct vm_area_struct *vma, unsigned long prot) > +{ > + return call_int_hook(enclave_load, 0, vma, prot); > +} > +#endif /* CONFIG_INTEL_SGX */ >
next prev parent 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 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 [this message] 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 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=5706a7ec-5497-c560-92fa-91c9751b9096@tycho.nsa.gov \ --to=sds@tycho.nsa.gov \ --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=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
SELinux Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/selinux/0 selinux/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 selinux selinux/ https://lore.kernel.org/selinux \ selinux@vger.kernel.org public-inbox-index selinux Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.selinux AGPL code for this site: git clone https://public-inbox.org/public-inbox.git