From: Reinette Chatre <reinette.chatre@intel.com>
To: dave.hansen@linux.intel.com, jarkko@kernel.org,
linux-sgx@vger.kernel.org
Cc: haitao.huang@intel.com
Subject: [RFC PATCH 4/4] x86/sgx: Do not allocate backing pages when loading from backing store
Date: Thu, 28 Apr 2022 13:11:27 -0700 [thread overview]
Message-ID: <117862f7eb5bfef54d3b28f53746e6cf9e05508e.1651171455.git.reinette.chatre@intel.com> (raw)
In-Reply-To: <cover.1651171455.git.reinette.chatre@intel.com>
Preface:
This is intended as a debugging aid forming part of the investigation
into ENCLS[ELDU] returning #GP. This is not intended for inclusion.
Changelog:
The shmem backing store is used to (a) store encrypted enclave pages
when they are reclaimed from the enclave, and (b) load encrypted
enclave pages back into the enclave when they are accessed.
The same interface, sgx_encl_get_backing_page(), is used whether
a new backing page is needed to store an enclave page being
reclaimed or whether an existing backing page is loaded to reload
an enclave page to the EPC. This is because of this flow:
sgx_encl_get_backing_page()
shmem_read_mapping_page_gfp()
shmem_getpage_gfp(..., ..., ..., SGP_CACHE, ...)
With this interface used the backing pages are retrieved with the
SGP_CACHE flag that will automatically allocate a backing page if
it is not present.
In an effort to diagnose #GP ENCLS[ELDU] the interface is split
to ensure that when a backing page is expected to exist it is just
loaded (lookup), not allocated.
Replace sgx_encl_get_backing() with sgx_encl_lookup_backing() and
sgx_encl_alloc_backing() to distinguish whether a backing page needs
to be allocated or is expected to exist. sgx_encl_alloc_backing()
is used by the reclaimer during the ENCLS[EWB] flow and
sgx_encl_lookup_backing() is used in the ENCLS[ELDU] flow.
An IDA is used to keep track of PCMD page allocation to ensure these
pages are allocated once.
This patch revealed that there are scenarios where the backing store
does not contain a page expected to exist - sgx_encl_lookup_backing()
fails with -ENOENT. This would explain ENCLS[ELDU] returning #GP
since previously such a missing page would be allocated and thus
trigger a MAC verification failure.
Specifically, with the debugging included enabled an oversubscribe
stress test encounters the error:
sgx: sgx_encl_get_backing_page:847 fail 1 backing page with -2
The reason why the backing page is not present is not understood.
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
---
arch/x86/kernel/cpu/sgx/encl.c | 83 ++++++++++++++++++++++++++++-----
arch/x86/kernel/cpu/sgx/encl.h | 8 +++-
arch/x86/kernel/cpu/sgx/ioctl.c | 1 +
arch/x86/kernel/cpu/sgx/main.c | 6 +--
4 files changed, 82 insertions(+), 16 deletions(-)
diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
index e03f124ce772..22ed886dc825 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -60,7 +60,7 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page,
page_pcmd_off = sgx_encl_get_backing_page_pcmd_offset(encl, page_index);
- ret = sgx_encl_get_backing(encl, page_index, &b);
+ ret = sgx_encl_lookup_backing(encl, page_index, &b);
if (ret)
return ret;
@@ -102,8 +102,10 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page,
sgx_encl_truncate_backing_page(encl, page_index);
- if (pcmd_page_empty)
+ if (pcmd_page_empty) {
+ ida_free(&encl->pcmd_in_backing, PFN_DOWN(page_pcmd_off));
sgx_encl_truncate_backing_page(encl, PFN_DOWN(page_pcmd_off));
+ }
return ret;
}
@@ -617,6 +619,7 @@ void sgx_encl_release(struct kref *ref)
if (encl->backing)
fput(encl->backing);
+ ida_destroy(&encl->pcmd_in_backing);
cleanup_srcu_struct(&encl->srcu);
@@ -807,17 +810,39 @@ const cpumask_t *sgx_encl_cpumask(struct sgx_encl *encl)
}
static struct page *sgx_encl_get_backing_page(struct sgx_encl *encl,
- pgoff_t index)
+ pgoff_t index, enum sgp_type sgp)
{
struct inode *inode = encl->backing->f_path.dentry->d_inode;
- struct address_space *mapping = inode->i_mapping;
- gfp_t gfpmask = mapping_gfp_mask(mapping);
+ struct page *page = NULL;
+ int ret;
+
+ ret = shmem_getpage(inode, index, &page, sgp);
+ if (ret) {
+ pr_debug("%s:%d fail %d backing page with %d\n",
+ __func__, __LINE__, sgp, ret);
+ return ERR_PTR(ret);
+ }
+
+ if (!page) {
+ pr_debug("%s:%d fail %d backing page with NULL page\n",
+ __func__, __LINE__, sgp);
+ return ERR_PTR(-EFAULT);
+ }
- return shmem_read_mapping_page_gfp(mapping, index, gfpmask);
+ if (PageHWPoison(page)) {
+ pr_debug("%s:%d fail %d backing page with poison page\n",
+ __func__, __LINE__, sgp);
+ unlock_page(page);
+ put_page(page);
+ return ERR_PTR(-EIO);
+ }
+
+ unlock_page(page);
+ return page;
}
/**
- * sgx_encl_get_backing() - Pin the backing storage
+ * sgx_encl_alloc_backing() - Pin the backing storage
* @encl: an enclave pointer
* @page_index: enclave page index
* @backing: data for accessing backing storage for the page
@@ -829,18 +854,54 @@ 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)
+int sgx_encl_alloc_backing(struct sgx_encl *encl, unsigned long page_index,
+ struct sgx_backing *backing)
+{
+ pgoff_t page_pcmd_off = sgx_encl_get_backing_page_pcmd_offset(encl, page_index);
+ pgoff_t pcmd_index = PFN_DOWN(page_pcmd_off);
+ struct page *contents;
+ struct page *pcmd;
+ int ret;
+
+ contents = sgx_encl_get_backing_page(encl, page_index, SGP_CACHE);
+ if (IS_ERR(contents))
+ return PTR_ERR(contents);
+
+ ret = ida_alloc_range(&encl->pcmd_in_backing, pcmd_index,
+ pcmd_index, GFP_KERNEL);
+ if (ret == -ENOSPC) {
+ /* pcmd_index backing page already created, just look it up */
+ pcmd = sgx_encl_get_backing_page(encl, pcmd_index, SGP_NOALLOC);
+ } else if (ret >= 0) {
+ pcmd = sgx_encl_get_backing_page(encl, pcmd_index, SGP_CACHE);
+ } else {
+ pcmd = ERR_PTR(ret);
+ }
+ if (IS_ERR(pcmd)) {
+ put_page(contents);
+ return PTR_ERR(pcmd);
+ }
+
+ backing->page_index = page_index;
+ backing->contents = contents;
+ backing->pcmd = pcmd;
+ backing->pcmd_offset = page_pcmd_off & (PAGE_SIZE - 1);
+
+ return 0;
+}
+
+int sgx_encl_lookup_backing(struct sgx_encl *encl, unsigned long page_index,
+ struct sgx_backing *backing)
{
pgoff_t page_pcmd_off = sgx_encl_get_backing_page_pcmd_offset(encl, page_index);
struct page *contents;
struct page *pcmd;
- contents = sgx_encl_get_backing_page(encl, page_index);
+ contents = sgx_encl_get_backing_page(encl, page_index, SGP_NOALLOC);
if (IS_ERR(contents))
return PTR_ERR(contents);
- pcmd = sgx_encl_get_backing_page(encl, PFN_DOWN(page_pcmd_off));
+ pcmd = sgx_encl_get_backing_page(encl, PFN_DOWN(page_pcmd_off), SGP_NOALLOC);
if (IS_ERR(pcmd)) {
put_page(contents);
return PTR_ERR(pcmd);
diff --git a/arch/x86/kernel/cpu/sgx/encl.h b/arch/x86/kernel/cpu/sgx/encl.h
index 66adb8faec45..2a8d3bd3338f 100644
--- a/arch/x86/kernel/cpu/sgx/encl.h
+++ b/arch/x86/kernel/cpu/sgx/encl.h
@@ -8,6 +8,7 @@
#define _X86_ENCL_H
#include <linux/cpumask.h>
+#include <linux/idr.h>
#include <linux/kref.h>
#include <linux/list.h>
#include <linux/mm_types.h>
@@ -62,6 +63,7 @@ struct sgx_encl {
cpumask_t cpumask;
struct file *backing;
+ struct ida pcmd_in_backing;
struct kref refcount;
struct list_head va_pages;
unsigned long mm_list_version;
@@ -107,8 +109,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);
const cpumask_t *sgx_encl_cpumask(struct sgx_encl *encl);
-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/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index 1c2f40b72551..94d3817b40ff 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -82,6 +82,7 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
}
encl->backing = backing;
+ ida_init(&encl->pcmd_in_backing);
secs_epc = sgx_alloc_epc_page(&encl->secs, true);
if (IS_ERR(secs_epc)) {
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index ae79b8d6f645..148ec695b1b3 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -255,8 +255,8 @@ static void sgx_reclaimer_write(struct sgx_epc_page *epc_page,
sgx_encl_put_backing(backing, true);
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;
@@ -326,7 +326,7 @@ static void sgx_reclaim_pages(void)
page_index = PFN_DOWN(encl_page->desc - encl_page->encl->base);
mutex_lock(&encl_page->encl->lock);
- 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) {
mutex_unlock(&encl_page->encl->lock);
goto skip;
--
2.25.1
next prev parent reply other threads:[~2022-04-28 20:11 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-28 20:11 [RFC PATCH 0/4] SGX shmem backing store issue Reinette Chatre
2022-04-28 20:11 ` [RFC PATCH 1/4] x86/sgx: Do not free backing memory on ENCLS[ELDU] failure Reinette Chatre
2022-04-28 21:30 ` Dave Hansen
2022-04-28 22:20 ` Reinette Chatre
2022-04-28 22:53 ` Dave Hansen
2022-04-28 23:49 ` Reinette Chatre
2022-05-03 2:01 ` Kai Huang
2022-05-07 17:25 ` Jarkko Sakkinen
2022-05-09 17:17 ` Reinette Chatre
2022-05-10 0:36 ` Kai Huang
2022-05-11 10:26 ` Jarkko Sakkinen
2022-05-11 18:29 ` Haitao Huang
2022-05-11 22:00 ` Kai Huang
2022-05-12 21:14 ` Jarkko Sakkinen
2022-05-06 22:09 ` Jarkko Sakkinen
2022-04-28 20:11 ` [RFC PATCH 2/4] x86/sgx: Set dirty bit after modifying page contents Reinette Chatre
2022-04-28 21:40 ` Dave Hansen
2022-04-28 22:41 ` Reinette Chatre
2022-05-06 22:27 ` Jarkko Sakkinen
2022-05-06 22:40 ` Reinette Chatre
2022-05-07 18:01 ` Jarkko Sakkinen
2022-04-28 20:11 ` [RFC PATCH 3/4] x86/sgx: Obtain backing storage page with enclave mutex held Reinette Chatre
2022-04-28 21:58 ` Dave Hansen
2022-04-28 22:44 ` Reinette Chatre
2022-05-06 22:43 ` Jarkko Sakkinen
2022-04-28 20:11 ` Reinette Chatre [this message]
2022-04-28 21:12 ` [RFC PATCH 0/4] SGX shmem backing store issue Dave Hansen
2022-04-29 18:50 ` Reinette Chatre
2022-04-29 19:45 ` Dave Hansen
2022-04-30 3:22 ` Reinette Chatre
2022-04-30 15:52 ` Reinette Chatre
2022-05-02 14:36 ` Dave Hansen
2022-05-02 17:11 ` Reinette Chatre
2022-05-02 21:33 ` Dave Hansen
2022-05-04 22:13 ` Reinette Chatre
2022-05-04 22:58 ` Dave Hansen
2022-05-04 23:36 ` Reinette Chatre
2022-05-04 23:50 ` Dave Hansen
2022-05-05 0:08 ` Reinette Chatre
2022-05-04 23:05 ` Dave Hansen
2022-05-07 17:46 ` Jarkko Sakkinen
2022-05-07 17:48 ` Jarkko Sakkinen
2022-05-09 17:09 ` Reinette Chatre
2022-05-10 22:28 ` Jarkko Sakkinen
2022-05-11 17:23 ` Reinette Chatre
2022-05-12 14:10 ` Jarkko Sakkinen
2022-04-28 21:29 ` Dave Hansen
2022-04-28 22:20 ` Reinette Chatre
2022-05-04 6:40 ` Jarkko Sakkinen
2022-05-05 6:09 ` Jarkko Sakkinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=117862f7eb5bfef54d3b28f53746e6cf9e05508e.1651171455.git.reinette.chatre@intel.com \
--to=reinette.chatre@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=haitao.huang@intel.com \
--cc=jarkko@kernel.org \
--cc=linux-sgx@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).