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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux