Re: [PATCH] x86/sgx: fix a NULL pointer

On Mon Jul 17, 2023 at 6:17 PM UTC, Haitao Huang wrote:

Under heavy load, the SGX EPC reclaimers (current ksgxd or future EPC cgroup worker) 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 load it if it was reclaimed.

Fixes: 5a90d2c3f5ef8 ("x86/sgx: Support adding of pages to an initialized enclave") Cc: stable@vger.kernel.org Signed-off-by: Haitao Huang haitao.huang@linux.intel.com

---

arch/x86/kernel/cpu/sgx/encl.c | 25 ++++++++++++++++++++----- arch/x86/kernel/cpu/sgx/main.c | 4 ++++ 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c index 2a0e90fe2abc..d39e502bb7b0 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);
  

• }

remove curly braces

• 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;
  

• goto err_out_unlock;
  

• }
• epc_page = sgx_alloc_epc_page(encl_page, false); if (IS_ERR(epc_page)) { if (PTR_ERR(epc_page) == -EBUSY)

diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index 166692f2d501..4662a364ce62 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -257,6 +257,10 @@ static void sgx_reclaimer_write(struct sgx_epc_page *epc_page, mutex_lock(&encl->lock);

• /* Should not be possible */
• if (WARN_ON(!(encl->secs.epc_page)))

• goto out;
  

• sgx_encl_ewb(epc_page, backing); encl_page->epc_page = NULL; encl->secs_child_cnt--;

-- 2.25.1

BR, Jarkko