Hi
On Mon, 17 Jul 2023 20:39:31 -0500, Huang, Kai <kai.huang@xxxxxxxxx> wrote:
On Mon, 2023-07-17 at 17:45 -0700, Haitao Huang wrote:
Under heavy load, the SGX EPC reclaimer (ksgxd) may reclaim the SECS EPC
page for an enclave and set encl->secs.epc_page to NULL. But the SECS
EPC page is used for EAUG in the SGX #PF handler without checking for
NULL and reloading.
Fix this by checking if SECS is loaded before EAUG and loading it if it
was
reclaimed.
Nit:
Looks the sentence break of the second paragraph isn't consistent with
the first
paragraph, i.e., looks the first line is too long and the "was" should
be broken
to the second line.
Yeah, I think I forgot to reformat this line after revising.
And I think even for this simple patch, you are sending too frequently.
The
patch subject should contain the version number too so people can
distinguish it
from the previous one. Even better, please use "--base=auto" when
generating
the patch so people can know the base commit to apply to.
I'll send the next one as V2 and start a separate email thread.
Fixes: 5a90d2c3f5ef ("x86/sgx: Support adding of pages to an
initialized enclave")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Haitao Huang <haitao.huang@xxxxxxxxxxxxxxx>
---
arch/x86/kernel/cpu/sgx/encl.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kernel/cpu/sgx/encl.c
b/arch/x86/kernel/cpu/sgx/encl.c
index 2a0e90fe2abc..2ab544da1664 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -235,6 +235,16 @@ static struct sgx_epc_page *sgx_encl_eldu(struct
sgx_encl_page *encl_page,
return epc_page;
}
+static struct sgx_epc_page *sgx_encl_load_secs(struct sgx_encl *encl)
+{
+ struct sgx_epc_page *epc_page = encl->secs.epc_page;
+
+ if (!epc_page)
+ epc_page = sgx_encl_eldu(&encl->secs, NULL);
+
+ return epc_page;
+}
+
static struct sgx_encl_page *__sgx_encl_load_page(struct sgx_encl
*encl,
struct sgx_encl_page *entry)
{
@@ -248,11 +258,9 @@ static struct sgx_encl_page
*__sgx_encl_load_page(struct sgx_encl *encl,
return entry;
}
- if (!(encl->secs.epc_page)) {
- epc_page = sgx_encl_eldu(&encl->secs, NULL);
- if (IS_ERR(epc_page))
- return ERR_CAST(epc_page);
- }
+ epc_page = sgx_encl_load_secs(encl);
+ if (IS_ERR(epc_page))
+ return ERR_CAST(epc_page);
epc_page = sgx_encl_eldu(entry, encl->secs.epc_page);
if (IS_ERR(epc_page))
@@ -339,6 +347,13 @@ static vm_fault_t sgx_encl_eaug_page(struct
vm_area_struct *vma,
mutex_lock(&encl->lock);
+ epc_page = sgx_encl_load_secs(encl);
+ if (IS_ERR(epc_page)) {
+ if (PTR_ERR(epc_page) == -EBUSY)
+ vmret = VM_FAULT_NOPAGE;
^
Nit: two spaces here (yeah you copied from the existing code, and sorry
forgot
to point out in the previous version).
Sure. will fix in V2.
Thanks
Haitao
