Re: [PATCH v36 11/24] x86/sgx: Add SGX enclave driver
From: Borislav Petkov <bp@alien8.de>
Date: 2020-08-25 16:44:28
Also in:
linux-mm, lkml
On Thu, Jul 16, 2020 at 04:52:50PM +0300, Jarkko Sakkinen wrote: Just minor things below - I'm not even going to pretend I fully understand what's going on but FWICT, it looks non-threateningly ok to me.
quoted hunk ↗ jump to hunk
diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c new file mode 100644 index 000000000000..b52520407f5b --- /dev/null +++ b/arch/x86/kernel/cpu/sgx/driver.c@@ -0,0 +1,177 @@ +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// Copyright(c) 2016-18 Intel Corporation. + +#include <linux/acpi.h> +#include <linux/miscdevice.h> +#include <linux/mman.h> +#include <linux/security.h> +#include <linux/suspend.h> +#include <asm/traps.h> +#include "driver.h" +#include "encl.h" + +MODULE_DESCRIPTION("Intel SGX Enclave Driver"); +MODULE_AUTHOR("Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>"); +MODULE_LICENSE("Dual BSD/GPL");
That boilerplate stuff usually goes to the end of the file. ...
+static struct sgx_encl_page *sgx_encl_load_page(struct sgx_encl *encl,
+ unsigned long addr)
+{
+ struct sgx_encl_page *entry;
+ unsigned int flags;
+
+ /* If process was forked, VMA is still there but vm_private_data is set
+ * to NULL.
+ */
+ if (!encl)
+ return ERR_PTR(-EFAULT);
+
+ flags = atomic_read(&encl->flags);
+^ Superfluous newline.
+ if ((flags & SGX_ENCL_DEAD) || !(flags & SGX_ENCL_INITIALIZED))
+ return ERR_PTR(-EFAULT);
+
+ entry = xa_load(&encl->page_array, PFN_DOWN(addr));
+ if (!entry)
+ return ERR_PTR(-EFAULT);
+
+ /* Page is already resident in the EPC. */
+ if (entry->epc_page)
+ return entry;
+
+ return ERR_PTR(-EFAULT);
+}
+
+static void sgx_mmu_notifier_release(struct mmu_notifier *mn,
+ struct mm_struct *mm)
+{
+ struct sgx_encl_mm *encl_mm =
+ container_of(mn, struct sgx_encl_mm, mmu_notifier);Just let it stick out.
+ struct sgx_encl_mm *tmp = NULL;
+
+ /*
+ * The enclave itself can remove encl_mm. Note, objects can't be moved
+ * off an RCU protected list, but deletion is ok.
+ */
+ spin_lock(&encl_mm->encl->mm_lock);
+ list_for_each_entry(tmp, &encl_mm->encl->mm_list, list) {
+ if (tmp == encl_mm) {
+ list_del_rcu(&encl_mm->list);
+ break;
+ }
+ }
+ spin_unlock(&encl_mm->encl->mm_lock);
+
+ if (tmp == encl_mm) {
+ synchronize_srcu(&encl_mm->encl->srcu);
+ mmu_notifier_put(mn);
+ }
+}
+
+static void sgx_mmu_notifier_free(struct mmu_notifier *mn)
+{
+ struct sgx_encl_mm *encl_mm =
+ container_of(mn, struct sgx_encl_mm, mmu_notifier);Ditto. ...
+/**
+ * sgx_encl_may_map() - Check if a requested VMA mapping is allowed
+ * @encl: an enclave
+ * @start: lower bound of the address range, inclusive
+ * @end: upper bound of the address range, exclusive
+ * @vm_prot_bits: requested protections of the address range
+ *
+ * Iterate through the enclave pages contained within [@start, @end) to verify
+ * the permissions requested by @vm_prot_bits do not exceed that of any enclave
+ * page to be mapped.
+ *
+ * Return:
+ * 0 on success,
+ * -EACCES if VMA permissions exceed enclave page permissions
+ */
+int sgx_encl_may_map(struct sgx_encl *encl, unsigned long start,
+ unsigned long end, unsigned long vm_flags)
+{
+ unsigned long vm_prot_bits = vm_flags & (VM_READ | VM_WRITE | VM_EXEC);
+ unsigned long idx_start = PFN_DOWN(start);
+ unsigned long idx_end = PFN_DOWN(end - 1);
+ struct sgx_encl_page *page;
+ XA_STATE(xas, &encl->page_array, idx_start);
+
+ /*
+ * Disallow RIE tasks as their VMA permissions might conflict with the"RIE", hmm what is that? /me looks at the test Aaah, READ_IMPLIES_EXEC. Is "RIE" some widely accepted acronym I'm not aware of?
+ * enclave page permissions. + */ + if (!!(current->personality & READ_IMPLIES_EXEC))
The "!!" is not really needed - you're in boolean context.
...
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette