diff mbox series

[v2] x86/sgx: Do not consider unsanitized pages an error

Message ID 20220825061710.256125-1-jarkko@kernel.org (mailing list archive)
State New, archived
Headers show
Series [v2] x86/sgx: Do not consider unsanitized pages an error | expand

Commit Message

Jarkko Sakkinen Aug. 25, 2022, 6:17 a.m. UTC 
If sgx_dirty_page_list ends up being non-empty, currently this triggers
WARN_ON(), which produces a lot of noise, and can potentially crash the
kernel, depending on the kernel command line.

However, if the SGX subsystem initialization is retracted, the sanitization
process could end up in the middle, and sgx_dirty_page_list be left
non-empty for legit reasons.

Replace this faulty behavior with more verbose version
__sgx_sanitize_pages(), which can optionally print EREMOVE error code and
the number of unsanitized pages.

Link: https://lore.kernel.org/linux-sgx/20220825051827.246698-1-jarkko@kernel.org/T/#u
Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
Fixes: 51ab30eb2ad4 ("x86/sgx: Replace section->init_laundry_list with sgx_dirty_page_list")
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

Cc: Haitao Huang <haitao.huang@linux.intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Reinette Chatre <reinette.chatre@intel.com>
---
v2:
- Replaced WARN_ON() with optional pr_info() inside
  __sgx_sanitize_pages().
- Rewrote the commit message.
- Added the fixes tag.
---
 arch/x86/kernel/cpu/sgx/main.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 515e2a5f25bb..b57118f8641d 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -50,16 +50,17 @@  static LIST_HEAD(sgx_dirty_page_list);
  * from the input list, and made available for the page allocator. SECS pages
  * prepending their children in the input list are left intact.
  */
-static void __sgx_sanitize_pages(struct list_head *dirty_page_list)
+static void __sgx_sanitize_pages(struct list_head *dirty_page_list, bool verbose)
 {
 	struct sgx_epc_page *page;
+	int dirty_count = 0;
 	LIST_HEAD(dirty);
 	int ret;
 
 	/* dirty_page_list is thread-local, no need for a lock: */
 	while (!list_empty(dirty_page_list)) {
 		if (kthread_should_stop())
-			return;
+			break;
 
 		page = list_first_entry(dirty_page_list, struct sgx_epc_page, list);
 
@@ -90,14 +91,27 @@  static void __sgx_sanitize_pages(struct list_head *dirty_page_list)
 			list_del(&page->list);
 			sgx_free_epc_page(page);
 		} else {
+			if (verbose)
+				pr_err_ratelimited(EREMOVE_ERROR_MESSAGE, ret, ret);
+
 			/* The page is not yet clean - move to the dirty list. */
 			list_move_tail(&page->list, &dirty);
+			dirty_count++;
 		}
 
 		cond_resched();
 	}
 
 	list_splice(&dirty, dirty_page_list);
+
+	/*
+	 * In addition to the kexec usual scenario, if the driver and/or KVM
+	 * does not initialize, ksgx will be stopped, which can leave pages
+	 * unsanitized. It's legit behaviour but it does not hurt to make it
+	 * visible.
+	 */
+	if (verbose && dirty_count > 0)
+		pr_info("%d unsanitized pages\n", dirty_count);
 }
 
 static bool sgx_reclaimer_age(struct sgx_epc_page *epc_page)
@@ -394,8 +408,8 @@  static int ksgxd(void *p)
 	 * Sanitize pages in order to recover from kexec(). The 2nd pass is
 	 * required for SECS pages, whose child pages blocked EREMOVE.
 	 */
-	__sgx_sanitize_pages(&sgx_dirty_page_list);
-	__sgx_sanitize_pages(&sgx_dirty_page_list);
+	__sgx_sanitize_pages(&sgx_dirty_page_list, false);
+	__sgx_sanitize_pages(&sgx_dirty_page_list, true);
 
 	/* sanity check: */
 	WARN_ON(!list_empty(&sgx_dirty_page_list));