| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5706a7ec-5497-c560-92fa-91c9751b9096@tycho.nsa.gov>
Date: Fri, 7 Jun 2019 15:58:34 -0400
From: Stephen Smalley <sds@...ho.nsa.gov>
To: Sean Christopherson <sean.j.christopherson@...el.com>,
Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
Cc: Andy Lutomirski <luto@...nel.org>,
Cedric Xing <cedric.xing@...el.com>,
James Morris <jmorris@...ei.org>,
"Serge E . Hallyn" <serge@...lyn.com>,
LSM List <linux-security-module@...r.kernel.org>,
Paul Moore <paul@...l-moore.com>,
Eric Paris <eparis@...isplace.org>, selinux@...r.kernel.org,
Jethro Beekman <jethro@...tanix.com>,
Dave Hansen <dave.hansen@...el.com>,
Thomas Gleixner <tglx@...utronix.de>,
Linus Torvalds <torvalds@...ux-foundation.org>,
LKML <linux-kernel@...r.kernel.org>, X86 ML <x86@...nel.org>,
linux-sgx@...r.kernel.org,
Andrew Morton <akpm@...ux-foundation.org>, nhorman@...hat.com,
npmccallum@...hat.com, Serge Ayoun <serge.ayoun@...el.com>,
Shay Katz-zamir <shay.katz-zamir@...el.com>,
Haitao Huang <haitao.huang@...el.com>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Kai Svahn <kai.svahn@...el.com>,
Borislav Petkov <bp@...en8.de>,
Josh Triplett <josh@...htriplett.org>,
Kai Huang <kai.huang@...el.com>,
David Rientjes <rientjes@...gle.com>,
William Roberts <william.c.roberts@...el.com>,
Philip Tricca <philip.b.tricca@...el.com>
Subject: Re: [RFC PATCH v2 4/5] LSM: x86/sgx: Introduce ->enclave_load() hook
for Intel SGX
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@...el.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 */
>
Powered by blists - more mailing lists