| Message ID | 20220118175717.6922-3-kristen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/sgx: Limit EPC overcommit | expand |
On Tue, 2022-01-18 at 09:57 -0800, Kristen Carlson Accardi wrote: > SGX may allow EPC pages to be overcommitted. If the system is > out of enclave memory, EPC pages are swapped to normal RAM via > a per enclave shared memory area. This shared memory is not > charged to the enclave or the task mapping it, making it hard > to account for using normal methods. > > In order to avoid unlimited usage of normal RAM, enclaves must be > charged for each new page used for backing storage, and uncharged > when they are no longer using a backing page. > > Modify the existing flow for requesting backing pages to reduce the > available backing page counter and confirm that the limit has not > been exceeded. Backing page usage for loading EPC pages back out of > the shared memory do not incur a charge. > > When a backing page is released from usage, increment the available > backing page counter. > > When swapping EPC pages to RAM, in addition to storing the page > contents, SGX must store some additional metadata to protect > against a malicious kernel when the page is swapped back in. This > additional metadata is called Paging Crypto MetaData. PCMD is > allocated from the same shared memory area as the backing page > contents and consumes RAM the same way. > > PCMD is 128 bytes in size, and there is one PCMD structure per > page written to shared RAM. The page index for the PCMD page is > calculatted from the page index of the backing page, so it is > possible > that the PCMD structures are not packed into the minimum number of > pages possible. If 32 PCMDs can fit onto a single page, then PCMD > usage is 1/32 of total EPC pages. In the worst case, PCMD can > consume the same amount of RAM as EPC backing pages (1:1). For > simplicity, this implementation does not account for PCMD page usage. > > Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com> > Tested-by: Jarkko Sakkinen <jarkko@kernel.org> > --- > arch/x86/kernel/cpu/sgx/encl.c | 76 > ++++++++++++++++++++++++++++++++-- > arch/x86/kernel/cpu/sgx/encl.h | 6 ++- > arch/x86/kernel/cpu/sgx/main.c | 6 +-- > 3 files changed, 80 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/kernel/cpu/sgx/encl.c > b/arch/x86/kernel/cpu/sgx/encl.c > index 001808e3901c..8be6f0592bdc 100644 > --- a/arch/x86/kernel/cpu/sgx/encl.c > +++ b/arch/x86/kernel/cpu/sgx/encl.c > @@ -32,7 +32,7 @@ static int __sgx_encl_eldu(struct sgx_encl_page > *encl_page, > else > page_index = PFN_DOWN(encl->size); > > - ret = sgx_encl_get_backing(encl, page_index, &b); > + ret = sgx_encl_lookup_backing(encl, page_index, &b); > if (ret) > return ret; > > @@ -407,6 +407,12 @@ void sgx_encl_release(struct kref *ref) > sgx_encl_free_epc_page(entry->epc_page); > encl->secs_child_cnt--; > entry->epc_page = NULL; > + } else { > + /* > + * If there is no epc_page, it means it has > been > + * swapped out. Uncharge the backing storage. > + */ > + sgx_uncharge_mem(); > } > > kfree(entry); > @@ -574,8 +580,8 @@ static struct page > *sgx_encl_get_backing_page(struct sgx_encl *encl, > * 0 on success, > * -errno otherwise. > */ > -int sgx_encl_get_backing(struct sgx_encl *encl, unsigned long > page_index, > - struct sgx_backing *backing) > +static int sgx_encl_get_backing(struct sgx_encl *encl, unsigned long > page_index, > + struct sgx_backing *backing) > { > pgoff_t pcmd_index = PFN_DOWN(encl->size) + 1 + (page_index > >> 5); > struct page *contents; > @@ -601,6 +607,62 @@ int sgx_encl_get_backing(struct sgx_encl *encl, > unsigned long page_index, > return 0; > } > > +/** > + * sgx_encl_alloc_backing() - allocate a new backing storage page > + * @encl: an enclave pointer > + * @page_index: enclave page index > + * @backing: data for accessing backing storage for the page > + * > + * Confirm that the global overcommit limit has not been reached > before > + * requesting a new backing storage page for storing the encrypted > contents > + * and Paging Crypto MetaData (PCMD) of an enclave page. This is > called when > + * there is no existing backing page, just before writing to the > backing > + * storage with EWB. > + * > + * Return: > + * 0 on success, > + * -errno otherwise. > + */ > +int sgx_encl_alloc_backing(struct sgx_encl *encl, unsigned long > page_index, > + struct sgx_backing *backing) > +{ > + int ret; > + > + if (sgx_charge_mem()) > + return -ENOMEM; > + > + ret = sgx_encl_get_backing(encl, page_index, backing); > + if (ret) > + sgx_uncharge_mem(); > + > + return ret; > +} > + > +/** > + * sgx_encl_lookup_backing() - retrieve an existing backing storage > page > + * @encl: an enclave pointer > + * @page_index: enclave page index > + * @backing: data for accessing backing storage for the page > + * > + * Retrieve a backing page for loading data back into an EPC page > with ELDU. > + * This call does not cause a charge to the overcommit limit because > a page > + * has already been allocated, but has been swapped out or is in RAM > + * > + * It is the caller's responsibility to ensure that it is > appropriate to > + * use sgx_encl_lookup_backing() rather than > sgx_encl_alloc_backing(). If > + * lookup is not used correctly, this will cause an allocation that > is > + * not accounted for. > + * > + * Return: > + * 0 on success, > + * -errno otherwise. > + */ > +int sgx_encl_lookup_backing(struct sgx_encl *encl, unsigned long > page_index, > + struct sgx_backing *backing) > +{ > + return sgx_encl_get_backing(encl, page_index, backing); > +} > + > /** > * sgx_encl_put_backing() - Unpin the backing storage > * @backing: data for accessing backing storage for the page > @@ -608,9 +670,17 @@ int sgx_encl_get_backing(struct sgx_encl *encl, > unsigned long page_index, > */ > void sgx_encl_put_backing(struct sgx_backing *backing, bool > do_write) > { > + /* > + * If the page is being written to by the reclaimer, it is > + * still in use and the backing page usage should not be > + * uncharged. However, if the page is not being written to, > + * it is no longer in use and may be uncharged. > + */ > if (do_write) { > set_page_dirty(backing->pcmd); > set_page_dirty(backing->contents); > + } else { > + sgx_uncharge_mem(); > } > > put_page(backing->pcmd); > diff --git a/arch/x86/kernel/cpu/sgx/encl.h > b/arch/x86/kernel/cpu/sgx/encl.h > index fec43ca65065..8ffb8a83263f 100644 > --- a/arch/x86/kernel/cpu/sgx/encl.h > +++ b/arch/x86/kernel/cpu/sgx/encl.h > @@ -105,8 +105,10 @@ int sgx_encl_may_map(struct sgx_encl *encl, > unsigned long start, > > void sgx_encl_release(struct kref *ref); > int sgx_encl_mm_add(struct sgx_encl *encl, struct mm_struct *mm); > -int sgx_encl_get_backing(struct sgx_encl *encl, unsigned long > page_index, > - struct sgx_backing *backing); > +int sgx_encl_alloc_backing(struct sgx_encl *encl, unsigned long > page_index, > + struct sgx_backing *backing); > +int sgx_encl_lookup_backing(struct sgx_encl *encl, unsigned long > page_index, > + struct sgx_backing *backing); > void sgx_encl_put_backing(struct sgx_backing *backing, bool > do_write); > int sgx_encl_test_and_clear_young(struct mm_struct *mm, > struct sgx_encl_page *page); > diff --git a/arch/x86/kernel/cpu/sgx/main.c > b/arch/x86/kernel/cpu/sgx/main.c > index 261e3702aef9..990f341bbd30 100644 > --- a/arch/x86/kernel/cpu/sgx/main.c > +++ b/arch/x86/kernel/cpu/sgx/main.c > @@ -347,8 +347,8 @@ static void sgx_reclaimer_write(struct > sgx_epc_page *epc_page, > encl->secs_child_cnt--; > > if (!encl->secs_child_cnt && test_bit(SGX_ENCL_INITIALIZED, > &encl->flags)) { > - ret = sgx_encl_get_backing(encl, PFN_DOWN(encl- > >size), > - &secs_backing); > + ret = sgx_encl_alloc_backing(encl, PFN_DOWN(encl- > >size), > + &secs_backing); > if (ret) > goto out; > > @@ -418,7 +418,7 @@ static void sgx_reclaim_pages(void) > goto skip; > > page_index = PFN_DOWN(encl_page->desc - encl_page- > >encl->base); > - ret = sgx_encl_get_backing(encl_page->encl, > page_index, &backing[i]); > + ret = sgx_encl_alloc_backing(encl_page->encl, > page_index, &backing[i]); > if (ret) > goto skip; > Wrappers make sense to do extra checks so: Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> BR, Jarkko
diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c index 001808e3901c..8be6f0592bdc 100644 --- a/arch/x86/kernel/cpu/sgx/encl.c +++ b/arch/x86/kernel/cpu/sgx/encl.c @@ -32,7 +32,7 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page, else page_index = PFN_DOWN(encl->size); - ret = sgx_encl_get_backing(encl, page_index, &b); + ret = sgx_encl_lookup_backing(encl, page_index, &b); if (ret) return ret; @@ -407,6 +407,12 @@ void sgx_encl_release(struct kref *ref) sgx_encl_free_epc_page(entry->epc_page); encl->secs_child_cnt--; entry->epc_page = NULL; + } else { + /* + * If there is no epc_page, it means it has been + * swapped out. Uncharge the backing storage. + */ + sgx_uncharge_mem(); } kfree(entry); @@ -574,8 +580,8 @@ static struct page *sgx_encl_get_backing_page(struct sgx_encl *encl, * 0 on success, * -errno otherwise. */ -int sgx_encl_get_backing(struct sgx_encl *encl, unsigned long page_index, - struct sgx_backing *backing) +static int sgx_encl_get_backing(struct sgx_encl *encl, unsigned long page_index, + struct sgx_backing *backing) { pgoff_t pcmd_index = PFN_DOWN(encl->size) + 1 + (page_index >> 5); struct page *contents; @@ -601,6 +607,62 @@ int sgx_encl_get_backing(struct sgx_encl *encl, unsigned long page_index, return 0; } +/** + * sgx_encl_alloc_backing() - allocate a new backing storage page + * @encl: an enclave pointer + * @page_index: enclave page index + * @backing: data for accessing backing storage for the page + * + * Confirm that the global overcommit limit has not been reached before + * requesting a new backing storage page for storing the encrypted contents + * and Paging Crypto MetaData (PCMD) of an enclave page. This is called when + * there is no existing backing page, just before writing to the backing + * storage with EWB. + * + * Return: + * 0 on success, + * -errno otherwise. + */ +int sgx_encl_alloc_backing(struct sgx_encl *encl, unsigned long page_index, + struct sgx_backing *backing) +{ + int ret; + + if (sgx_charge_mem()) + return -ENOMEM; + + ret = sgx_encl_get_backing(encl, page_index, backing); + if (ret) + sgx_uncharge_mem(); + + return ret; +} + +/** + * sgx_encl_lookup_backing() - retrieve an existing backing storage page + * @encl: an enclave pointer + * @page_index: enclave page index + * @backing: data for accessing backing storage for the page + * + * Retrieve a backing page for loading data back into an EPC page with ELDU. + * This call does not cause a charge to the overcommit limit because a page + * has already been allocated, but has been swapped out or is in RAM + * + * It is the caller's responsibility to ensure that it is appropriate to + * use sgx_encl_lookup_backing() rather than sgx_encl_alloc_backing(). If + * lookup is not used correctly, this will cause an allocation that is + * not accounted for. + * + * Return: + * 0 on success, + * -errno otherwise. + */ +int sgx_encl_lookup_backing(struct sgx_encl *encl, unsigned long page_index, + struct sgx_backing *backing) +{ + return sgx_encl_get_backing(encl, page_index, backing); +} + /** * sgx_encl_put_backing() - Unpin the backing storage * @backing: data for accessing backing storage for the page @@ -608,9 +670,17 @@ int sgx_encl_get_backing(struct sgx_encl *encl, unsigned long page_index, */ void sgx_encl_put_backing(struct sgx_backing *backing, bool do_write) { + /* + * If the page is being written to by the reclaimer, it is + * still in use and the backing page usage should not be + * uncharged. However, if the page is not being written to, + * it is no longer in use and may be uncharged. + */ if (do_write) { set_page_dirty(backing->pcmd); set_page_dirty(backing->contents); + } else { + sgx_uncharge_mem(); } put_page(backing->pcmd); diff --git a/arch/x86/kernel/cpu/sgx/encl.h b/arch/x86/kernel/cpu/sgx/encl.h index fec43ca65065..8ffb8a83263f 100644 --- a/arch/x86/kernel/cpu/sgx/encl.h +++ b/arch/x86/kernel/cpu/sgx/encl.h @@ -105,8 +105,10 @@ int sgx_encl_may_map(struct sgx_encl *encl, unsigned long start, void sgx_encl_release(struct kref *ref); int sgx_encl_mm_add(struct sgx_encl *encl, struct mm_struct *mm); -int sgx_encl_get_backing(struct sgx_encl *encl, unsigned long page_index, - struct sgx_backing *backing); +int sgx_encl_alloc_backing(struct sgx_encl *encl, unsigned long page_index, + struct sgx_backing *backing); +int sgx_encl_lookup_backing(struct sgx_encl *encl, unsigned long page_index, + struct sgx_backing *backing); void sgx_encl_put_backing(struct sgx_backing *backing, bool do_write); int sgx_encl_test_and_clear_young(struct mm_struct *mm, struct sgx_encl_page *page); diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index 261e3702aef9..990f341bbd30 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -347,8 +347,8 @@ static void sgx_reclaimer_write(struct sgx_epc_page *epc_page, encl->secs_child_cnt--; if (!encl->secs_child_cnt && test_bit(SGX_ENCL_INITIALIZED, &encl->flags)) { - ret = sgx_encl_get_backing(encl, PFN_DOWN(encl->size), - &secs_backing); + ret = sgx_encl_alloc_backing(encl, PFN_DOWN(encl->size), + &secs_backing); if (ret) goto out; @@ -418,7 +418,7 @@ static void sgx_reclaim_pages(void) goto skip; page_index = PFN_DOWN(encl_page->desc - encl_page->encl->base); - ret = sgx_encl_get_backing(encl_page->encl, page_index, &backing[i]); + ret = sgx_encl_alloc_backing(encl_page->encl, page_index, &backing[i]); if (ret) goto skip;