| Message ID | 50b9da1f491eb31d9b038afa0d75871965f474eb.1648847675.git.reinette.chatre@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/sgx and selftests/sgx: Support SGX2 | expand |
On Mon, 2022-04-04 at 09:49 -0700, Reinette Chatre wrote: > With SGX1 an enclave needs to be created with its maximum memory demands > allocated. Pages cannot be added to an enclave after it is initialized. > SGX2 introduces a new function, ENCLS[EAUG], that can be used to add > pages to an initialized enclave. With SGX2 the enclave still needs to > set aside address space for its maximum memory demands during enclave > creation, but all pages need not be added before enclave initialization. > Pages can be added during enclave runtime. > > Add support for dynamically adding pages to an initialized enclave, > architecturally limited to RW permission at creation but allowed to > obtain RWX permissions after enclave runs EMODPE. Add pages via the > page fault handler at the time an enclave address without a backing > enclave page is accessed, potentially directly reclaiming pages if > no free pages are available. > > The enclave is still required to run ENCLU[EACCEPT] on the page before > it can be used. A useful flow is for the enclave to run ENCLU[EACCEPT] > on an uninitialized address. This will trigger the page fault handler > that will add the enclave page and return execution to the enclave to > repeat the ENCLU[EACCEPT] instruction, this time successful. > > If the enclave accesses an uninitialized address in another way, for > example by expanding the enclave stack to a page that has not yet been > added, then the page fault handler would add the page on the first > write but upon returning to the enclave the instruction that triggered > the page fault would be repeated and since ENCLU[EACCEPT] was not run > yet it would trigger a second page fault, this time with the SGX flag > set in the page fault error code. This can only be recovered by entering > the enclave again and directly running the ENCLU[EACCEPT] instruction on > the now initialized address. > > Accessing an uninitialized address from outside the enclave also > triggers this flow but the page will remain inaccessible (access will > result in #PF) until accepted from within the enclave via > ENCLU[EACCEPT]. > > Signed-off-by: Reinette Chatre <reinette.chatre@intel.com> > --- > Changes since V2: > - Remove runtime tracking of EPCM permissions > (sgx_encl_page->vm_run_prot_bits) (Jarkko). > - Move export of sgx_encl_{grow,shrink}() to separate patch. (Jarkko) > - Use sgx_encl_page_alloc(). (Jarkko) > - Set max allowed permissions to be RWX (Jarkko). Update changelog > to indicate the change and use comment in code as > created by Jarkko in: > https://lore.kernel.org/linux-sgx/20220306053211.135762-4-jarkko@kernel.org > - Do not set protection bits but let it be inherited by VMA (Jarkko) > > Changes since V1: > - Fix subject line "to initialized" -> "to an initialized" (Jarkko). > - Move text about hardware's PENDING state to the patch that introduces > the ENCLS[EAUG] wrapper (Jarkko). > - Ensure kernel-doc uses brackets when referring to function. > > arch/x86/kernel/cpu/sgx/encl.c | 124 +++++++++++++++++++++++++++++++++ > 1 file changed, 124 insertions(+) > > diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c > index 546423753e4c..fa4f947f8496 100644 > --- a/arch/x86/kernel/cpu/sgx/encl.c > +++ b/arch/x86/kernel/cpu/sgx/encl.c > @@ -194,6 +194,119 @@ struct sgx_encl_page *sgx_encl_load_page(struct sgx_encl *encl, > return __sgx_encl_load_page(encl, entry); > } > > +/** > + * sgx_encl_eaug_page() - Dynamically add page to initialized enclave > + * @vma: VMA obtained from fault info from where page is accessed > + * @encl: enclave accessing the page > + * @addr: address that triggered the page fault > + * > + * When an initialized enclave accesses a page with no backing EPC page > + * on a SGX2 system then the EPC can be added dynamically via the SGX2 > + * ENCLS[EAUG] instruction. > + * > + * Returns: Appropriate vm_fault_t: VM_FAULT_NOPAGE when PTE was installed > + * successfully, VM_FAULT_SIGBUS or VM_FAULT_OOM as error otherwise. > + */ > +static vm_fault_t sgx_encl_eaug_page(struct vm_area_struct *vma, > + struct sgx_encl *encl, unsigned long addr) > +{ > + struct sgx_pageinfo pginfo = {0}; > + struct sgx_encl_page *encl_page; > + struct sgx_epc_page *epc_page; > + struct sgx_va_page *va_page; > + unsigned long phys_addr; > + u64 secinfo_flags; > + vm_fault_t vmret; > + int ret; > + > + if (!test_bit(SGX_ENCL_INITIALIZED, &encl->flags)) > + return VM_FAULT_SIGBUS; > + > + /* > + * Ignore internal permission checking for dynamically added pages. > + * They matter only for data added during the pre-initialization > + * phase. The enclave decides the permissions by the means of > + * EACCEPT, EACCEPTCOPY and EMODPE. > + */ > + secinfo_flags = SGX_SECINFO_R | SGX_SECINFO_W | SGX_SECINFO_X; > + encl_page = sgx_encl_page_alloc(encl, addr - encl->base, secinfo_flags); > + if (IS_ERR(encl_page)) > + return VM_FAULT_OOM; > + > + epc_page = sgx_alloc_epc_page(encl_page, true); > + if (IS_ERR(epc_page)) { > + kfree(encl_page); > + return VM_FAULT_SIGBUS; > + } > + > + va_page = sgx_encl_grow(encl); > + if (IS_ERR(va_page)) { > + ret = PTR_ERR(va_page); > + goto err_out_free; > + } > + > + mutex_lock(&encl->lock); > + > + /* > + * Copy comment from sgx_encl_add_page() to maintain guidance in > + * this similar flow: > + * Adding to encl->va_pages must be done under encl->lock. Ditto for > + * deleting (via sgx_encl_shrink()) in the error path. > + */ > + if (va_page) > + list_add(&va_page->list, &encl->va_pages); > + > + ret = xa_insert(&encl->page_array, PFN_DOWN(encl_page->desc), > + encl_page, GFP_KERNEL); > + /* > + * If ret == -EBUSY then page was created in another flow while > + * running without encl->lock > + */ > + if (ret) > + goto err_out_unlock; > + > + pginfo.secs = (unsigned long)sgx_get_epc_virt_addr(encl->secs.epc_page); > + pginfo.addr = encl_page->desc & PAGE_MASK; > + pginfo.metadata = 0; > + > + ret = __eaug(&pginfo, sgx_get_epc_virt_addr(epc_page)); > + if (ret) > + goto err_out; > + > + encl_page->encl = encl; > + encl_page->epc_page = epc_page; > + encl_page->type = SGX_PAGE_TYPE_REG; > + encl->secs_child_cnt++; > + > + sgx_mark_page_reclaimable(encl_page->epc_page); > + > + phys_addr = sgx_get_epc_phys_addr(epc_page); > + /* > + * Do not undo everything when creating PTE entry fails - next #PF > + * would find page ready for a PTE. > + */ > + vmret = vmf_insert_pfn(vma, addr, PFN_DOWN(phys_addr)); > + if (vmret != VM_FAULT_NOPAGE) { > + mutex_unlock(&encl->lock); > + return VM_FAULT_SIGBUS; > + } > + mutex_unlock(&encl->lock); > + return VM_FAULT_NOPAGE; > + > +err_out: > + xa_erase(&encl->page_array, PFN_DOWN(encl_page->desc)); > + > +err_out_unlock: > + sgx_encl_shrink(encl, va_page); > + mutex_unlock(&encl->lock); > + > +err_out_free: > + sgx_encl_free_epc_page(epc_page); > + kfree(encl_page); > + > + return VM_FAULT_SIGBUS; > +} > + > static vm_fault_t sgx_vma_fault(struct vm_fault *vmf) > { > unsigned long addr = (unsigned long)vmf->address; > @@ -213,6 +326,17 @@ static vm_fault_t sgx_vma_fault(struct vm_fault *vmf) > if (unlikely(!encl)) > return VM_FAULT_SIGBUS; > > + /* > + * The page_array keeps track of all enclave pages, whether they > + * are swapped out or not. If there is no entry for this page and > + * the system supports SGX2 then it is possible to dynamically add > + * a new enclave page. This is only possible for an initialized > + * enclave that will be checked for right away. > + */ > + if (cpu_feature_enabled(X86_FEATURE_SGX2) && > + (!xa_load(&encl->page_array, PFN_DOWN(addr)))) > + return sgx_encl_eaug_page(vma, encl, addr); > + > mutex_lock(&encl->lock); > > entry = sgx_encl_load_page_in_vma(encl, addr, vma->vm_flags); Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> BR, Jarkko
On Tue, 2022-04-05 at 08:05 +0300, Jarkko Sakkinen wrote: > On Mon, 2022-04-04 at 09:49 -0700, Reinette Chatre wrote: > > With SGX1 an enclave needs to be created with its maximum memory demands > > allocated. Pages cannot be added to an enclave after it is initialized. > > SGX2 introduces a new function, ENCLS[EAUG], that can be used to add > > pages to an initialized enclave. With SGX2 the enclave still needs to > > set aside address space for its maximum memory demands during enclave > > creation, but all pages need not be added before enclave initialization. > > Pages can be added during enclave runtime. > > > > Add support for dynamically adding pages to an initialized enclave, > > architecturally limited to RW permission at creation but allowed to > > obtain RWX permissions after enclave runs EMODPE. Add pages via the > > page fault handler at the time an enclave address without a backing > > enclave page is accessed, potentially directly reclaiming pages if > > no free pages are available. > > > > The enclave is still required to run ENCLU[EACCEPT] on the page before > > it can be used. A useful flow is for the enclave to run ENCLU[EACCEPT] > > on an uninitialized address. This will trigger the page fault handler > > that will add the enclave page and return execution to the enclave to > > repeat the ENCLU[EACCEPT] instruction, this time successful. > > > > If the enclave accesses an uninitialized address in another way, for > > example by expanding the enclave stack to a page that has not yet been > > added, then the page fault handler would add the page on the first > > write but upon returning to the enclave the instruction that triggered > > the page fault would be repeated and since ENCLU[EACCEPT] was not run > > yet it would trigger a second page fault, this time with the SGX flag > > set in the page fault error code. This can only be recovered by entering > > the enclave again and directly running the ENCLU[EACCEPT] instruction on > > the now initialized address. > > > > Accessing an uninitialized address from outside the enclave also > > triggers this flow but the page will remain inaccessible (access will > > result in #PF) until accepted from within the enclave via > > ENCLU[EACCEPT]. > > > > Signed-off-by: Reinette Chatre <reinette.chatre@intel.com> > > --- > > Changes since V2: > > - Remove runtime tracking of EPCM permissions > > (sgx_encl_page->vm_run_prot_bits) (Jarkko). > > - Move export of sgx_encl_{grow,shrink}() to separate patch. (Jarkko) > > - Use sgx_encl_page_alloc(). (Jarkko) > > - Set max allowed permissions to be RWX (Jarkko). Update changelog > > to indicate the change and use comment in code as > > created by Jarkko in: > > https://lore.kernel.org/linux-sgx/20220306053211.135762-4-jarkko@kernel.org > > - Do not set protection bits but let it be inherited by VMA (Jarkko) > > > > Changes since V1: > > - Fix subject line "to initialized" -> "to an initialized" (Jarkko). > > - Move text about hardware's PENDING state to the patch that introduces > > the ENCLS[EAUG] wrapper (Jarkko). > > - Ensure kernel-doc uses brackets when referring to function. > > > > arch/x86/kernel/cpu/sgx/encl.c | 124 +++++++++++++++++++++++++++++++++ > > 1 file changed, 124 insertions(+) > > > > diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c > > index 546423753e4c..fa4f947f8496 100644 > > --- a/arch/x86/kernel/cpu/sgx/encl.c > > +++ b/arch/x86/kernel/cpu/sgx/encl.c > > @@ -194,6 +194,119 @@ struct sgx_encl_page *sgx_encl_load_page(struct sgx_encl *encl, > > return __sgx_encl_load_page(encl, entry); > > } > > > > +/** > > + * sgx_encl_eaug_page() - Dynamically add page to initialized enclave > > + * @vma: VMA obtained from fault info from where page is accessed > > + * @encl: enclave accessing the page > > + * @addr: address that triggered the page fault > > + * > > + * When an initialized enclave accesses a page with no backing EPC page > > + * on a SGX2 system then the EPC can be added dynamically via the SGX2 > > + * ENCLS[EAUG] instruction. > > + * > > + * Returns: Appropriate vm_fault_t: VM_FAULT_NOPAGE when PTE was installed > > + * successfully, VM_FAULT_SIGBUS or VM_FAULT_OOM as error otherwise. > > + */ > > +static vm_fault_t sgx_encl_eaug_page(struct vm_area_struct *vma, > > + struct sgx_encl *encl, unsigned long addr) > > +{ > > + struct sgx_pageinfo pginfo = {0}; > > + struct sgx_encl_page *encl_page; > > + struct sgx_epc_page *epc_page; > > + struct sgx_va_page *va_page; > > + unsigned long phys_addr; > > + u64 secinfo_flags; > > + vm_fault_t vmret; > > + int ret; > > + > > + if (!test_bit(SGX_ENCL_INITIALIZED, &encl->flags)) > > + return VM_FAULT_SIGBUS; > > + > > + /* > > + * Ignore internal permission checking for dynamically added pages. > > + * They matter only for data added during the pre-initialization > > + * phase. The enclave decides the permissions by the means of > > + * EACCEPT, EACCEPTCOPY and EMODPE. > > + */ > > + secinfo_flags = SGX_SECINFO_R | SGX_SECINFO_W | SGX_SECINFO_X; > > + encl_page = sgx_encl_page_alloc(encl, addr - encl->base, secinfo_flags); > > + if (IS_ERR(encl_page)) > > + return VM_FAULT_OOM; > > + > > + epc_page = sgx_alloc_epc_page(encl_page, true); > > + if (IS_ERR(epc_page)) { > > + kfree(encl_page); > > + return VM_FAULT_SIGBUS; > > + } > > + > > + va_page = sgx_encl_grow(encl); > > + if (IS_ERR(va_page)) { > > + ret = PTR_ERR(va_page); > > + goto err_out_free; > > + } > > + > > + mutex_lock(&encl->lock); > > + > > + /* > > + * Copy comment from sgx_encl_add_page() to maintain guidance in > > + * this similar flow: > > + * Adding to encl->va_pages must be done under encl->lock. Ditto for > > + * deleting (via sgx_encl_shrink()) in the error path. > > + */ > > + if (va_page) > > + list_add(&va_page->list, &encl->va_pages); > > + > > + ret = xa_insert(&encl->page_array, PFN_DOWN(encl_page->desc), > > + encl_page, GFP_KERNEL); > > + /* > > + * If ret == -EBUSY then page was created in another flow while > > + * running without encl->lock > > + */ > > + if (ret) > > + goto err_out_unlock; > > + > > + pginfo.secs = (unsigned long)sgx_get_epc_virt_addr(encl->secs.epc_page); > > + pginfo.addr = encl_page->desc & PAGE_MASK; > > + pginfo.metadata = 0; > > + > > + ret = __eaug(&pginfo, sgx_get_epc_virt_addr(epc_page)); > > + if (ret) > > + goto err_out; > > + > > + encl_page->encl = encl; > > + encl_page->epc_page = epc_page; > > + encl_page->type = SGX_PAGE_TYPE_REG; > > + encl->secs_child_cnt++; > > + > > + sgx_mark_page_reclaimable(encl_page->epc_page); > > + > > + phys_addr = sgx_get_epc_phys_addr(epc_page); > > + /* > > + * Do not undo everything when creating PTE entry fails - next #PF > > + * would find page ready for a PTE. > > + */ > > + vmret = vmf_insert_pfn(vma, addr, PFN_DOWN(phys_addr)); > > + if (vmret != VM_FAULT_NOPAGE) { > > + mutex_unlock(&encl->lock); > > + return VM_FAULT_SIGBUS; > > + } > > + mutex_unlock(&encl->lock); > > + return VM_FAULT_NOPAGE; > > + > > +err_out: > > + xa_erase(&encl->page_array, PFN_DOWN(encl_page->desc)); > > + > > +err_out_unlock: > > + sgx_encl_shrink(encl, va_page); > > + mutex_unlock(&encl->lock); > > + > > +err_out_free: > > + sgx_encl_free_epc_page(epc_page); > > + kfree(encl_page); > > + > > + return VM_FAULT_SIGBUS; > > +} > > + > > static vm_fault_t sgx_vma_fault(struct vm_fault *vmf) > > { > > unsigned long addr = (unsigned long)vmf->address; > > @@ -213,6 +326,17 @@ static vm_fault_t sgx_vma_fault(struct vm_fault *vmf) > > if (unlikely(!encl)) > > return VM_FAULT_SIGBUS; > > > > + /* > > + * The page_array keeps track of all enclave pages, whether they > > + * are swapped out or not. If there is no entry for this page and > > + * the system supports SGX2 then it is possible to dynamically add > > + * a new enclave page. This is only possible for an initialized > > + * enclave that will be checked for right away. > > + */ > > + if (cpu_feature_enabled(X86_FEATURE_SGX2) && > > + (!xa_load(&encl->page_array, PFN_DOWN(addr)))) > > + return sgx_encl_eaug_page(vma, encl, addr); > > + > > mutex_lock(&encl->lock); > > > > entry = sgx_encl_load_page_in_vma(encl, addr, vma->vm_flags); > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> Tested-by: Jarkko Sakkinen <jarkko@kernel.org> BR, Jarkko
On Tue, 2022-04-05 at 13:03 +0300, Jarkko Sakkinen wrote: > On Tue, 2022-04-05 at 08:05 +0300, Jarkko Sakkinen wrote: > > On Mon, 2022-04-04 at 09:49 -0700, Reinette Chatre wrote: > > > With SGX1 an enclave needs to be created with its maximum memory demands > > > allocated. Pages cannot be added to an enclave after it is initialized. > > > SGX2 introduces a new function, ENCLS[EAUG], that can be used to add > > > pages to an initialized enclave. With SGX2 the enclave still needs to > > > set aside address space for its maximum memory demands during enclave > > > creation, but all pages need not be added before enclave initialization. > > > Pages can be added during enclave runtime. > > > > > > Add support for dynamically adding pages to an initialized enclave, > > > architecturally limited to RW permission at creation but allowed to > > > obtain RWX permissions after enclave runs EMODPE. Add pages via the > > > page fault handler at the time an enclave address without a backing > > > enclave page is accessed, potentially directly reclaiming pages if > > > no free pages are available. > > > > > > The enclave is still required to run ENCLU[EACCEPT] on the page before > > > it can be used. A useful flow is for the enclave to run ENCLU[EACCEPT] > > > on an uninitialized address. This will trigger the page fault handler > > > that will add the enclave page and return execution to the enclave to > > > repeat the ENCLU[EACCEPT] instruction, this time successful. > > > > > > If the enclave accesses an uninitialized address in another way, for > > > example by expanding the enclave stack to a page that has not yet been > > > added, then the page fault handler would add the page on the first > > > write but upon returning to the enclave the instruction that triggered > > > the page fault would be repeated and since ENCLU[EACCEPT] was not run > > > yet it would trigger a second page fault, this time with the SGX flag > > > set in the page fault error code. This can only be recovered by entering > > > the enclave again and directly running the ENCLU[EACCEPT] instruction on > > > the now initialized address. > > > > > > Accessing an uninitialized address from outside the enclave also > > > triggers this flow but the page will remain inaccessible (access will > > > result in #PF) until accepted from within the enclave via > > > ENCLU[EACCEPT]. > > > > > > Signed-off-by: Reinette Chatre <reinette.chatre@intel.com> > > > --- > > > Changes since V2: > > > - Remove runtime tracking of EPCM permissions > > > (sgx_encl_page->vm_run_prot_bits) (Jarkko). > > > - Move export of sgx_encl_{grow,shrink}() to separate patch. (Jarkko) > > > - Use sgx_encl_page_alloc(). (Jarkko) > > > - Set max allowed permissions to be RWX (Jarkko). Update changelog > > > to indicate the change and use comment in code as > > > created by Jarkko in: > > > https://lore.kernel.org/linux-sgx/20220306053211.135762-4-jarkko@kernel.org > > > - Do not set protection bits but let it be inherited by VMA (Jarkko) > > > > > > Changes since V1: > > > - Fix subject line "to initialized" -> "to an initialized" (Jarkko). > > > - Move text about hardware's PENDING state to the patch that introduces > > > the ENCLS[EAUG] wrapper (Jarkko). > > > - Ensure kernel-doc uses brackets when referring to function. > > > > > > arch/x86/kernel/cpu/sgx/encl.c | 124 +++++++++++++++++++++++++++++++++ > > > 1 file changed, 124 insertions(+) > > > > > > diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c > > > index 546423753e4c..fa4f947f8496 100644 > > > --- a/arch/x86/kernel/cpu/sgx/encl.c > > > +++ b/arch/x86/kernel/cpu/sgx/encl.c > > > @@ -194,6 +194,119 @@ struct sgx_encl_page *sgx_encl_load_page(struct sgx_encl *encl, > > > return __sgx_encl_load_page(encl, entry); > > > } > > > > > > +/** > > > + * sgx_encl_eaug_page() - Dynamically add page to initialized enclave > > > + * @vma: VMA obtained from fault info from where page is accessed > > > + * @encl: enclave accessing the page > > > + * @addr: address that triggered the page fault > > > + * > > > + * When an initialized enclave accesses a page with no backing EPC page > > > + * on a SGX2 system then the EPC can be added dynamically via the SGX2 > > > + * ENCLS[EAUG] instruction. > > > + * > > > + * Returns: Appropriate vm_fault_t: VM_FAULT_NOPAGE when PTE was installed > > > + * successfully, VM_FAULT_SIGBUS or VM_FAULT_OOM as error otherwise. > > > + */ > > > +static vm_fault_t sgx_encl_eaug_page(struct vm_area_struct *vma, > > > + struct sgx_encl *encl, unsigned long addr) > > > +{ > > > + struct sgx_pageinfo pginfo = {0}; > > > + struct sgx_encl_page *encl_page; > > > + struct sgx_epc_page *epc_page; > > > + struct sgx_va_page *va_page; > > > + unsigned long phys_addr; > > > + u64 secinfo_flags; > > > + vm_fault_t vmret; > > > + int ret; > > > + > > > + if (!test_bit(SGX_ENCL_INITIALIZED, &encl->flags)) > > > + return VM_FAULT_SIGBUS; > > > + > > > + /* > > > + * Ignore internal permission checking for dynamically added pages. > > > + * They matter only for data added during the pre-initialization > > > + * phase. The enclave decides the permissions by the means of > > > + * EACCEPT, EACCEPTCOPY and EMODPE. > > > + */ > > > + secinfo_flags = SGX_SECINFO_R | SGX_SECINFO_W | SGX_SECINFO_X; > > > + encl_page = sgx_encl_page_alloc(encl, addr - encl->base, secinfo_flags); > > > + if (IS_ERR(encl_page)) > > > + return VM_FAULT_OOM; > > > + > > > + epc_page = sgx_alloc_epc_page(encl_page, true); > > > + if (IS_ERR(epc_page)) { > > > + kfree(encl_page); > > > + return VM_FAULT_SIGBUS; > > > + } > > > + > > > + va_page = sgx_encl_grow(encl); > > > + if (IS_ERR(va_page)) { > > > + ret = PTR_ERR(va_page); > > > + goto err_out_free; > > > + } > > > + > > > + mutex_lock(&encl->lock); > > > + > > > + /* > > > + * Copy comment from sgx_encl_add_page() to maintain guidance in > > > + * this similar flow: > > > + * Adding to encl->va_pages must be done under encl->lock. Ditto for > > > + * deleting (via sgx_encl_shrink()) in the error path. > > > + */ > > > + if (va_page) > > > + list_add(&va_page->list, &encl->va_pages); > > > + > > > + ret = xa_insert(&encl->page_array, PFN_DOWN(encl_page->desc), > > > + encl_page, GFP_KERNEL); > > > + /* > > > + * If ret == -EBUSY then page was created in another flow while > > > + * running without encl->lock > > > + */ > > > + if (ret) > > > + goto err_out_unlock; > > > + > > > + pginfo.secs = (unsigned long)sgx_get_epc_virt_addr(encl->secs.epc_page); > > > + pginfo.addr = encl_page->desc & PAGE_MASK; > > > + pginfo.metadata = 0; > > > + > > > + ret = __eaug(&pginfo, sgx_get_epc_virt_addr(epc_page)); > > > + if (ret) > > > + goto err_out; > > > + > > > + encl_page->encl = encl; > > > + encl_page->epc_page = epc_page; > > > + encl_page->type = SGX_PAGE_TYPE_REG; > > > + encl->secs_child_cnt++; > > > + > > > + sgx_mark_page_reclaimable(encl_page->epc_page); > > > + > > > + phys_addr = sgx_get_epc_phys_addr(epc_page); > > > + /* > > > + * Do not undo everything when creating PTE entry fails - next #PF > > > + * would find page ready for a PTE. > > > + */ > > > + vmret = vmf_insert_pfn(vma, addr, PFN_DOWN(phys_addr)); > > > + if (vmret != VM_FAULT_NOPAGE) { > > > + mutex_unlock(&encl->lock); > > > + return VM_FAULT_SIGBUS; > > > + } > > > + mutex_unlock(&encl->lock); > > > + return VM_FAULT_NOPAGE; > > > + > > > +err_out: > > > + xa_erase(&encl->page_array, PFN_DOWN(encl_page->desc)); > > > + > > > +err_out_unlock: > > > + sgx_encl_shrink(encl, va_page); > > > + mutex_unlock(&encl->lock); > > > + > > > +err_out_free: > > > + sgx_encl_free_epc_page(epc_page); > > > + kfree(encl_page); > > > + > > > + return VM_FAULT_SIGBUS; > > > +} > > > + > > > static vm_fault_t sgx_vma_fault(struct vm_fault *vmf) > > > { > > > unsigned long addr = (unsigned long)vmf->address; > > > @@ -213,6 +326,17 @@ static vm_fault_t sgx_vma_fault(struct vm_fault *vmf) > > > if (unlikely(!encl)) > > > return VM_FAULT_SIGBUS; > > > > > > + /* > > > + * The page_array keeps track of all enclave pages, whether they > > > + * are swapped out or not. If there is no entry for this page and > > > + * the system supports SGX2 then it is possible to dynamically add > > > + * a new enclave page. This is only possible for an initialized > > > + * enclave that will be checked for right away. > > > + */ > > > + if (cpu_feature_enabled(X86_FEATURE_SGX2) && > > > + (!xa_load(&encl->page_array, PFN_DOWN(addr)))) > > > + return sgx_encl_eaug_page(vma, encl, addr); > > > + > > > mutex_lock(&encl->lock); > > > > > > entry = sgx_encl_load_page_in_vma(encl, addr, vma->vm_flags); > > > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> > > Tested-by: Jarkko Sakkinen <jarkko@kernel.org> For what is worth I also get a full pass with our test suite, where the runtime is using EAUG together with EACCEPTCOPY: Finished test [unoptimized + debuginfo] target(s) in 0.26s Running unittests src/main.rs (target/debug/deps/enarx-ee7f422740eab404) running 7 tests test backend::sgx::attestation::tests::request_target_info ... ignored test backend::sev::snp::tests::test_const_id_macro ... ok test backend::sev::snp::firmware::test::test_vcek_url ... ok test backend::sgx::ioctls::tests::restrict_permissions ... ok test cli::snp::tests::test_empty_cache_path ... ok test workldr::wasmldr::test::is_builtin ... ok test cli::snp::tests::test_get_or_write ... ok test result: ok. 6 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.20s Running tests/c_integration_tests.rs (target/debug/deps/c_integration_tests-f7a69c2274f59f90) running 21 tests test get_att ... ignored test bind ... ok test clock_gettime ... ok test close ... ok test exit_one ... ok test getegid ... ok test geteuid ... ok test sgx_get_att_quote ... ignored test sgx_get_att_quote_size ... ignored test exit_zero ... ok test getgid ... ok test write_emsgsize ... ignored test write_stderr ... ignored test getuid ... ok test listen ... ok test read ... ok test read_udp ... ok test readv ... ok test socket ... ok test uname ... ok test write_stdout ... ok test result: ok. 16 passed; 0 failed; 5 ignored; 0 measured; 0 filtered out; finished in 18.46s Running tests/rust_integration_tests.rs (target/debug/deps/rust_integration_tests-0122fb231e20ea63) running 6 tests test rust_sev_attestation ... ignored test echo ... ok test cpuid ... ok test memory_stress_test ... ok test memspike ... ok test unix_echo ... ok test result: ok. 5 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 48.22s Running tests/wasmldr_tests.rs (target/debug/deps/wasmldr_tests-98b6ff656b9d815e) running 9 tests test check_tcp ... ok test hello_wasi_snapshot1 ... ok test memspike ... ok test echo has been running for over 60 seconds test memory_stress_test has been running for over 60 seconds test no_export has been running for over 60 seconds test return_1 has been running for over 60 seconds test wasi_snapshot1 has been running for over 60 seconds test memory_stress_test ... ok Error: default export in '' is not a function test no_export ... ok test return_1 ... ok test wasi_snapshot1 ... ok test zerooneone ... ok test echo ... ok test result: ok. 9 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 102.75s BR, Jarkko
Hi Jarkko, On 4/6/2022 12:37 AM, Jarkko Sakkinen wrote: ... > For what is worth I also get a full pass with our test suite, > where the runtime is using EAUG together with EACCEPTCOPY: Thank you very much for all the testing. Haitao is also busy with significant testing and uncovered a bug via encountering the WARN at arch/x86/kernel/cpu/sgx/ioctl.c:40. The issue is clear and I prepared the fix below that can be applied on top of this series. I plan to split this patch in two (1st the main change, 2nd the changes to sgx_encl_eaug_page() that will be rolled into "x86/sgx: Support adding of pages to an initialized enclave") and roll it into the next version of this series: -----8<----- Subject: [PATCH] x86/sgx: Support VA page allocation without reclaiming VA page allocation is done during enclave creation and adding of pages to an enclave. In both usages VA pages are allocated to always attempt a direct reclaim if no EPC pages are available and because of this the VA pages are allocated without the enclave's mutex held. Both usages are protected from concurrent attempts with an atomic operation on SGX_ENCL_IOCTL making it possible to allocate the VA pages without the enclave's mutex held. Dynamically adding pages via the page fault handler does not have the protection of SGX_ENCL_IOCTL but does not require VA pages to be allocated with default direct reclaim. Make VA page allocation with direct reclaim optional to make it possible to perform allocation with enclave's mutex held and thus protect against concurrent updates to encl->page_cnt. Reported-by: Haitao Huang <haitao.huang@intel.com> Tested-by: Haitao Huang <haitao.huang@intel.com> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com> --- arch/x86/kernel/cpu/sgx/encl.c | 27 +++++++++++---------------- arch/x86/kernel/cpu/sgx/encl.h | 4 ++-- arch/x86/kernel/cpu/sgx/ioctl.c | 8 ++++---- 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c index 7909570736a0..11f97fdcac1e 100644 --- a/arch/x86/kernel/cpu/sgx/encl.c +++ b/arch/x86/kernel/cpu/sgx/encl.c @@ -239,20 +239,14 @@ static vm_fault_t sgx_encl_eaug_page(struct vm_area_struct *vma, return VM_FAULT_SIGBUS; } - va_page = sgx_encl_grow(encl); + mutex_lock(&encl->lock); + + va_page = sgx_encl_grow(encl, false); if (IS_ERR(va_page)) { ret = PTR_ERR(va_page); - goto err_out_free; + goto err_out_unlock; } - mutex_lock(&encl->lock); - - /* - * Copy comment from sgx_encl_add_page() to maintain guidance in - * this similar flow: - * Adding to encl->va_pages must be done under encl->lock. Ditto for - * deleting (via sgx_encl_shrink()) in the error path. - */ if (va_page) list_add(&va_page->list, &encl->va_pages); @@ -263,7 +257,7 @@ static vm_fault_t sgx_encl_eaug_page(struct vm_area_struct *vma, * running without encl->lock */ if (ret) - goto err_out_unlock; + goto err_out_shrink; pginfo.secs = (unsigned long)sgx_get_epc_virt_addr(encl->secs.epc_page); pginfo.addr = encl_page->desc & PAGE_MASK; @@ -296,11 +290,10 @@ static vm_fault_t sgx_encl_eaug_page(struct vm_area_struct *vma, err_out: xa_erase(&encl->page_array, PFN_DOWN(encl_page->desc)); -err_out_unlock: +err_out_shrink: sgx_encl_shrink(encl, va_page); +err_out_unlock: mutex_unlock(&encl->lock); - -err_out_free: sgx_encl_free_epc_page(epc_page); kfree(encl_page); @@ -998,6 +991,8 @@ void sgx_zap_enclave_ptes(struct sgx_encl *encl, unsigned long addr) /** * sgx_alloc_va_page() - Allocate a Version Array (VA) page + * @reclaim: Reclaim EPC pages directly if none available. Enclave + * mutex should not be held if this is set. * * Allocate a free EPC page and convert it to a Version Array (VA) page. * @@ -1005,12 +1000,12 @@ void sgx_zap_enclave_ptes(struct sgx_encl *encl, unsigned long addr) * a VA page, * -errno otherwise */ -struct sgx_epc_page *sgx_alloc_va_page(void) +struct sgx_epc_page *sgx_alloc_va_page(bool reclaim) { struct sgx_epc_page *epc_page; int ret; - epc_page = sgx_alloc_epc_page(NULL, true); + epc_page = sgx_alloc_epc_page(NULL, reclaim); if (IS_ERR(epc_page)) return ERR_CAST(epc_page); diff --git a/arch/x86/kernel/cpu/sgx/encl.h b/arch/x86/kernel/cpu/sgx/encl.h index 253ebdd1c5be..66adb8faec45 100644 --- a/arch/x86/kernel/cpu/sgx/encl.h +++ b/arch/x86/kernel/cpu/sgx/encl.h @@ -116,14 +116,14 @@ struct sgx_encl_page *sgx_encl_page_alloc(struct sgx_encl *encl, unsigned long offset, u64 secinfo_flags); void sgx_zap_enclave_ptes(struct sgx_encl *encl, unsigned long addr); -struct sgx_epc_page *sgx_alloc_va_page(void); +struct sgx_epc_page *sgx_alloc_va_page(bool reclaim); unsigned int sgx_alloc_va_slot(struct sgx_va_page *va_page); void sgx_free_va_slot(struct sgx_va_page *va_page, unsigned int offset); bool sgx_va_page_full(struct sgx_va_page *va_page); void sgx_encl_free_epc_page(struct sgx_epc_page *page); struct sgx_encl_page *sgx_encl_load_page(struct sgx_encl *encl, unsigned long addr); -struct sgx_va_page *sgx_encl_grow(struct sgx_encl *encl); +struct sgx_va_page *sgx_encl_grow(struct sgx_encl *encl, bool reclaim); void sgx_encl_shrink(struct sgx_encl *encl, struct sgx_va_page *va_page); #endif /* _X86_ENCL_H */ diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index f88bc1236276..b163afff239f 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -17,7 +17,7 @@ #include "encl.h" #include "encls.h" -struct sgx_va_page *sgx_encl_grow(struct sgx_encl *encl) +struct sgx_va_page *sgx_encl_grow(struct sgx_encl *encl, bool reclaim) { struct sgx_va_page *va_page = NULL; void *err; @@ -30,7 +30,7 @@ struct sgx_va_page *sgx_encl_grow(struct sgx_encl *encl) if (!va_page) return ERR_PTR(-ENOMEM); - va_page->epc_page = sgx_alloc_va_page(); + va_page->epc_page = sgx_alloc_va_page(reclaim); if (IS_ERR(va_page->epc_page)) { err = ERR_CAST(va_page->epc_page); kfree(va_page); @@ -64,7 +64,7 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) struct file *backing; long ret; - va_page = sgx_encl_grow(encl); + va_page = sgx_encl_grow(encl, true); if (IS_ERR(va_page)) return PTR_ERR(va_page); else if (va_page) @@ -275,7 +275,7 @@ static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long src, return PTR_ERR(epc_page); } - va_page = sgx_encl_grow(encl); + va_page = sgx_encl_grow(encl, true); if (IS_ERR(va_page)) { ret = PTR_ERR(va_page); goto err_out_free;
diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c index 546423753e4c..fa4f947f8496 100644 --- a/arch/x86/kernel/cpu/sgx/encl.c +++ b/arch/x86/kernel/cpu/sgx/encl.c @@ -194,6 +194,119 @@ struct sgx_encl_page *sgx_encl_load_page(struct sgx_encl *encl, return __sgx_encl_load_page(encl, entry); } +/** + * sgx_encl_eaug_page() - Dynamically add page to initialized enclave + * @vma: VMA obtained from fault info from where page is accessed + * @encl: enclave accessing the page + * @addr: address that triggered the page fault + * + * When an initialized enclave accesses a page with no backing EPC page + * on a SGX2 system then the EPC can be added dynamically via the SGX2 + * ENCLS[EAUG] instruction. + * + * Returns: Appropriate vm_fault_t: VM_FAULT_NOPAGE when PTE was installed + * successfully, VM_FAULT_SIGBUS or VM_FAULT_OOM as error otherwise. + */ +static vm_fault_t sgx_encl_eaug_page(struct vm_area_struct *vma, + struct sgx_encl *encl, unsigned long addr) +{ + struct sgx_pageinfo pginfo = {0}; + struct sgx_encl_page *encl_page; + struct sgx_epc_page *epc_page; + struct sgx_va_page *va_page; + unsigned long phys_addr; + u64 secinfo_flags; + vm_fault_t vmret; + int ret; + + if (!test_bit(SGX_ENCL_INITIALIZED, &encl->flags)) + return VM_FAULT_SIGBUS; + + /* + * Ignore internal permission checking for dynamically added pages. + * They matter only for data added during the pre-initialization + * phase. The enclave decides the permissions by the means of + * EACCEPT, EACCEPTCOPY and EMODPE. + */ + secinfo_flags = SGX_SECINFO_R | SGX_SECINFO_W | SGX_SECINFO_X; + encl_page = sgx_encl_page_alloc(encl, addr - encl->base, secinfo_flags); + if (IS_ERR(encl_page)) + return VM_FAULT_OOM; + + epc_page = sgx_alloc_epc_page(encl_page, true); + if (IS_ERR(epc_page)) { + kfree(encl_page); + return VM_FAULT_SIGBUS; + } + + va_page = sgx_encl_grow(encl); + if (IS_ERR(va_page)) { + ret = PTR_ERR(va_page); + goto err_out_free; + } + + mutex_lock(&encl->lock); + + /* + * Copy comment from sgx_encl_add_page() to maintain guidance in + * this similar flow: + * Adding to encl->va_pages must be done under encl->lock. Ditto for + * deleting (via sgx_encl_shrink()) in the error path. + */ + if (va_page) + list_add(&va_page->list, &encl->va_pages); + + ret = xa_insert(&encl->page_array, PFN_DOWN(encl_page->desc), + encl_page, GFP_KERNEL); + /* + * If ret == -EBUSY then page was created in another flow while + * running without encl->lock + */ + if (ret) + goto err_out_unlock; + + pginfo.secs = (unsigned long)sgx_get_epc_virt_addr(encl->secs.epc_page); + pginfo.addr = encl_page->desc & PAGE_MASK; + pginfo.metadata = 0; + + ret = __eaug(&pginfo, sgx_get_epc_virt_addr(epc_page)); + if (ret) + goto err_out; + + encl_page->encl = encl; + encl_page->epc_page = epc_page; + encl_page->type = SGX_PAGE_TYPE_REG; + encl->secs_child_cnt++; + + sgx_mark_page_reclaimable(encl_page->epc_page); + + phys_addr = sgx_get_epc_phys_addr(epc_page); + /* + * Do not undo everything when creating PTE entry fails - next #PF + * would find page ready for a PTE. + */ + vmret = vmf_insert_pfn(vma, addr, PFN_DOWN(phys_addr)); + if (vmret != VM_FAULT_NOPAGE) { + mutex_unlock(&encl->lock); + return VM_FAULT_SIGBUS; + } + mutex_unlock(&encl->lock); + return VM_FAULT_NOPAGE; + +err_out: + xa_erase(&encl->page_array, PFN_DOWN(encl_page->desc)); + +err_out_unlock: + sgx_encl_shrink(encl, va_page); + mutex_unlock(&encl->lock); + +err_out_free: + sgx_encl_free_epc_page(epc_page); + kfree(encl_page); + + return VM_FAULT_SIGBUS; +} + static vm_fault_t sgx_vma_fault(struct vm_fault *vmf) { unsigned long addr = (unsigned long)vmf->address; @@ -213,6 +326,17 @@ static vm_fault_t sgx_vma_fault(struct vm_fault *vmf) if (unlikely(!encl)) return VM_FAULT_SIGBUS; + /* + * The page_array keeps track of all enclave pages, whether they + * are swapped out or not. If there is no entry for this page and + * the system supports SGX2 then it is possible to dynamically add + * a new enclave page. This is only possible for an initialized + * enclave that will be checked for right away. + */ + if (cpu_feature_enabled(X86_FEATURE_SGX2) && + (!xa_load(&encl->page_array, PFN_DOWN(addr)))) + return sgx_encl_eaug_page(vma, encl, addr); + mutex_lock(&encl->lock); entry = sgx_encl_load_page_in_vma(encl, addr, vma->vm_flags);
With SGX1 an enclave needs to be created with its maximum memory demands allocated. Pages cannot be added to an enclave after it is initialized. SGX2 introduces a new function, ENCLS[EAUG], that can be used to add pages to an initialized enclave. With SGX2 the enclave still needs to set aside address space for its maximum memory demands during enclave creation, but all pages need not be added before enclave initialization. Pages can be added during enclave runtime. Add support for dynamically adding pages to an initialized enclave, architecturally limited to RW permission at creation but allowed to obtain RWX permissions after enclave runs EMODPE. Add pages via the page fault handler at the time an enclave address without a backing enclave page is accessed, potentially directly reclaiming pages if no free pages are available. The enclave is still required to run ENCLU[EACCEPT] on the page before it can be used. A useful flow is for the enclave to run ENCLU[EACCEPT] on an uninitialized address. This will trigger the page fault handler that will add the enclave page and return execution to the enclave to repeat the ENCLU[EACCEPT] instruction, this time successful. If the enclave accesses an uninitialized address in another way, for example by expanding the enclave stack to a page that has not yet been added, then the page fault handler would add the page on the first write but upon returning to the enclave the instruction that triggered the page fault would be repeated and since ENCLU[EACCEPT] was not run yet it would trigger a second page fault, this time with the SGX flag set in the page fault error code. This can only be recovered by entering the enclave again and directly running the ENCLU[EACCEPT] instruction on the now initialized address. Accessing an uninitialized address from outside the enclave also triggers this flow but the page will remain inaccessible (access will result in #PF) until accepted from within the enclave via ENCLU[EACCEPT]. Signed-off-by: Reinette Chatre <reinette.chatre@intel.com> --- Changes since V2: - Remove runtime tracking of EPCM permissions (sgx_encl_page->vm_run_prot_bits) (Jarkko). - Move export of sgx_encl_{grow,shrink}() to separate patch. (Jarkko) - Use sgx_encl_page_alloc(). (Jarkko) - Set max allowed permissions to be RWX (Jarkko). Update changelog to indicate the change and use comment in code as created by Jarkko in: https://lore.kernel.org/linux-sgx/20220306053211.135762-4-jarkko@kernel.org - Do not set protection bits but let it be inherited by VMA (Jarkko) Changes since V1: - Fix subject line "to initialized" -> "to an initialized" (Jarkko). - Move text about hardware's PENDING state to the patch that introduces the ENCLS[EAUG] wrapper (Jarkko). - Ensure kernel-doc uses brackets when referring to function. arch/x86/kernel/cpu/sgx/encl.c | 124 +++++++++++++++++++++++++++++++++ 1 file changed, 124 insertions(+)