lkml.org 
[lkml]   [2022]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH V2 06/32] x86/sgx: Support VMA permissions more relaxed than enclave permissions
    On Mon, Feb 07, 2022 at 04:45:28PM -0800, Reinette Chatre wrote:
    > === Summary ===
    >
    > An SGX VMA can only be created if its permissions are the same or
    > weaker than the Enclave Page Cache Map (EPCM) permissions. After VMA
    > creation this same rule is again enforced by the page fault handler:
    > faulted enclave pages are required to have equal or more relaxed
    > EPCM permissions than the VMA permissions.
    >
    > On SGX1 systems the additional enforcement in the page fault handler
    > is redundant and on SGX2 systems it incorrectly prevents access.
    > On SGX1 systems it is unnecessary to repeat the enforcement of the
    > permission rule. The rule used during original VMA creation will
    > ensure that any access attempt will use correct permissions.
    > With SGX2 the EPCM permissions of a page can change after VMA
    > creation resulting in the VMA permissions potentially being more
    > relaxed than the EPCM permissions and the page fault handler
    > incorrectly blocking valid access attempts.
    >
    > Enable the VMA's pages to remain accessible while ensuring that
    > the PTEs are installed to match the EPCM permissions but not be
    > more relaxed than the VMA permissions.
    >
    > === Full Changelog ===
    >
    > An SGX enclave is an area of memory where parts of an application
    > can reside. First an enclave is created and loaded (from
    > non-enclave memory) with the code and data of an application,
    > then user space can map (mmap()) the enclave memory to
    > be able to enter the enclave at its defined entry points for
    > execution within it.
    >
    > The hardware maintains a secure structure, the Enclave Page Cache Map
    > (EPCM), that tracks the contents of the enclave. Of interest here is
    > its tracking of the enclave page permissions. When a page is loaded
    > into the enclave its permissions are specified and recorded in the
    > EPCM. In parallel the kernel maintains permissions within the
    > page table entries (PTEs) and the rule is that PTE permissions
    > are not allowed to be more relaxed than the EPCM permissions.
    >
    > A new mapping (mmap()) of enclave memory can only succeed if the
    > mapping has the same or weaker permissions than the permissions that
    > were vetted during enclave creation. This is enforced by
    > sgx_encl_may_map() that is called on the mmap() as well as mprotect()
    > paths. This rule remains.
    >
    > One feature of SGX2 is to support the modification of EPCM permissions
    > after enclave initialization. Enclave pages may thus already be part
    > of a VMA at the time their EPCM permissions are changed resulting
    > in the VMA's permissions potentially being more relaxed than the EPCM
    > permissions.
    >
    > Allow permissions of existing VMAs to be more relaxed than EPCM
    > permissions in preparation for dynamic EPCM permission changes
    > made possible in SGX2. New VMAs that attempt to have more relaxed
    > permissions than EPCM permissions continue to be unsupported.
    >
    > Reasons why permissions of existing VMAs are allowed to be more relaxed
    > than EPCM permissions instead of dynamically changing VMA permissions
    > when EPCM permissions change are:
    > 1) Changing VMA permissions involve splitting VMAs which is an
    > operation that can fail. Additionally changing EPCM permissions of
    > a range of pages could also fail on any of the pages involved.
    > Handling these error cases causes problems. For example, if an
    > EPCM permission change fails and the VMA has already been split
    > then it is not possible to undo the VMA split nor possible to
    > undo the EPCM permission changes that did succeed before the
    > failure.
    > 2) The kernel has little insight into the user space where EPCM
    > permissions are controlled from. For example, a RW page may
    > be made RO just before it is made RX and splitting the VMAs
    > while the VMAs may change soon is unnecessary.
    >
    > Remove the extra permission check called on a page fault
    > (vm_operations_struct->fault) or during debugging
    > (vm_operations_struct->access) when loading the enclave page from swap
    > that ensures that the VMA permissions are not more relaxed than the
    > EPCM permissions. Since a VMA could only exist if it passed the
    > original permission checks during mmap() and a VMA may indeed
    > have more relaxed permissions than the EPCM permissions this extra
    > permission check is no longer appropriate.
    >
    > With the permission check removed, ensure that PTEs do
    > not blindly inherit the VMA permissions but instead the permissions
    > that the VMA and EPCM agree on. PTEs for writable pages (from VMA
    > and enclave perspective) are installed with the writable bit set,
    > reducing the need for this additional flow to the permission mismatch
    > cases handled next.
    >
    > Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
    > ---
    > Changes since V1:
    > - Reword commit message (Jarkko).
    > - Use "relax" instead of "exceed" when referring to permissions (Dave).
    > - Add snippet to Documentation/x86/sgx.rst that highlights the
    > relationship between VMA, EPCM, and PTE permissions on SGX
    > systems (Andy).
    >
    > Documentation/x86/sgx.rst | 10 +++++++++
    > arch/x86/kernel/cpu/sgx/encl.c | 38 ++++++++++++++++++----------------
    > 2 files changed, 30 insertions(+), 18 deletions(-)
    >
    > diff --git a/Documentation/x86/sgx.rst b/Documentation/x86/sgx.rst
    > index 89ff924b1480..5659932728a5 100644
    > --- a/Documentation/x86/sgx.rst
    > +++ b/Documentation/x86/sgx.rst
    > @@ -99,6 +99,16 @@ The relationships between the different permission masks are:
    > * PTEs are installed to match the EPCM permissions, but not be more
    > relaxed than the VMA permissions.
    >
    > +On systems supporting SGX2 EPCM permissions may change while the
    > +enclave page belongs to a VMA without impacting the VMA permissions.
    > +This means that a running VMA may appear to allow access to an enclave
    > +page that is not allowed by its EPCM permissions. For example, when an
    > +enclave page with RW EPCM permissions is mapped by a RW VMA but is
    > +subsequently changed to have read-only EPCM permissions. The kernel
    > +continues to maintain correct access to the enclave page through the
    > +PTE that will ensure that only access allowed by both the VMA
    > +and EPCM permissions are permitted.
    > +
    > Application interface
    > =====================
    >
    > diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
    > index 48afe96ae0f0..b6105d9e7c46 100644
    > --- a/arch/x86/kernel/cpu/sgx/encl.c
    > +++ b/arch/x86/kernel/cpu/sgx/encl.c
    > @@ -91,10 +91,8 @@ static struct sgx_epc_page *sgx_encl_eldu(struct sgx_encl_page *encl_page,
    > }
    >
    > static struct sgx_encl_page *sgx_encl_load_page(struct sgx_encl *encl,
    > - unsigned long addr,
    > - unsigned long vm_flags)
    > + unsigned long addr)
    > {
    > - unsigned long vm_prot_bits = vm_flags & (VM_READ | VM_WRITE | VM_EXEC);
    > struct sgx_epc_page *epc_page;
    > struct sgx_encl_page *entry;
    >
    > @@ -102,14 +100,6 @@ static struct sgx_encl_page *sgx_encl_load_page(struct sgx_encl *encl,
    > if (!entry)
    > return ERR_PTR(-EFAULT);
    >
    > - /*
    > - * Verify that the faulted page has equal or higher build time
    > - * permissions than the VMA permissions (i.e. the subset of {VM_READ,
    > - * VM_WRITE, VM_EXECUTE} in vma->vm_flags).
    > - */
    > - if ((entry->vm_max_prot_bits & vm_prot_bits) != vm_prot_bits)
    > - return ERR_PTR(-EFAULT);
    > -
    > /* Entry successfully located. */
    > if (entry->epc_page) {
    > if (entry->desc & SGX_ENCL_PAGE_BEING_RECLAIMED)
    > @@ -138,7 +128,9 @@ static vm_fault_t sgx_vma_fault(struct vm_fault *vmf)
    > {
    > unsigned long addr = (unsigned long)vmf->address;
    > struct vm_area_struct *vma = vmf->vma;
    > + unsigned long page_prot_bits;
    > struct sgx_encl_page *entry;
    > + unsigned long vm_prot_bits;
    > unsigned long phys_addr;
    > struct sgx_encl *encl;
    > vm_fault_t ret;
    > @@ -155,7 +147,7 @@ static vm_fault_t sgx_vma_fault(struct vm_fault *vmf)
    >
    > mutex_lock(&encl->lock);
    >
    > - entry = sgx_encl_load_page(encl, addr, vma->vm_flags);
    > + entry = sgx_encl_load_page(encl, addr);
    > if (IS_ERR(entry)) {
    > mutex_unlock(&encl->lock);

    > @@ -167,7 +159,19 @@ static vm_fault_t sgx_vma_fault(struct vm_fault *vmf)
    >
    > phys_addr = sgx_get_epc_phys_addr(entry->epc_page);
    >
    > - ret = vmf_insert_pfn(vma, addr, PFN_DOWN(phys_addr));
    > + /*
    > + * Insert PTE to match the EPCM page permissions ensured to not
    > + * exceed the VMA permissions.
    > + */
    > + vm_prot_bits = vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC);
    > + page_prot_bits = entry->vm_max_prot_bits & vm_prot_bits;
    > + /*
    > + * Add VM_SHARED so that PTE is made writable right away if VMA
    > + * and EPCM are writable (no COW in SGX).
    > + */
    > + page_prot_bits |= (vma->vm_flags & VM_SHARED);
    > + ret = vmf_insert_pfn_prot(vma, addr, PFN_DOWN(phys_addr),
    > + vm_get_page_prot(page_prot_bits));
    > if (ret != VM_FAULT_NOPAGE) {
    > mutex_unlock(&encl->lock);
    >
    > @@ -295,15 +299,14 @@ static int sgx_encl_debug_write(struct sgx_encl *encl, struct sgx_encl_page *pag
    > * Load an enclave page to EPC if required, and take encl->lock.
    > */
    > static struct sgx_encl_page *sgx_encl_reserve_page(struct sgx_encl *encl,
    > - unsigned long addr,
    > - unsigned long vm_flags)
    > + unsigned long addr)
    > {
    > struct sgx_encl_page *entry;
    >
    > for ( ; ; ) {
    > mutex_lock(&encl->lock);
    >
    > - entry = sgx_encl_load_page(encl, addr, vm_flags);
    > + entry = sgx_encl_load_page(encl, addr);
    > if (PTR_ERR(entry) != -EBUSY)
    > break;
    >
    > @@ -339,8 +342,7 @@ static int sgx_vma_access(struct vm_area_struct *vma, unsigned long addr,
    > return -EFAULT;
    >
    > for (i = 0; i < len; i += cnt) {
    > - entry = sgx_encl_reserve_page(encl, (addr + i) & PAGE_MASK,
    > - vma->vm_flags);
    > + entry = sgx_encl_reserve_page(encl, (addr + i) & PAGE_MASK);
    > if (IS_ERR(entry)) {
    > ret = PTR_ERR(entry);
    > break;
    > --
    > 2.25.1
    >

    If you unconditionally set vm_max_prot_bits to RWX for dynamically created
    pags, you would not need to do this.

    These patches could be then safely dropped then:

    - [PATCH V2 06/32] x86/sgx: Support VMA permissions more relaxed than enclave permissions
    - [PATCH V2 08/32] x86/sgx: x86/sgx: Add sgx_encl_page->vm_run_prot_bits for dynamic permission changes
    - [PATCH V2 15/32] x86/sgx: Support relaxing of enclave page permissions

    And that would also keep full ABI compatibility without exceptions to the
    existing mainline code.

    BR, Jarkko

    \
     
     \ /
      Last update: 2022-03-07 18:11    [W:2.262 / U:5.824 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site