Linux-Sgx Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] x86/sgx: Sanitize pages in two rounds
@ 2020-02-06 16:15 Jarkko Sakkinen
  2020-02-09 21:04 ` Jarkko Sakkinen
  0 siblings, 1 reply; 2+ messages in thread
From: Jarkko Sakkinen @ 2020-02-06 16:15 UTC (permalink / raw)
  To: linux-sgx; +Cc: Jarkko Sakkinen, x86, Sean Christopherson

WARN_ONCE() can trigger after kexec() on systems with multiple EPC
sections. This can happen when an enclave has a SECS page in some section
and child pages in a section processed after the section containing the
SECS page. CPU does not allow to remove SECS before all of its children
have been removed.

Fix this by removing the tail from sgx_sanitize_section() and iterate
sections in two rounds by calling the resulting function.

Finally, report to the user space only after all processing has been
done and not in the middle of processing as before. This improves the
quality of reporting as kernel can tell how many unsanitized pages in
each section was left unprocessed.

Cc: x86@kernel.org
Reported-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
This is a patch to fix the issue in the version 25 of the SGX patch set.

Apply against https://github.com/jsakkine-intel/linux-sgx.git.
 arch/x86/kernel/cpu/sgx/reclaim.c | 38 ++++++++++++++-----------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/reclaim.c b/arch/x86/kernel/cpu/sgx/reclaim.c
index a33f1c45477a..5a88e1c388b5 100644
--- a/arch/x86/kernel/cpu/sgx/reclaim.c
+++ b/arch/x86/kernel/cpu/sgx/reclaim.c
@@ -18,10 +18,6 @@ DECLARE_WAIT_QUEUE_HEAD(ksgxswapd_waitq);
 LIST_HEAD(sgx_active_page_list);
 DEFINE_SPINLOCK(sgx_active_page_list_lock);
 
-/*
- * Reset all pages to uninitialized state. Pages could be in initialized on
- * kmemexec.
- */
 static void sgx_sanitize_section(struct sgx_epc_section *section)
 {
 	struct sgx_epc_page *page, *tmp;
@@ -47,23 +43,6 @@ static void sgx_sanitize_section(struct sgx_epc_section *section)
 
 		cond_resched();
 	}
-
-	list_for_each_entry_safe(page, tmp, &secs_list, list) {
-		if (kthread_should_stop())
-			return;
-
-		ret = __eremove(sgx_epc_addr(page));
-		if (!WARN_ON_ONCE(ret)) {
-			spin_lock(&section->lock);
-			list_move(&page->list, &section->page_list);
-			spin_unlock(&section->lock);
-		} else {
-			list_del(&page->list);
-			kfree(page);
-		}
-
-		cond_resched();
-	}
 }
 
 static int ksgxswapd(void *p)
@@ -72,9 +51,26 @@ static int ksgxswapd(void *p)
 
 	set_freezable();
 
+	/*
+	 * Reset all pages to uninitialized state. Pages could be in initialized
+	 * on kmemexec.
+	 */
+	for (i = 0; i < sgx_nr_epc_sections; i++)
+		sgx_sanitize_section(&sgx_epc_sections[i]);
+
+	/*
+	 * 2nd round for the SECS pages as they cannot be removed when they
+	 * still hold child pages.
+	 */
 	for (i = 0; i < sgx_nr_epc_sections; i++)
 		sgx_sanitize_section(&sgx_epc_sections[i]);
 
+	/* Check that all pages were removed. */
+	for (i = 0; i < sgx_nr_epc_sections; i++) {
+		if (!list_empty(&sgx_epc_sections[i].unsanitized_page_list))
+			WARN(1, "EPC section %d has unsanitized pages.\n", i);
+	}
+
 	while (!kthread_should_stop()) {
 		if (try_to_freeze())
 			continue;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] x86/sgx: Sanitize pages in two rounds
  2020-02-06 16:15 [PATCH] x86/sgx: Sanitize pages in two rounds Jarkko Sakkinen
@ 2020-02-09 21:04 ` Jarkko Sakkinen
  0 siblings, 0 replies; 2+ messages in thread
From: Jarkko Sakkinen @ 2020-02-09 21:04 UTC (permalink / raw)
  To: linux-sgx; +Cc: x86, Sean Christopherson

On Thu, Feb 06, 2020 at 06:15:50PM +0200, Jarkko Sakkinen wrote:
> WARN_ONCE() can trigger after kexec() on systems with multiple EPC
> sections. This can happen when an enclave has a SECS page in some section
> and child pages in a section processed after the section containing the
> SECS page. CPU does not allow to remove SECS before all of its children
> have been removed.
> 
> Fix this by removing the tail from sgx_sanitize_section() and iterate
> sections in two rounds by calling the resulting function.
> 
> Finally, report to the user space only after all processing has been
> done and not in the middle of processing as before. This improves the
> quality of reporting as kernel can tell how many unsanitized pages in
> each section was left unprocessed.
> 
> Cc: x86@kernel.org
> Reported-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Merged.

/Jarkko

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-06 16:15 [PATCH] x86/sgx: Sanitize pages in two rounds Jarkko Sakkinen
2020-02-09 21:04 ` Jarkko Sakkinen

Linux-Sgx Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-sgx/0 linux-sgx/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-sgx linux-sgx/ https://lore.kernel.org/linux-sgx \
		linux-sgx@vger.kernel.org
	public-inbox-index linux-sgx

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-sgx


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git